Skip to content

Commit

Permalink
validation BUGFIX avoid accessing reallocated memory
Browse files Browse the repository at this point in the history
Getnext items are directly stored in a HT,
which could be resized while it was used.
  • Loading branch information
michalvasko committed Dec 4, 2024
1 parent 631ccc5 commit dadf574
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 27 deletions.
13 changes: 6 additions & 7 deletions src/tree_data_new.c
Original file line number Diff line number Diff line change
Expand Up @@ -1916,11 +1916,10 @@ lyd_new_implicit(struct lyd_node *parent, struct lyd_node **first, const struct
uint32_t impl_opts, struct ly_ht *getnext_ht, struct lyd_node **diff)
{
LY_ERR ret;
const struct lysc_node *snode;
const struct lysc_node *snode, **choices, **snodes;
struct lyd_node *node = NULL;
struct lyd_value **dflts;
LY_ARRAY_COUNT_TYPE u;
const struct lyd_val_getnext *getnext;
uint32_t i;

assert(first && (parent || sparent || mod));
Expand All @@ -1930,11 +1929,11 @@ lyd_new_implicit(struct lyd_node *parent, struct lyd_node **first, const struct
}

/* get cached getnext schema nodes */
LY_CHECK_RET(lyd_val_getnext_get(sparent, mod, impl_opts & LYD_IMPLICIT_OUTPUT, getnext_ht, &getnext));
LY_CHECK_RET(lyd_val_getnext_get(sparent, mod, impl_opts & LYD_IMPLICIT_OUTPUT, getnext_ht, &choices, &snodes));

/* choice nodes */
for (i = 0; getnext->choices && getnext->choices[i]; ++i) {
snode = getnext->choices[i];
for (i = 0; choices && choices[i]; ++i) {
snode = choices[i];

if ((impl_opts & LYD_IMPLICIT_NO_STATE) && (snode->flags & LYS_CONFIG_R)) {
continue;
Expand All @@ -1956,8 +1955,8 @@ lyd_new_implicit(struct lyd_node *parent, struct lyd_node **first, const struct
}

/* container, leaf, leaf-list nodes */
for (i = 0; getnext->snodes && getnext->snodes[i]; ++i) {
snode = getnext->snodes[i];
for (i = 0; snodes && snodes[i]; ++i) {
snode = snodes[i];

if ((impl_opts & LYD_IMPLICIT_NO_STATE) && (snode->flags & LYS_CONFIG_R)) {
continue;
Expand Down
38 changes: 20 additions & 18 deletions src/validation.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,17 @@ lyd_val_getnext_ht_free(struct ly_ht *getnext_ht)

LY_ERR
lyd_val_getnext_get(const struct lysc_node *sparent, const struct lys_module *mod, ly_bool output,
struct ly_ht *getnext_ht, const struct lyd_val_getnext **getnext_p)
struct ly_ht *getnext_ht, const struct lysc_node ***choices, const struct lysc_node ***snodes)
{
LY_ERR rc = LY_SUCCESS;
struct lyd_val_getnext val = {0};
struct lyd_val_getnext val = {0}, *getnext = NULL;
const struct lysc_node *snode = NULL;
uint32_t getnext_opts, snode_count = 0, choice_count = 0;

/* try to find the entry for this schema parent */
val.sparent = sparent;
if (!lyht_find(getnext_ht, &val, (uintptr_t)sparent, (void **)getnext_p)) {
return LY_SUCCESS;
if (!lyht_find(getnext_ht, &val, (uintptr_t)sparent, (void **)&getnext)) {
goto cleanup;
}

/* traverse all the children using getnext and store them */
Expand Down Expand Up @@ -143,14 +143,17 @@ lyd_val_getnext_get(const struct lysc_node *sparent, const struct lys_module *mo
}

/* add into the hash table */
if ((rc = lyht_insert(getnext_ht, &val, (uintptr_t)sparent, (void **)getnext_p))) {
if ((rc = lyht_insert(getnext_ht, &val, (uintptr_t)sparent, (void **)&getnext))) {
goto cleanup;
}

cleanup:
if (rc) {
free(val.snodes);
free(val.choices);
} else {
*choices = getnext->choices;
*snodes = getnext->snodes;
}
return rc;
}
Expand Down Expand Up @@ -901,23 +904,23 @@ lyd_validate_choice_r(struct lyd_node **first, const struct lysc_node *sparent,
uint32_t val_opts, uint32_t int_opts, struct ly_ht *getnext_ht, struct lyd_node **diff)
{
LY_ERR r, rc = LY_SUCCESS;
const struct lyd_val_getnext *getnext;
const struct lysc_node **choices, **snodes;
uint32_t i;

/* get cached getnext schema nodes */
rc = lyd_val_getnext_get(sparent, mod, int_opts & LYD_INTOPT_REPLY, getnext_ht, &getnext);
rc = lyd_val_getnext_get(sparent, mod, int_opts & LYD_INTOPT_REPLY, getnext_ht, &choices, &snodes);
LY_CHECK_GOTO(rc, cleanup);
if (!getnext->choices) {
if (!choices) {
goto cleanup;
}

for (i = 0; *first && getnext->choices[i]; ++i) {
for (i = 0; *first && choices[i]; ++i) {
/* check case duplicites */
r = lyd_validate_cases(first, mod, (struct lysc_node_choice *)getnext->choices[i], diff);
r = lyd_validate_cases(first, mod, (struct lysc_node_choice *)choices[i], diff);
LY_VAL_ERR_GOTO(r, rc = r, val_opts, cleanup);

/* check for nested choice */
r = lyd_validate_choice_r(first, getnext->choices[i], mod, val_opts, int_opts, getnext_ht, diff);
r = lyd_validate_choice_r(first, choices[i], mod, val_opts, int_opts, getnext_ht, diff);
LY_VAL_ERR_GOTO(r, rc = r, val_opts, cleanup);
}

Expand Down Expand Up @@ -1495,18 +1498,17 @@ lyd_validate_siblings_schema_r(const struct lyd_node *first, const struct lyd_no
struct ly_ht *getnext_ht)
{
LY_ERR r, rc = LY_SUCCESS;
const struct lyd_val_getnext *getnext;
const struct lysc_node *snode, *scase;
const struct lysc_node *snode, *scase, **choices, **snodes;
struct lysc_node_list *slist;
struct lysc_node_leaflist *sllist;
uint32_t i;

/* get cached getnext schema nodes */
rc = lyd_val_getnext_get(sparent, mod, int_opts & LYD_INTOPT_REPLY, getnext_ht, &getnext);
rc = lyd_val_getnext_get(sparent, mod, int_opts & LYD_INTOPT_REPLY, getnext_ht, &choices, &snodes);
LY_CHECK_GOTO(rc, cleanup);

for (i = 0; getnext->choices && getnext->choices[i]; ++i) {
snode = getnext->choices[i];
for (i = 0; choices && choices[i]; ++i) {
snode = choices[i];

if ((val_opts & LYD_VALIDATE_NO_STATE) && (snode->flags & LYS_CONFIG_R)) {
/* skip state nodes */
Expand All @@ -1530,8 +1532,8 @@ lyd_validate_siblings_schema_r(const struct lyd_node *first, const struct lyd_no
}
}

for (i = 0; getnext->snodes && getnext->snodes[i]; ++i) {
snode = getnext->snodes[i];
for (i = 0; snodes && snodes[i]; ++i) {
snode = snodes[i];

if ((val_opts & LYD_VALIDATE_NO_STATE) && (snode->flags & LYS_CONFIG_R)) {
/* skip state nodes */
Expand Down
7 changes: 5 additions & 2 deletions src/validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,18 @@ void lyd_val_getnext_ht_free(struct ly_ht *getnext_ht);
/**
* @brief Get the schema children of a schema parent.
*
* Getnext structure cannot be returned because the pointer may become invalid on HT resize.
*
* @param[in] sparent Schema parent to use.
* @param[in] mod Module to use.
* @param[in] output Whether to traverse operation output instead of input nodes.
* @param[in,out] getnext_ht Getnext HT to use, new @p sparent is added to it.
* @param[out] getnext_p Filled getnext structure.
* @param[out] choices Array of getnext choices of @p sparent.
* @param[out] snodes Array of getnext schema nodes except for choices of @p sparent.
* @return LY_ERR value.
*/
LY_ERR lyd_val_getnext_get(const struct lysc_node *sparent, const struct lys_module *mod, ly_bool output,
struct ly_ht *getnext_ht, const struct lyd_val_getnext **getnext_p);
struct ly_ht *getnext_ht, const struct lysc_node ***choices, const struct lysc_node ***snodes);

/**
* @brief Add new changes into a diff. They are always merged.
Expand Down

0 comments on commit dadf574

Please sign in to comment.