Fix multiple must deviations to same node

A segmentation fault can occur when multiple deviations add must deviations
to the same target node. The sequence of events is as follows:

1. Some number of must deviations have already been parsed, and the
module's deviation array and the target node are populated to point an array
of `struct lys_restr`.
2. An additional must deviation is parsed, deviating the same target node,
and the `ly_realloc()` in `yang_check_deviate()` (for YANG parsing) or
`yin_fill_deviation()` (for YIN parsing) cannot extend the existing buffer
any further, and so it frees the original buffer and allocates a new, larger
buffer.
3. Any existing must deviations on the module that point to that same target
node now point to memory freed by `ly_realloc()`. Attempts to use that memory
will result in undefined behavior, including a segmentation fault.

I've added some additional leafs and must deviations to those leafs in the
test models that demonstrate this problem. `test_parse_print` and
`test_deviation` will crash with a segmentation fault given these models.

(Unrelated to these changes, I also updated leafs 9-10 of all.yang to match
all.yin. These nodes were changed in all.yin in c27114c, but all.yang was
never updated to match.)

I've also implemented a possible fix. If the deviation is an `add`, the
module's deviation now contains a shallow copy of the `struct lys_restr` array
pointed to by the target node, instead of them pointing to the same memory.
(This meant I had to add a shallow free of this copy when the module was
cleaned up. See `lys_sub_module_remove_devs_augs()`.)

Another possible solution, which I did not implement, might be for the parser
to revisit all siblings of the module's deviations when `ly_realloc()` freed
the original memory. We would also have to keep track of what offset each
module deviation pointed to within the larger target node array.

Author: alangefe

src/parser_yang.c

@@ -1878,8 +1878,6 @@ yang_check_deviate_must(struct lys_module *module, struct unres_schema *unres,
         LY_CHECK_ERR_GOTO(!must, LOGMEM(ctx), error);
         *trg_must = must;
         memcpy(&(*trg_must)[*trg_must_size], deviate->must, deviate->must_size * sizeof *must);
-        free(deviate->must);
-        deviate->must = &must[*trg_must_size];
         *trg_must_size = *trg_must_size + deviate->must_size;
         erase_must = 0;
     } else if (deviate->mod == LY_DEVIATE_DEL) {

src/parser_yin.c

@@ -2186,6 +2186,7 @@ fill_yin_deviation(struct lys_module *module, struct lyxml_elem *yin, struct lys
     struct unres_schema tmp_unres;
     struct lys_module *mod;
     void *reallocated;
+    size_t deviate_must_index;
 
     GETVAL(ctx, value, yin, "target-node");
     dev->target_name = transform_schema2json(module, value);
@@ -2727,10 +2728,10 @@ fill_yin_deviation(struct lys_module *module, struct lyxml_elem *yin, struct lys
                 goto error;
             } else if (d->mod == LY_DEVIATE_ADD) {
                 /* reallocate the must array of the target */
-                d->must = ly_realloc(*trg_must, (c_must + *trg_must_size) * sizeof *d->must);
-                LY_CHECK_ERR_GOTO(!d->must, LOGMEM(ctx), error);
-                *trg_must = d->must;
-                d->must = &((*trg_must)[*trg_must_size]);
+                struct lys_restr *must = ly_realloc(*trg_must, (c_must + *trg_must_size) * sizeof *d->must);
+                LY_CHECK_ERR_GOTO(!must, LOGMEM(ctx), error);
+                *trg_must = must;
+                d->must = calloc(c_must, sizeof *d->must);
                 d->must_size = c_must;
             } else { /* LY_DEVIATE_DEL */
                 d->must = calloc(c_must, sizeof *d->must);
@@ -2824,6 +2825,7 @@ fill_yin_deviation(struct lys_module *module, struct lyxml_elem *yin, struct lys
         }
 
         /* process deviation properties with 0..n cardinality */
+        deviate_must_index = 0;
         LY_TREE_FOR_SAFE(develem->child, next2, child) {
             if (strcmp(child->ns->value, LY_NSYIN)) {
                 /* extension */
@@ -2879,6 +2881,8 @@ fill_yin_deviation(struct lys_module *module, struct lyxml_elem *yin, struct lys
                     if (fill_yin_must(module, child, &((*trg_must)[*trg_must_size]), unres)) {
                         goto error;
                     }
+                    memcpy(d->must + deviate_must_index, &((*trg_must)[*trg_must_size]), sizeof *d->must);
+                    ++deviate_must_index;
                     (*trg_must_size)++;
                 }
 

src/tree_schema.c

@@ -4702,7 +4702,7 @@ lys_sub_module_apply_devs_augs(struct lys_module *module)
 void
 lys_sub_module_remove_devs_augs(struct lys_module *module)
 {
-    uint8_t u, v;
+    uint8_t u, v, w;
     struct unres_schema *unres;
 
     unres = calloc(1, sizeof *unres);
@@ -4714,6 +4714,14 @@ lys_sub_module_remove_devs_augs(struct lys_module *module)
         if (module->deviation[u].orig_node) {
             remove_dev(&module->deviation[u], module, unres);
         }
+
+        /* Free the deviation's must array(s). These are shallow copies of the arrays
+           on the target node(s), so a deep free is not needed. */
+        for (v = 0; v < module->deviation[u].deviate_size; ++v) {
+            if (module->deviation[u].deviate[v].mod == LY_DEVIATE_ADD) {
+                free(module->deviation[u].deviate[v].must);
+            }
+        }
     }
     /* remove applied augments */
     for (u = 0; u < module->augment_size; ++u) {
@@ -4726,6 +4734,14 @@ lys_sub_module_remove_devs_augs(struct lys_module *module)
             if (module->inc[v].submodule->deviation[u].orig_node) {
                 remove_dev(&module->inc[v].submodule->deviation[u], module, unres);
             }
+
+            /* Free the deviation's must array(s). These are shallow copies of the arrays
+               on the target node(s), so a deep free is not needed. */
+            for (w = 0; w < module->inc[v].submodule->deviation[u].deviate_size; ++w) {
+                if (module->inc[v].submodule->deviation[u].deviate[w].mod == LY_DEVIATE_ADD) {
+                    free(module->inc[v].submodule->deviation[u].deviate[w].must);
+                }
+            }
         }
 
         for (u = 0; u < module->inc[v].submodule->augment_size; ++u) {

tests/data/files/all-dev.yang

@@ -39,4 +39,18 @@ module all-dev {
   deviation "/all_mod:cont1/all_mod:leaf23" {
     deviate not-supported;
   }
+
+  deviation "/all_mod:cont1/all_mod:must-deviations-container" {
+    deviate add {
+      must "all_mod:leaf30 != 'foo'";
+    }
+  }
+
+  deviation "/all_mod:cont1/all_mod:must-deviations-container" {
+    deviate add {
+      must "all_mod:leaf30 != 'bar'";
+      must "all_mod:leaf30 != 'baz'";
+      must "all_mod:leaf30 != 'qux'";
+    }
+  }
 }

tests/data/files/all-dev.yin

@@ -38,4 +38,16 @@
   <deviation target-node="/all_mod:cont1/all_mod:leaf23">
     <deviate value="not-supported"/>
   </deviation>
+  <deviation target-node="/all_mod:cont1/all_mod:must-deviations-container">
+    <deviate value="add">
+      <must condition="all_mod:leaf30 != 'foo'"/>
+    </deviate>
+  </deviation>
+  <deviation target-node="/all_mod:cont1/all_mod:must-deviations-container">
+    <deviate value="add">
+      <must condition="all_mod:leaf30 != 'bar'"/>
+      <must condition="all_mod:leaf30 != 'baz'"/>
+      <must condition="all_mod:leaf30 != 'qux'"/>
+    </deviate>
+  </deviation>
 </module>

tests/data/files/all.yang

@@ -100,19 +100,23 @@ module all {
     }
 
     choice choic1 {
-      default "leaf10";
-      leaf leaf9 {
+      default "leaf9b";
+      leaf leaf9a {
         type decimal64 {
           fraction-digits 9;
         }
       }
 
-      leaf leaf10 {
+      leaf leaf9b {
         type boolean;
         default "false";
       }
     }
 
+    leaf leaf10 {
+      type boolean;
+    }
+
     leaf leaf11 {
       type enumeration {
         enum "one";
@@ -218,6 +222,13 @@ module all {
       type instance-identifier;
     }
 
+    container must-deviations-container {
+      presence "Allows deviations on the leaf";
+      leaf leaf30 {
+        type string;
+      }
+    }
+
     leaf leaf23 {
       type empty;
     }

tests/data/files/all.yin

@@ -191,6 +191,12 @@
     <leaf name="leaf29">
       <type name="instance-identifier"/>
     </leaf>
+    <container name="must-deviations-container">
+      <presence value="Allows deviations on the leaf"/>
+      <leaf name="leaf30">
+        <type name="string"/>
+      </leaf>
+    </container>
     <leaf name="leaf23">
       <type name="empty"/>
     </leaf>

tests/schema/yang/files/deviation1-dv-sub.yang

@@ -0,0 +1,16 @@
+submodule deviation1-dv-sub {
+  belongs-to deviation1-dv {
+    prefix dev1-dv;
+  }
+
+  import deviation1 {
+    prefix dev1;
+  }
+
+  deviation "/dev1:deviation1/dev1:cont" {
+    description "An additional must deviation.";
+    deviate add {
+      must "dev1:cont-leaf-2 != 'quux'";
+    }
+  }
+}

tests/schema/yang/files/deviation1-dv.yang

@@ -3,6 +3,8 @@ module deviation1-dv{
   namespace "urn:fri:params:xml:ns:yang:deviation1-dv";
   prefix dev1-dv;
 
+  include deviation1-dv-sub;
+
   import deviation1 {
     prefix dev1;
   }
@@ -11,7 +13,7 @@ module deviation1-dv{
     "This module contains a deviation which caused a double free crash...";
 
   deviation "/dev1:deviation1/dev1:cont"
-          + "/dev1:cont-leaf" {
+          + "/dev1:cont-leaf-1" {
     description
       "Restrict the values for the c-tag cont to only allow 0x8100
        (33024) and 0x88A8 (34984).";
@@ -24,4 +26,32 @@ module deviation1-dv{
       }
     }
   }
+
+  deviation "/dev1:deviation1/dev1:cont" {
+    description "An additional must deviation.";
+    deviate add {
+      must "dev1:cont-leaf-2 != 'foo'";
+    }
+  }
+
+  deviation "/dev1:deviation1/dev1:cont" {
+    description "An additional must deviation.";
+    deviate add {
+      must "dev1:cont-leaf-2 != 'bar'";
+    }
+  }
+
+  deviation "/dev1:deviation1/dev1:cont" {
+    description "An additional must deviation.";
+    deviate add {
+      must "dev1:cont-leaf-2 != 'baz'";
+    }
+  }
+
+  deviation "/dev1:deviation1/dev1:cont" {
+    description "An additional must deviation.";
+    deviate add {
+      must "dev1:cont-leaf-2 != 'qux'";
+    }
+  }
 }

tests/schema/yang/files/deviation1.yang

@@ -16,13 +16,19 @@ module deviation1 {
     container cont {
       description "Just a container.";
 
-      leaf cont-leaf {
+      leaf cont-leaf-1 {
         if-feature a-feature;
         type uint16;
         default 0x8100;
         description
           "A container leaf.";
       }
+
+      leaf cont-leaf-2 {
+        type string;
+        description
+          "A second container leaf.";
+      }
     }
   }
 }