Memory error using ly_ctx_destroy and specific YANG models with extensions and identities

[jeremie6wind]

Hi,
happy new year!
I experienced some crashes involving ly_ctx_destroy and specific YANG models.
I have two models containing identities that use the same extension.
It often does not crash, but Valgrind can detect errors.

The example yang models:

module myextension {
	yang-version 1.1;
	namespace "urn:myextension";
	prefix myext;

	extension my-description-extension {
		description
			"Description.";
	}
}

module example {
	yang-version 1.1;
	namespace "urn:example";
	prefix e;
	import myextension {
		prefix myext;
	}

	identity myidentity1 {
		description
			"Descr.";

		myext:my-description-extension;
	}
}

module example2 {
	yang-version 1.1;
	namespace "urn:example2";
	prefix e2;
	import myextension {
		prefix myext;
	}

	identity myidentity3 {
		description
			"Descr.";

		myext:my-description-extension;
	}
}

Step to repoduce:

sysrepoctl -i myextension.yang -v3 &&
sysrepoctl -i example.yang -v3 &&
sysrepoctl -i example2.yang -v3 &&
/usr/bin/valgrind --log-file=/tmp/example-valgrind.log /usr/local/sbin/netopeer2-server -v3 -d -t 600;

I let netopeer2-server start, and I stop it after a few seconds.

This is the output of valgrind file /tmp/example-valgrind.log :

==4307== Memcheck, a memory error detector
==4307== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==4307== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==4307== Command: /usr/local/sbin/netopeer2-server -v3 -d -t 600
==4307== Parent PID: 673
==4307== 
==4307== Invalid read of size 8
==4307==    at 0x4B49AD7: lysc_ext_instance_free (tree_schema_free.c:646)
==4307==    by 0x4B4A646: lysc_ident_free (tree_schema_free.c:808)
==4307==    by 0x4B4C6CC: lys_module_free (tree_schema_free.c:1324)
==4307==    by 0x4ACE17A: ly_ctx_destroy (context.c:1600)
==4307==    by 0x4C6B02F: sr_conn_free (sysrepo.c:171)
==4307==    by 0x4C6B978: sr_disconnect (sysrepo.c:409)
==4307==    by 0x112C34: server_destroy (main.c:950)
==4307==    by 0x11491E: main (main.c:1579)
==4307==  Address 0x74feb20 is 32 bytes inside a block of size 56 free'd
==4307==    at 0x484988F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4307==    by 0x4B4C7B1: lys_module_free (tree_schema_free.c:1330)
==4307==    by 0x4ACE17A: ly_ctx_destroy (context.c:1600)
==4307==    by 0x4C6B02F: sr_conn_free (sysrepo.c:171)
==4307==    by 0x4C6B978: sr_disconnect (sysrepo.c:409)
==4307==    by 0x112C34: server_destroy (main.c:950)
==4307==    by 0x11491E: main (main.c:1579)
==4307==  Block was alloc'd at
==4307==    at 0x484D953: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4307==    by 0x4B11687: lys_compile_extensions (schema_compile.c:1835)
==4307==    by 0x4B44FB6: lys_parse_in (tree_schema.c:2184)
==4307==    by 0x4B51222: lys_parse_localfile (tree_schema_common.c:740)
==4307==    by 0x4B51536: lys_load_mod_from_clb_or_file (tree_schema_common.c:824)
==4307==    by 0x4B51911: lys_parse_load (tree_schema_common.c:932)
==4307==    by 0x4AC97F0: ly_ctx_load_module (context.c:243)
==4307==    by 0x4C938E0: sr_ly_ctx_load_module (ly_wrap.c:265)
==4307==    by 0x4CC8609: sr_shmmod_ctx_load_modules (shm_mod.c:1126)
==4307==    by 0x4CB7C8A: sr_ly_ctx_init (context_change.c:77)
==4307==    by 0x4CB801E: sr_lycc_lock (context_change.c:176)
==4307==    by 0x4C6B575: sr_connect (sysrepo.c:300)
==4307== 
==4307== 
==4307== HEAP SUMMARY:
==4307==     in use at exit: 0 bytes in 0 blocks
==4307==   total heap usage: 91,370 allocs, 91,370 frees, 29,546,026 bytes allocated
==4307== 
==4307== All heap blocks were freed -- no leaks are possible
==4307== 
==4307== For lists of detected and suppressed errors, rerun with: -s
==4307== ERROR SUMMARY: 2 errors from 1 contexts (suppressed: 0 from 0)

thank you.

[NLako-6WIND]

Checking with gdb showed that the issues appears when trying to de-reference the (lysc_ext_instance) ext->def that points to the location of the module->extensions[] of a previously freed plugin:

Thread 1 "netopeer2-serve" hit Breakpoint 1, lys_module_free (ctx=0x5b8f37ad08e0, module=0x5b8f37ba56f0,
remove_links=0 '\000') at /root/sshfs_root/libyang/src/tree_schema_free.c:1310
1310 return;
1: module->name = 0x5b8f37bf7e50 "myextension"
(gdb) p module->name
$1 = 0x5b8f37bf7e50 "myextension"
(gdb) p module->extensions->name
$2 = 0x5b8f37ba55b0 "my-description-extension"
(gdb) p module->extensions
$3 = (struct lysc_ext *) 0x5b8f37bf7e78 <<<< Address of the first extension in the extensions array
(gdb) c
Continuing.

Thread 1 "netopeer2-serve" hit Breakpoint 1, lys_module_free (ctx=0x5b8f37ad08e0, module=0x5b8f37bf6ff0,
remove_links=0 '\000') at /root/sshfs_root/libyang/src/tree_schema_free.c:1310
1310 return;
1: module->name = 0x5b8f37bf7d50 "example"
(gdb) p module->name
$4 = 0x5b8f37bf7d50 "example"
(gdb) p module->identities->exts->def
$5 = (struct lysc_ext *) 0x5b8f37bf7e78 <<<< Same address that was freed previously
(gdb)

(update) Removing the FREE_ARRAY(ctx, ident->exts, lysc_ext_instance_free); from lysc_ident_free() resolved the issues and surprisingly didn't create new leaks, but I'm unsure if this will not cause regressions elsewhere.

[michalvasko]

Luckily, the design already accounted for this use-case, it was just not covered. So after a simple fix it should work.