Potential bug in forward nodes references (this time with no awful horizontal wrapping...)

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hello,
This is my first contact and mail with this mailing list, to summarize i'm a coder maintening a personnal home server using several GNU tools since the late 90's.
I build LIBMSI-0.4.8 in a chroot jail (gcc 4.5.2/libc 2.13/binutils 2.21/make 3.82) with an athlon architecture on ext3 FS and, as root,got one failed test on tests/parser :
[root@pompomgalli] mkdir libmsi-0.4.8_build && cd libmsi-0.4.8_build
[root@pompomgalli] ../libsmi-0.4.8/configure --prefix=/usr
checking for a BSD-compatible install... /bin/install -c checking whether build environment is sane... yes ....
[root@pompomgalli] make
.... make[2]: Leaving directory `/usr/src_app/libsmi-0.4.8_build' make[1]: Leaving directory `/usr/src_app/libsmi-0.4.8_build'
[root@pompomgalli] make check
... PASS: smidump-orig-smiv2.test comparing `smidump -f smiv2 ../dumps/smiv2/SNMPv2-MIB' output with dumps/smiv2-smiv2/*. comparing `smidump -f smiv2 ../dumps/smiv2/IF-MIB' output with dumps/smiv2-smiv2/*. comparing `smidump -f smiv2 ../dumps/smiv2/MAU-MIB' output with dumps/smiv2-smiv2/*. comparing `smidump -f smiv2 ../dumps/smiv2/RMON2-MIB' output with dumps/smiv2-smiv2/*. PASS: smidump-smiv2-smiv2.test Checking LIBSMI-TEST-001-MIB. 0 errors/warnings, ok. Checking LIBSMI-TEST-002-MIB. 1 errors/warnings, ok. Checking LIBSMI-TEST-003-MIB. unexpected output. see parser.out directory. Checking LIBSMI-TEST-004-MIB. 1 errors/warnings, ok. Checking LIBSMI-TEST-005-MIB. unexpected output. see parser.out directory. Checking LIBSMI-TEST-006-MIB. 1 errors/warnings, ok. Checking LIBSMI-TEST-007-MIB. unexpected output. see parser.out directory. Checking LIBSMI-TEST-008-MIB. unexpected output. see parser.out directory. Checking LIBSMI-TEST-009-MIB. unexpected output. see parser.out directory. Checking LIBSMI-TEST-010-MIB. unexpected output. see parser.out directory. Checking LIBSMI-TEST-011-MIB. unexpected output. see parser.out directory. Checking LIBSMI-TEST-012-MIB. unexpected output. see parser.out directory. Checking LIBSMI-TEST-013-MIB. unexpected output. see parser.out directory. Checking LIBSMI-TEST-014-MIB. unexpected output. see parser.out directory. Checking LIBSMI-TEST-015-MIB. 0 errors/warnings, ok. Checking LIBSMI-TEST-016-MIB. unexpected output. see parser.out directory. Checking LIBSMI-TEST-017-MIB. unexpected output. see parser.out directory. Checking LIBSMI-TEST-018-MIB. unexpected output. see parser.out directory. FAIL: parser.test checking smidiff results for *. /bin/diff: *.diff: No such file or directory PASS: smidiff.test ==================== 1 of 21 tests failed ==================== make[3]: *** [check-TESTS] Error 1 make[3]: Leaving directory `/usr/src_app/libsmi-0.4.8_build/test' make[2]: *** [check-am] Error 2 make[2]: Leaving directory `/usr/src_app/libsmi-0.4.8_build/test' make[1]: *** [check-recursive] Error 1 make[1]: Leaving directory `/usr/src_app/libsmi-0.4.8_build/test' make: *** [check-recursive] Error 1
[root@pompomgalli]
Therefore, i check one of the failed output :
[root@pompomgalli] cat test/parser.out/LIBSMI-TEST-003-MIB.err
[root@pompomgalli] cat test/parser.out/LIBSMI-TEST-003-MIB.expect
LIBSMI-TEST-003-MIB:45: more than one MODULE-IDENTITY clause in SMIv2 MIB LIBSMI-TEST-003-MIB:45: MODULE-IDENTITY clause must be the first declaration in a module LIBSMI-TEST-003-MIB:53: revision not in reverse chronological order
I then launch manually the LIBSMI-TEST-003-MIB test :
[root@pompomgalli] SMIPATH=$SMIPATH:../libsmi-0.4.8/mibs/ietf:../libsmi-0.4.8/mibs:../libsmi-0.4.8/test/mibs tools/smilint -c/dev/null -l9 ../libsmi-0.4.8/test/mibs/LIBSMI-TEST-003-MIB 2>&1
Segmentation fault (core dumped)
[root@pompomgalli]
I have 'straced' the test but found nothing noticeable. As i am too lazy to launch gdb, i have first googling about related troubles regarding libsmi and found Vincent Bernat mail (http://www.ibr.cs.tu-bs.de/pipermail/libsmi/2010-August/001214.html).
So, starting from his premises, i looked after the handling of module's nodes trees, and, to resume, seen that Vincent Bernat seems to be right when commenting the freeNodeTree and smiFree calls in loadModule (data.c). The nodes allocated for the module could have been referenced by the parser and used later in the parse process (case 344 in parser-smi.c), therefore they should not be freed at the end of the loadModule call but at the end of the parse process (imho).
While i was investigate, i have also noticed that the checkObjects method in parser-smi.c does not handle the case of a malformed tree where a node could not have been linked to the root tree while searching possibly incorrect oidlen/oid values.
I suggest theses two patchs to address that :
--- ../libsmi-0.4.8/lib/parser-smi.c.orig 2008-04-18 14:58:11.000000000 +0200 +++ ../libsmi-0.4.8/lib/parser-smi.c 2011-05-08 16:33:24.000000000 +0200 @@ -954,6 +954,7 @@ nodePtr->parentPtr != thisParserPtr->pendingNodePtr && nodePtr->parentPtr != smiHandle->rootNodePtr && nodePtr != nodePtr->parentPtr && + nodePtr->parentPtr != NULL && i <= 128; nodePtr = nodePtr->parentPtr, i++); if ((objectPtr->export.name) &&
--- ../libsmi-0.4.8/lib/data.c.orig 2008-04-18 12:42:50.000000000 +0200 +++ ../libsmi-0.4.8/lib/data.c 2011-05-08 16:24:21.000000000 +0200 @@ -4737,8 +4737,9 @@ smiDepth++; parser.line = 1; smiparse((void *)&parser); - freeNodeTree(parser.pendingNodePtr); - smiFree(parser.pendingNodePtr); + // Nodes must NOT be freed here because forward references could be reached by imported modules ref + // freeNodeTree(parser.pendingNodePtr); + // smiFree(parser.pendingNodePtr); smiLeaveLexRecursion(); smiDepth--; fclose(parser.file); @@ -4780,6 +4781,9 @@ smiDepth++; parser.line = 1; smingparse((void *)&parser); + // Nodes must NOT be freed here because forward references could be reached by imported modules ref + // freeNodeTree(parser.pendingNodePtr); + // smiFree(parser.pendingNodePtr); freeNodeTree(parser.pendingNodePtr); smiFree(parser.pendingNodePtr); smingLeaveLexRecursion();
For parser-smi.c, it seems that quitting the loop when the node's parent is null is right regarding the goal to control that the tree does not exceed 128 nodes. Perhaps it could be useful to add an error message when this loop detects an 'unlinked' node.
With theses patchs, the parser test gives the following results :
[root@pompomgalli] make check
...
PASS: smidump-smiv2-smiv2.test Checking LIBSMI-TEST-001-MIB. 0 errors/warnings, ok. Checking LIBSMI-TEST-002-MIB. 1 errors/warnings, ok. Checking LIBSMI-TEST-003-MIB. 3 errors/warnings, ok. Checking LIBSMI-TEST-004-MIB. 1 errors/warnings, ok. Checking LIBSMI-TEST-005-MIB. 1 errors/warnings, ok. Checking LIBSMI-TEST-006-MIB. 1 errors/warnings, ok. Checking LIBSMI-TEST-007-MIB. 1 errors/warnings, ok. Checking LIBSMI-TEST-008-MIB. 21 errors/warnings, ok. Checking LIBSMI-TEST-009-MIB. 23 errors/warnings, ok. Checking LIBSMI-TEST-010-MIB. 10 errors/warnings, ok. Checking LIBSMI-TEST-011-MIB. 3 errors/warnings, ok. Checking LIBSMI-TEST-012-MIB. 5 errors/warnings, ok. Checking LIBSMI-TEST-013-MIB. 14 errors/warnings, ok. Checking LIBSMI-TEST-014-MIB. unexpected output. see parser.out directory. Checking LIBSMI-TEST-015-MIB. 0 errors/warnings, ok. Checking LIBSMI-TEST-016-MIB. 10 errors/warnings, ok. Checking LIBSMI-TEST-017-MIB. 12 errors/warnings, ok. Checking LIBSMI-TEST-018-MIB. 1 errors/warnings, ok. FAIL: parser.test checking smidiff results for *. /bin/diff: *.diff: No such file or directory PASS: smidiff.test ==================== 1 of 21 tests failed ==================== make[3]: *** [check-TESTS] Error 1 make[3]: Leaving directory `/usr/src_app/libsmi-0.4.8_build/test' make[2]: *** [check-am] Error 2 make[2]: Leaving directory `/usr/src_app/libsmi-0.4.8_build/test' make[1]: *** [check-recursive] Error 1 make[1]: Leaving directory `/usr/src_app/libsmi-0.4.8_build/test' make: *** [check-recursive] Error 1
The LIBSMI-TEST-014-MIB is still failing with :
[root@pompomgalli] diff -u test/parser.out/LIBSMI-TEST-014-MIB.err test/parser.out/LIBSMI-TEST-014-MIB.expect
--- test/parser.out/LIBSMI-TEST-014-MIB.err 2011-05-08 17:50:51.000000000 +0200 +++ test/parser.out/LIBSMI-TEST-014-MIB.expect 2011-05-08 17:50:51.000000000 +0200 @@ -1,5 +1,7 @@ +LIBSMI-TEST-014-MIB:102: warning: index of row `barEntry' can exceed OID size limit by 1 subidentifier(s) +LIBSMI-TEST-014-MIB:186: warning: index of row `barImpliedEntry' can exceed OID size limit by 1 subidentifier(s) LIBSMI-TEST-014-MIB:251: index element `fooStringName' of row `fooStringEntry' must have a size restriction -LIBSMI-TEST-014-MIB:230: warning: index of row `fooStringEntry' can exceed OID size limit by 65421 subidentifier(s) +LIBSMI-TEST-014-MIB:230: warning: index of row `fooStringEntry' can exceed OID size limit by 65427 subidentifier(s) LIBSMI-TEST-014-MIB:294: index element `fooOidName' of row `fooOidEntry' should but cannot have a size restriction -LIBSMI-TEST-014-MIB:273: warning: index of row `fooOidEntry' can exceed OID size limit by 14 subidentifier(s) +LIBSMI-TEST-014-MIB:273: warning: index of row `fooOidEntry' can exceed OID size limit by 20 subidentifier(s) LIBSMI-TEST-014-MIB:309: warning: current group `fooOidBarGroup' is not referenced in this module
Apparently, it is something else, i could investigate if it seems pertinent. I will maintain my chroot environment for few days, therefore i could make more tests if needed.
Regards, Cedric.

On Sun, May 08, 2011 at 06:38:46PM +0200, cedric arbogast wrote:
[...]
I suggest theses two patchs to address that:
[...]
I have commited the patches to trunk.
/js
participants (2)
-
cedric arbogast
-
Juergen Schoenwaelder