Assertion violation in lyd_defaults_add_unres

[milanlenco]

While testing validation with cross-module dependencies I came across this assertion violation:

tree_data.c:5638: lyd_defaults_add_unres: Assertion `!msg_sibling->parent || (msg_sibling->prev != msg_sibling)' failed.

Stacktrace:

#0  in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
#1  in __GI_abort () at abort.c:89
#2  in __assert_fail_base (fmt=<optimised out>, assertion=assertion@entry=0x7ffff6f728d8 "!msg_sibling->parent || (msg_sibling->prev != msg_sibling)", file=file@entry=0x7ffff6f71ce0 "tree_data.c", line=line@entry=5638, function=function@entry=0x7ffff6f72d70 <__PRETTY_FUNCTION__.9057> "lyd_defaults_add_unres") at assert.c:92
#3  in __GI___assert_fail (assertion=0x7ffff6f728d8 "!msg_sibling->parent || (msg_sibling->prev != msg_sibling)", file=0x7ffff6f71ce0 "/home/mlenco/dev/libyang/src/tree_data.c", line=5638, func
tion=0x7ffff6f72d70 <__PRETTY_FUNCTION__.9057> "lyd_defaults_add_unres") at assert.c:101
#4  in lyd_defaults_add_unres (root=0x7fffffffc5e0, options=320, ctx=0x765b50, data_tree=0x747b50, act_notif=0x780430, unres=0x794a50) at tree_data.c:5638
#5  in lyd_validate (node=0x7fffffffc5e0, options=320, var_arg=0x747b50) at tree_data.c:3728
#6  in dm_validate_procedure (dm_ctx=0x764210, session=0x754020, type=DM_PROCEDURE_EVENT_NOTIF, xpath=0x4ef538 "/test-module:kernel-modules/kernel-module[name=\"irqbypass.ko\"]/status-change",
api_variant=SR_API_VALUES, args_p=0x777ad0, arg_cnt=2, input=true, sr_mem=0x0, with_def=0x0, with_def_cnt=0x0, with_def_tree=0x0, with_def_tree_cnt=0x0) at data_manager.c:3973
#7  in dm_validate_event_notif (dm_ctx=0x764210, session=0x754020, event_notif_xpath=0x4ef538 "/test-module:kernel-modules/kernel-module[name=\"irqbypass.ko\"]/status-change", values=0x777ad0,
value_cnt=2, sr_mem=0x0, with_def=0x0, with_def_cnt=0x0, with_def_tree=0x0, with_def_tree_cnt=0x0) at data_manager.c:4082
#8  in dm_event_notif_test (state=0x73f260) at dm_test.c:656
#9  in cmocka_run_one_test_or_fixture (function_name=0x4efa1d "dm_event_notif_test", test_func=0x414796 <dm_event_notif_test>, setup_func=0x0, teardown_func=0x0, state=0x73f260, heap_check_poin
t=0x0) at cmocka.c:2534
#10 in cmocka_run_one_tests (test_state=0x73f250) at cmocka.c:2642
#11 in _cmocka_run_group_tests (group_name=0x4ef924 "tests", tests=0x7fffffffd920, num_tests=14, group_setup=0x412316 <setup>, group_teardown=0x0) at cmocka.c:2757
#12 in main () at dm_test.c:825

What is being validated is an instance of status-change notification from this module: https://github.com/milanlenco/sysrepo/blob/devel/tests/yang/test-module.yang In must condition it references leaf from this module: https://github.com/milanlenco/sysrepo/blob/devel/tests/yang/referenced-data.yang, which is therefore appended to the data tree.

The content of the notification (first argument to lyd_validate) is:

<kernel-modules xmlns="urn:ietf:params:xml:ns:yang:test-module">
  <kernel-module>
    <name>irqbypass.ko</name>
    <status-change>
      <loaded>true</loaded>
      <time-of-change>56</time-of-change>
    </status-change>
  </kernel-module>
</kernel-modules>

The content of complete data tree (with appended data of the referenced module), passed to lyd_validate as the last argument, is:

<main xmlns="urn:ietf:params:xml:ns:yang:test-module">
  <enum>maybe</enum>
  <raw>SGVsbG8gd29ybGQh</raw>
  <options>strict recursive</options>
  <dec64>9.85</dec64>
  <i8>8</i8>
  <i16>16</i16>
  <i32>32</i32>
  <i64>64</i64>
  <ui8>8</ui8>
  <ui16>16</ui16>
  <ui32>32</ui32>
  <ui64>64</ui64>
  <empty/>
  <boolean>true</boolean>
  <string>str</string>
  <id_ref>id_1</id_ref>
  <numbers>1</numbers>
  <numbers>2</numbers>
  <numbers>42</numbers>
</main>
<kernel-modules xmlns="urn:ietf:params:xml:ns:yang:test-module">
  <kernel-module>
    <name>netlink_diag.ko</name>
    <location>/lib/modules/kernel/net/netlink</location>
    <loaded>false</loaded>
  </kernel-module>
  <kernel-module>
    <name>irqbypass.ko</name>
    <location>/lib/modules/kernel/virt/lib</location>
    <loaded>true</loaded>
  </kernel-module>
  <kernel-module>
    <name>vboxvideo.ko</name>
    <location>/lib/modules/kernel/misc</location>
    <loaded>false</loaded>
  </kernel-module>
</kernel-modules>
<leafref-chain xmlns="urn:ietf:params:xml:ns:yang:test-module">
  <D>final-leaf</D>
  <C>final-leaf</C>
</leafref-chain>
<university xmlns="urn:ietf:params:xml:ns:yang:test-module">
  <students>
    <student>
      <name>nameA</name>
      <age>19</age>
    </student>
    <student>
      <name>nameB</name>
      <age>17</age>
    </student>
    <student>
      <name>nameC</name>
      <age>18</age>
    </student>
  </students>
  <classes>
    <class>
      <title>CCNA</title>
      <student>
        <name>nameB</name>
        <age>17</age>
      </student>
      <student>
        <name>nameC</name>
      </student>
    </class>
  </classes>
</university>
<list xmlns="urn:ietf:params:xml:ns:yang:test-module">
  <key>k2</key>
  <id_ref>id_2</id_ref>
  <union>infinity</union>
</list>
<list xmlns="urn:ietf:params:xml:ns:yang:test-module">
  <key>k1</key>
  <id_ref>id_1</id_ref>
  <union>42</union>
  <wireless/>
</list>
<list-b xmlns="urn:rd">
  <name>abc</name>
</list-b>
<magic_number xmlns="urn:rd">123</magic_number>

Both printed with lyd_print_fd so all the pointers seems OK. But it still could be a bug in how we append trees in sysrepo (the code is here: https://github.com/sysrepo/sysrepo/blob/master/src/data_manager.c#L1274). Do you have any idea what the issue could be?

[milanlenco]

With the same schema and also the same complete data tree, but validating action load this time, I get assertion:

tree_data.c:5620: lyd_defaults_add_unres: Assertion `act_notif->parent == data_tree_parent' failed.

The relevant part of the stacktrace is:

#3  in __GI___assert_fail (assertion=0x7ffff6f728b0 "act_notif->parent == data_tree_parent", file=0x7ffff6f71ce0 "tree_data.c", line=5620, function=0x7ffff6f72d70 <
__PRETTY_FUNCTION__.9057> "lyd_defaults_add_unres") at assert.c:101
#4  in lyd_defaults_add_unres (root=0x7fffffffc5c0, options=272, ctx=0x765b70, data_tree=0x772530, act_notif=0x7803e0, unres=0x7948e0) at tree_data.c:5620
#5  in lyd_validate (node=0x7fffffffc5c0, options=272, var_arg=0x772530) at tree_data.c:3728

The content of the action is:

<action xmlns="urn:ietf:params:xml:ns:yang:1">
  <kernel-modules xmlns="urn:ietf:params:xml:ns:yang:test-module">
    <kernel-module>
      <name>irqbypass.ko</name>
      <load>
        <params>--log-level 2</params>
      </load>
    </kernel-module>
  </kernel-modules>
</action>

I'm starting to think that we do not append trees properly in sysrepo, what do you think?

[milanlenco]

OK, so I have just learned about lyd_merge, so I'm trying to replace our code with your function as it seems more appropriate. At the moment I'm getting SIGSEGV in lyd_merge_siblings, so I will debug it and get back once I have more information. For the time being please ignore all assertion violations reported here.

[michalvasko]

Hi Milan, please slow down a bit, I started examining this issue only a while ago :) I believe you understood something wrong, maybe the documentation is not clear enough, but you are not supposed to merge those trees yourself because you would get an invalid data tree (data mixed with a notification/action). Validation does this internally, temporarily, but you should get the exact same data trees once the function finishes so it should be transparent.

But there should be no assert failures and no segmentation faults, that is what I am trying to reproduce and fix now. Please say what you passed to lyd_merge when it crashes, thanks.

Regards, Michal

PS: In your code snippet you tried to find last sibling of a list using next pointer from the first element. You can get to the last element using prev from the first element, prev pointers make a backward circular linked list. next is a forward linked list ended with NULL.

EDIT: I managed to reproduce the first issue when validating as was intended, so I will try to fix it.

[milanlenco]

Ok, I'm slowing down now :)

Regarding the SIGSEGV in lyd_merge, it looks like the child pointer of an empty leaf-list instance is not initialized to NULL.

In the yang module (https://github.com/milanlenco/sysrepo/blob/devel/tests/yang/test-module.yang) we have leaf-list on the top-level:

  leaf-list ordered-numbers {
    ordered-by user;
    type uint8;
}

But it is not set in either source or target data tree.

when it tries to merge (empty) ordered-numbers from the source to (empty) ordered-numbers from the target, it crashes in lyd_merge_siblings because src->child points to an invalid memory location. Let me know if you need more data (like content of the source and target data trees etc.) to resolve the issue.

[michalvasko]

Ok, thanks, should have enough information to reproduce it all, I'll let you know if not.

Regards, Michal

[michalvasko]

Hi Milan, it should all be fixed now.

Regards, Michal

[milanlenco]

Hi Michal, yes, all the reported assertion violations have disappeared. Two new issues, however, have emerged:

Using this yang module, with the following relevant part of the content including merged dependency:

<kernel-modules xmlns="urn:ietf:params:xml:ns:yang:test-module">
  <kernel-module>
    <name>netlink_diag.ko</name>
    <location>/lib/modules/kernel/net/netlink</location>
    <loaded>false</loaded>
  </kernel-module>
  <kernel-module>
    <name>irqbypass.ko</name>
    <location>/lib/modules/kernel/virt/lib</location>
    <loaded>true</loaded>
  </kernel-module>
  <kernel-module>
    <name>vboxvideo.ko</name>
    <location>/lib/modules/kernel/misc</location>
    <loaded>false</loaded>
  </kernel-module>
</kernel-modules>
<magic_number xmlns="urn:rd">123</magic_number>

That data tree is passed to lyd_validate as the third argument.

The data tree with an event notification for validation is as follows:

<kernel-modules xmlns="urn:ietf:params:xml:ns:yang:test-module">
  <kernel-module>
    <name>irqbypass.ko</name>
    <status-change>
      <loaded>true</loaded>
      <time-of-change>56</time-of-change>
    </status-change>
  </kernel-module>
</kernel-modules>

I get pointer to the status-change node and pass that to lyd_validate as the first arguments. Validation options are LYD_OPT_STRICT | LYD_OPT_NOTIF

First issue is that the validation reports no errors, even though the must condition is not satisfied:

leaf time-of-change {
  type uint32;
  must '. > /rd:magic_number' {
     error-message "time-of-change must be greater than magic_number";
  }
}

Second issue is that the data tree with the notification gets stripped of the name node in lyd_validate:

<kernel-modules xmlns="urn:ietf:params:xml:ns:yang:test-module">
  <kernel-module>
    <status-change>
      <loaded>true</loaded>
      <time-of-change>56</time-of-change>
    </status-change>
  </kernel-module>
</kernel-modules>

I need to access this data tree after the validation in order to read the newly added default values (none in this case). If I can safely assume that the pre-validation pointer to status-change hasn't changed during the validation, then this is not an issue for me, otherwise I need to get to the root of the notification using xpath and in that case the presence of the name node is vital.

[milanlenco]

ok, nevermind, that was my mistake. I don't know why I did that, but I shouldn't have obtain pointer for the event notification node, but instead pass the entire data tree with the notification as the first argument to lyd_validate. if only it occurred to me before writing such a long comment :)