[OpenWrt-Devel] [PATCH] uci: do not access invalid memory when updating an existing section
Enrico Mioso
mrkiko.rs at gmail.com
Tue Aug 14 01:42:18 EDT 2018
Hello!
First of all, thank you very very much for your patience and review.
You are right, my patch introduces an unintended change in behaviour, and actually does not solve the problem.
That day I didn't see this.
However, I think it is still useful to report what happens here, even just for the sole purpose of understanding it, and learning something for me. :)
[mrkiko at gatosaldo build]$ # both system installed uci ("uci"), and the modified one, have been built now. Some infos:
[mrkiko at gatosaldo build]$ cat /etc/config/normalcfg
config normal_section ns
option a 1
config normal_section ns2
option b 2
[mrkiko at gatosaldo build]$ cat /etc/config/exmcfg
config example_section "secttest"
option a 1
option b 2
option c 4
config example_section "secttest"
option d 4
option e 5
[mrkiko at gatosaldo build]$ cat /tmp/r/valgrind.sh
#!/bin/sh
valgrind --suppressions=/tmp/valgrind.suppressions --leak-check=full --leak-resolution=high --show-reachable=no --log-file=valgrind.out $@
[mrkiko at gatosaldo build]$ # system installed uci
[mrkiko at gatosaldo build]$ uci show normalcfg
normalcfg.ns=normal_section
normalcfg.ns.a='1'
normalcfg.ns2=normal_section
normalcfg.ns2.b='2'
[mrkiko at gatosaldo build]$ /tmp/r/valgrind.sh uci show normalcfg
normalcfg.ns=normal_section
normalcfg.ns.a='1'
normalcfg.ns2=normal_section
normalcfg.ns2.b='2'
[mrkiko at gatosaldo build]$ cat valgrind.out
==12860== Memcheck, a memory error detector
==12860== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==12860== Using Valgrind-3.14.0.GIT and LibVEX; rerun with -h for copyright info
==12860== Command: uci show normalcfg
==12860== Parent PID: 12859
==12860==
==12860==
==12860== HEAP SUMMARY:
==12860== in use at exit: 0 bytes in 0 blocks
==12860== total heap usage: 24 allocs, 24 frees, 6,315 bytes allocated
==12860==
==12860== All heap blocks were freed -- no leaks are possible
==12860==
==12860== For counts of detected and suppressed errors, rerun with: -v
==12860== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
[mrkiko at gatosaldo build]$ uci show exmcfg
exmcfg.secttest=example_section
exmcfg.secttest.a='1'
exmcfg.secttest.b='2'
exmcfg.secttest.c='4'
exmcfg.secttest.d='4'
exmcfg.secttest.e='5'
[mrkiko at gatosaldo build]$ /tmp/r/valgrind.sh uci show exmcfg
/tmp/r/valgrind.sh: line 2: 12864 Segmentation fault (core dumped) valgrind --suppressions=/tmp/valgrind.suppressions --leak-check=full --leak-resolution=high --show-reachable=no --log-file=valgrind.out $@
[mrkiko at gatosaldo build]$ cat valgrind.out
==12864== Memcheck, a memory error detector
==12864== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==12864== Using Valgrind-3.14.0.GIT and LibVEX; rerun with -h for copyright info
==12864== Command: uci show exmcfg
==12864== Parent PID: 12863
==12864==
==12864== Invalid read of size 4
==12864== at 0x486E601: uci_lookup_list (list.c:280)
==12864== by 0x4870A0D: uci_parse_option (file.c:478)
==12864== by 0x4871247: uci_parse_line (file.c:530)
==12864== by 0x4871247: uci_import (file.c:680)
==12864== by 0x487195B: uci_file_load (file.c:910)
==12864== by 0x486F10C: uci_load (libuci.c:216)
==12864== by 0x486F240: uci_lookup_ptr (list.c:394)
==12864== by 0x109787: package_cmd (cli.c:312)
==12864== by 0x10A1EF: uci_do_package_cmd (cli.c:422)
==12864== by 0x10A1EF: uci_cmd (cli.c:674)
==12864== by 0x1092CF: main (cli.c:767)
==12864== Address 0x4a6759c is 28 bytes inside a block of size 52 free'd
==12864== at 0x4837471: realloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==12864== by 0x4871A7C: uci_realloc (util.c:49)
==12864== by 0x486FDA5: uci_set (list.c:717)
==12864== by 0x487118D: uci_parse_config (file.c:448)
==12864== by 0x487118D: uci_parse_line (file.c:518)
==12864== by 0x487118D: uci_import (file.c:680)
==12864== by 0x487195B: uci_file_load (file.c:910)
==12864== by 0x486F10C: uci_load (libuci.c:216)
==12864== by 0x486F240: uci_lookup_ptr (list.c:394)
==12864== by 0x109787: package_cmd (cli.c:312)
==12864== by 0x10A1EF: uci_do_package_cmd (cli.c:422)
==12864== by 0x10A1EF: uci_cmd (cli.c:674)
==12864== by 0x1092CF: main (cli.c:767)
==12864== Block was alloc'd at
==12864== at 0x4835558: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==12864== by 0x4871A31: uci_malloc (util.c:39)
==12864== by 0x486E1B2: uci_alloc_generic (list.c:50)
==12864== by 0x486E2EE: uci_alloc_section (list.c:194)
==12864== by 0x486FCF2: uci_set (list.c:699)
==12864== by 0x487118D: uci_parse_config (file.c:448)
==12864== by 0x487118D: uci_parse_line (file.c:518)
==12864== by 0x487118D: uci_import (file.c:680)
==12864== by 0x487195B: uci_file_load (file.c:910)
==12864== by 0x486F10C: uci_load (libuci.c:216)
==12864== by 0x486F240: uci_lookup_ptr (list.c:394)
==12864== by 0x109787: package_cmd (cli.c:312)
==12864== by 0x10A1EF: uci_do_package_cmd (cli.c:422)
==12864== by 0x10A1EF: uci_cmd (cli.c:674)
==12864== by 0x1092CF: main (cli.c:767)
==12864==
==12864== Invalid read of size 1
==12864== at 0x4838A15: strcmp (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==12864== by 0x486E609: uci_lookup_list (list.c:280)
==12864== by 0x4870A0D: uci_parse_option (file.c:478)
==12864== by 0x4871247: uci_parse_line (file.c:530)
==12864== by 0x4871247: uci_import (file.c:680)
==12864== by 0x487195B: uci_file_load (file.c:910)
==12864== by 0x486F10C: uci_load (libuci.c:216)
==12864== by 0x486F240: uci_lookup_ptr (list.c:394)
==12864== by 0x109787: package_cmd (cli.c:312)
==12864== by 0x10A1EF: uci_do_package_cmd (cli.c:422)
==12864== by 0x10A1EF: uci_cmd (cli.c:674)
==12864== by 0x1092CF: main (cli.c:767)
==12864== Address 0x0 is not stack'd, malloc'd or (recently) free'd
==12864==
==12864==
==12864== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==12864== Access not within mapped region at address 0x0
==12864== at 0x4838A15: strcmp (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==12864== by 0x486E609: uci_lookup_list (list.c:280)
==12864== by 0x4870A0D: uci_parse_option (file.c:478)
==12864== by 0x4871247: uci_parse_line (file.c:530)
==12864== by 0x4871247: uci_import (file.c:680)
==12864== by 0x487195B: uci_file_load (file.c:910)
==12864== by 0x486F10C: uci_load (libuci.c:216)
==12864== by 0x486F240: uci_lookup_ptr (list.c:394)
==12864== by 0x109787: package_cmd (cli.c:312)
==12864== by 0x10A1EF: uci_do_package_cmd (cli.c:422)
==12864== by 0x10A1EF: uci_cmd (cli.c:674)
==12864== by 0x1092CF: main (cli.c:767)
==12864== If you believe this happened as a result of a stack
==12864== overflow in your program's main thread (unlikely but
==12864== possible), you can try to increase the size of the
==12864== main thread stack using the --main-stacksize= flag.
==12864== The main thread stack size used in this run was 8388608.
==12864==
==12864== HEAP SUMMARY:
==12864== in use at exit: 957 bytes in 18 blocks
==12864== total heap usage: 21 allocs, 3 frees, 5,137 bytes allocated
==12864==
==12864== LEAK SUMMARY:
==12864== definitely lost: 0 bytes in 0 blocks
==12864== indirectly lost: 0 bytes in 0 blocks
==12864== possibly lost: 0 bytes in 0 blocks
==12864== still reachable: 957 bytes in 18 blocks
==12864== suppressed: 0 bytes in 0 blocks
==12864== Reachable blocks (those to which a pointer was found) are not shown.
==12864== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==12864==
==12864== For counts of detected and suppressed errors, rerun with: -v
==12864== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
[mrkiko at gatosaldo build]$ # patched uci
[mrkiko at gatosaldo build]$ ./uci show normalcfg
normalcfg.ns=normal_section
normalcfg.ns.a='1'
normalcfg.ns2=normal_section
normalcfg.ns2.b='2'
[mrkiko at gatosaldo build]$ /tmp/r/valgrind.sh ./uci show normalcfg
normalcfg.ns=normal_section
normalcfg.ns.a='1'
normalcfg.ns2=normal_section
normalcfg.ns2.b='2'
[mrkiko at gatosaldo build]$ ./uci show exmcfg # **** unintended behaviour change ****
exmcfg.secttest=example_section
exmcfg.secttest.d='4'
exmcfg.secttest.e='5'
[mrkiko at gatosaldo build]$ /tmp/r/valgrind.sh ./uci show exmcfg
exmcfg.secttest=example_section
exmcfg.secttest.d='4'
exmcfg.secttest.e='5'
[mrkiko at gatosaldo build]$ cat valgrind.out
==12878== Memcheck, a memory error detector
==12878== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==12878== Using Valgrind-3.14.0.GIT and LibVEX; rerun with -h for copyright info
==12878== Command: ./uci show exmcfg
==12878== Parent PID: 12877
==12878==
==12878==
==12878== HEAP SUMMARY:
==12878== in use at exit: 108 bytes in 6 blocks
==12878== total heap usage: 31 allocs, 25 frees, 6,479 bytes allocated
==12878==
==12878== 108 (34 direct, 74 indirect) bytes in 1 blocks are definitely lost in loss record 3 of 3
==12878== at 0x4835558: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==12878== by 0x4846E69: uci_malloc (util.c:39)
==12878== by 0x4843621: uci_alloc_generic (list.c:50)
==12878== by 0x4843845: uci_alloc_option (list.c:85)
==12878== by 0x484512A: uci_set (list.c:696)
==12878== by 0x4845E85: uci_parse_option (file.c:488)
==12878== by 0x484667C: uci_parse_line (file.c:530)
==12878== by 0x484667C: uci_import (file.c:680)
==12878== by 0x4846D95: uci_file_load (file.c:910)
==12878== by 0x4844579: uci_load (libuci.c:216)
==12878== by 0x48446A7: uci_lookup_ptr (list.c:394)
==12878== by 0x109A48: package_cmd (cli.c:312)
==12878== by 0x10A475: uci_do_package_cmd (cli.c:422)
==12878== by 0x10A475: uci_cmd (cli.c:674)
==12878==
==12878== LEAK SUMMARY:
==12878== definitely lost: 34 bytes in 1 blocks
==12878== indirectly lost: 74 bytes in 5 blocks
==12878== possibly lost: 0 bytes in 0 blocks
==12878== still reachable: 0 bytes in 0 blocks
==12878== suppressed: 0 bytes in 0 blocks
==12878==
==12878== For counts of detected and suppressed errors, rerun with: -v
==12878== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
[mrkiko at gatosaldo build]$ exit
I guess I introduced this leak.
Some problems where still detectable with Valgrind 3.13. but I don't exactly remember.
This is a glibc-based system.
Thanks again for all the help,
Enrico
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
More information about the openwrt-devel
mailing list