lib: fix crashes with leafrefs that point to non-implemented modules

Whenever libyang loads a module that contains a leafref, it will
also implicitly load the module of the referring node if it's
not loaded already. That makes sense as otherwise it wouldn't be
possible to validate the leafref value correctly.

The problem is that loading a module implicitly violates the
assumption of the northbound layer that all loaded modules
are implemented (i.e. they have a northbound node associated
to each schema node). This means that loading a module that
isn't implemented can lead to crashes as the "priv" pointer
of schema nodes is no longer guaranteed to be valid. To fix this
problem, add a few null checks to ignore data nodes associated
to non-implemented modules.

The side effect of this change is harmless. If a daemon receives
configuration it doesn't support (e.g. BFD peers on staticd),
that configuration will be stored but otherwise ignored. This can
only happen when using a northbound client like gRPC, as the CLI
will never send to a daemon a command it doesn't support. This
minor problem should go away in the long run as FRR migrates to
a centralized management model, at which point the YANG-modeled
configuration of all daemons will be maintained in a single place.

Finally, update some daemons to stop implementing YANG modules
they don't need to (i.e. revert 1b741a01c and a74b47f5).

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
This commit is contained in:
Renato Westphal 2020-09-11 10:43:49 -03:00
parent a77bd0f4e6
commit f267201bf5
4 changed files with 16 additions and 6 deletions

View File

@ -383,6 +383,10 @@ static void nb_config_diff_add_change(struct nb_config_cbs *changes,
{
struct nb_config_change *change;
/* Ignore unimplemented nodes. */
if (!dnode->schema->priv)
return;
change = XCALLOC(MTYPE_TMP, sizeof(*change));
change->cb.operation = operation;
change->cb.seq = *seq;
@ -416,6 +420,10 @@ static void nb_config_diff_created(const struct lyd_node *dnode, uint32_t *seq,
enum nb_operation operation;
struct lyd_node *child;
/* Ignore unimplemented nodes. */
if (!dnode->schema->priv)
return;
switch (dnode->schema->nodetype) {
case LYS_LEAF:
case LYS_LEAFLIST:
@ -450,6 +458,10 @@ static void nb_config_diff_created(const struct lyd_node *dnode, uint32_t *seq,
static void nb_config_diff_deleted(const struct lyd_node *dnode, uint32_t *seq,
struct nb_config_cbs *changes)
{
/* Ignore unimplemented nodes. */
if (!dnode->schema->priv)
return;
if (nb_operation_is_valid(NB_OP_DESTROY, dnode->schema))
nb_config_diff_add_change(changes, NB_OP_DESTROY, seq, dnode);
else if (CHECK_FLAG(dnode->schema->nodetype, LYS_CONTAINER)) {
@ -618,7 +630,7 @@ static int nb_candidate_validate_code(struct nb_context *context,
struct nb_node *nb_node;
nb_node = child->schema->priv;
if (!nb_node->cbs.pre_validate)
if (!nb_node || !nb_node->cbs.pre_validate)
goto next;
ret = nb_callback_pre_validate(context, nb_node, child,
@ -1385,7 +1397,7 @@ static void nb_transaction_apply_finish(struct nb_transaction *transaction,
struct nb_node *nb_node;
nb_node = dnode->schema->priv;
if (!nb_node->cbs.apply_finish)
if (!nb_node || !nb_node->cbs.apply_finish)
goto next;
/*

View File

@ -573,7 +573,7 @@ void nb_cli_show_dnode_cmds(struct vty *vty, struct lyd_node *root,
struct nb_node *nb_node;
nb_node = child->schema->priv;
if (!nb_node->cbs.cli_show)
if (!nb_node || !nb_node->cbs.cli_show)
goto next;
/* Skip default values. */
@ -591,7 +591,7 @@ void nb_cli_show_dnode_cmds(struct vty *vty, struct lyd_node *root,
parent = ly_iter_next_up(child);
if (parent != NULL) {
nb_node = parent->schema->priv;
if (nb_node->cbs.cli_show_end)
if (nb_node && nb_node->cbs.cli_show_end)
(*nb_node->cbs.cli_show_end)(vty, parent);
}

View File

@ -119,7 +119,6 @@ static struct quagga_signal_t sighandlers[] = {
static const struct frr_yang_module_info *const nhrpd_yang_modules[] = {
&frr_filter_info,
&frr_interface_info,
&frr_vrf_info,
};
FRR_DAEMON_INFO(nhrpd, NHRP, .vty_port = NHRP_VTY_PORT,

View File

@ -117,7 +117,6 @@ struct quagga_signal_t pbr_signals[] = {
static const struct frr_yang_module_info *const pbrd_yang_modules[] = {
&frr_filter_info,
&frr_interface_info,
&frr_vrf_info,
};
FRR_DAEMON_INFO(pbrd, PBR, .vty_port = PBR_VTY_PORT,