[FS#1321] UCI potential invalid memory access when updating existing section
LEDE Bugs
lede-bugs at lists.infradead.org
Fri Feb 2 01:08:56 PST 2018
A new Flyspray task has been opened. Details are below.
User who did this - Mond WAN (mondwan)
Attached to Project - OpenWrt/LEDE Project
Summary - UCI potential invalid memory access when updating existing section
Task Type - Bug Report
Category - Packages
Status - Unconfirmed
Assigned To -
Operating System - All
Severity - Low
Priority - Very Low
Reported Version - Trunk
Due in Version - Undecided
Due Date - Undecided
Details - - Software versions
[[
https://git.openwrt.org/?p=project/uci.git;a=tree;h=5beb95da3dbec6db11a6bdfaab7807ee2daf41e6;hb=5beb95da3dbec6db11a6bdfaab7807ee2daf41e6|UCI 2018-01-01]]
- Device problem occurs on
There is a potential memory leak when updating existing section in [[
https://git.openwrt.org/?p=project/uci.git;a=blob;f=list.c;h=0347138cbbc19bc9a5ad5c6231ee528f2492aab0;hb=5beb95da3dbec6db11a6bdfaab7807ee2daf41e6#l709|list.c:709]]
Return pointer from realloc may not be the same as ptr->s. Due to realloc mechanism, pointers value from ptr->s->options are copied to the ptr->last. However, those pointers (ptr->last->s->options) are pointing back to the ptr->s which has been freed.
Below are steps to reproduce.
- Steps to reproduce
Given a config file like that
cat /etc/config/network
config interface wan
Given test codes like that
#include
int main(int argc, const char *argv[])
{
struct uci_context *uci_ctx = uci_alloc_context();
struct uci_ptr ptr = {0};
ptr.package = "network";
ptr.section = "wan";
ptr.value = "interface";
uci_lookup_ptr(uci_ctx, &ptr, NULL, false);
uci_set(uci_ctx, &ptr);
uci_save(uci_ctx, ptr.p);
uci_commit(uci_ctx, &ptr.p, false);
uci_free_context(uci_ctx);
return 0;
}
Runs like that
cd UCI_PROJECT_ROOT
mkdir build
cmake ..
make
valgrind ./test/test_mem_leak_section
.... CTRL-C after a while ....
Here is the output of valgrind before the hotfix below
==1567== Memcheck, a memory error detector
==1567== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==1567== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==1567== Command: ./test/test_mem_leak_section
==1567==
==1567== Invalid write of size 8
==1567== at 0x4E3946B: uci_list_del (uci_internal.h:116)
==1567== by 0x4E3946B: uci_free_element (list.c:71)
==1567== by 0x4E394F4: uci_free_section (list.c:211)
==1567== by 0x4E39590: uci_free_package (list.c:243)
==1567== by 0x4E39C97: uci_free_context (libuci.c:84)
==1567== by 0x400719: main (test_mem_leak_section.c:12)
==1567== Address 0x561a850 is 32 bytes inside a block of size 82 free'd
==1567== at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1567== by 0x4E3C4FE: uci_realloc (util.c:49)
==1567== by 0x4E3AC77: uci_set (list.c:709)
==1567== by 0x400711: main (test_mem_leak_section.c:11)
==1567==
==1567== Invalid write of size 8
==1567== at 0x4E3946E: uci_list_del (uci_internal.h:117)
==1567== by 0x4E3946E: uci_free_element (list.c:71)
==1567== by 0x4E394F4: uci_free_section (list.c:211)
==1567== by 0x4E39590: uci_free_package (list.c:243)
==1567== by 0x4E39C97: uci_free_context (libuci.c:84)
==1567== by 0x400719: main (test_mem_leak_section.c:12)
==1567== Address 0x561a858 is 40 bytes inside a block of size 82 free'd
==1567== at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1567== by 0x4E3C4FE: uci_realloc (util.c:49)
==1567== by 0x4E3AC77: uci_set (list.c:709)
==1567== by 0x400711: main (test_mem_leak_section.c:11)
==1567==
==1567== Invalid read of size 8
==1567== at 0x4E394F8: uci_free_section (list.c:210)
==1567== by 0x4E39590: uci_free_package (list.c:243)
==1567== by 0x4E39C97: uci_free_context (libuci.c:84)
==1567== by 0x400719: main (test_mem_leak_section.c:12)
==1567== Address 0x561a850 is 32 bytes inside a block of size 82 free'd
==1567== at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1567== by 0x4E3C4FE: uci_realloc (util.c:49)
==1567== by 0x4E3AC77: uci_set (list.c:709)
==1567== by 0x400711: main (test_mem_leak_section.c:11)
==1567==
==1567== Invalid read of size 4
==1567== at 0x4E39486: uci_free_option (list.c:97)
==1567== by 0x4E394F4: uci_free_section (list.c:211)
==1567== by 0x4E39590: uci_free_package (list.c:243)
==1567== by 0x4E39C97: uci_free_context (libuci.c:84)
==1567== by 0x400719: main (test_mem_leak_section.c:12)
==1567== Address 0x561a878 is 72 bytes inside a block of size 82 free'd
==1567== at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1567== by 0x4E3C4FE: uci_realloc (util.c:49)
==1567== by 0x4E3AC77: uci_set (list.c:709)
==1567== by 0x400711: main (test_mem_leak_section.c:11)
==1567==
==1567== Invalid read of size 8
==1567== at 0x4E39456: uci_free_element (list.c:69)
==1567== by 0x4E394F4: uci_free_section (list.c:211)
==1567== by 0x4E39590: uci_free_package (list.c:243)
==1567== by 0x4E39C97: uci_free_context (libuci.c:84)
==1567== by 0x400719: main (test_mem_leak_section.c:12)
==1567== Address 0x561a868 is 56 bytes inside a block of size 82 free'd
==1567== at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1567== by 0x4E3C4FE: uci_realloc (util.c:49)
==1567== by 0x4E3AC77: uci_set (list.c:709)
==1567== by 0x400711: main (test_mem_leak_section.c:11)
==1567==
==1567== Invalid read of size 8
==1567== at 0x4E3945F: uci_free_element (list.c:70)
==1567== by 0x4E394F4: uci_free_section (list.c:211)
==1567== by 0x4E39590: uci_free_package (list.c:243)
==1567== by 0x4E39C97: uci_free_context (libuci.c:84)
==1567== by 0x400719: main (test_mem_leak_section.c:12)
==1567== Address 0x561a850 is 32 bytes inside a block of size 82 free'd
==1567== at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1567== by 0x4E3C4FE: uci_realloc (util.c:49)
==1567== by 0x4E3AC77: uci_set (list.c:709)
==1567== by 0x400711: main (test_mem_leak_section.c:11)
==1567==
==1567== Invalid free() / delete / delete[] / realloc()
==1567== at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1567== by 0x4E394F4: uci_free_section (list.c:211)
==1567== by 0x4E39590: uci_free_package (list.c:243)
==1567== by 0x4E39C97: uci_free_context (libuci.c:84)
==1567== by 0x400719: main (test_mem_leak_section.c:12)
==1567== Address 0x561a850 is 32 bytes inside a block of size 82 free'd
==1567== at 0x4C2CE8E: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1567== by 0x4E3C4FE: uci_realloc (util.c:49)
==1567== by 0x4E3AC77: uci_set (list.c:709)
==1567== by 0x400711: main (test_mem_leak_section.c:11)
==1567==
^C
==1567==
==1567== HEAP SUMMARY:
==1567== in use at exit: 973 bytes in 17 blocks
==1567== total heap usage: 189 allocs, 408,927 frees, 8,483 bytes allocated
==1567==
==1567== LEAK SUMMARY:
==1567== definitely lost: 0 bytes in 0 blocks
==1567== indirectly lost: 0 bytes in 0 blocks
==1567== possibly lost: 0 bytes in 0 blocks
==1567== still reachable: 973 bytes in 17 blocks
==1567== suppressed: 0 bytes in 0 blocks
==1567== Rerun with --leak-check=full to see details of leaked memory
==1567==
==1567== For counts of detected and suppressed errors, rerun with: -v
==1567== ERROR SUMMARY: 2043783 errors from 7 contexts (suppressed: 0 from 0)
- Hot fix
- ptr->last = NULL;
- ptr->last = uci_realloc(ctx, ptr->s, sizeof(struct uci_section));
- ptr->s = uci_to_section(ptr->last);
- uci_list_fixup(&ptr->s->e.list);
+ ptr->last = (struct uci_element *) ptr->s;
Please take a look the attachment. It includes my hot fix for this issue and corresponding demo codes as illustrated above.
I am not sure how to "update" the "uci_options" associated to the "uci_section". So, I simply omit and replace the realloc part.
Tested by valgrind again
valgrind ./test/test_mem_leak_section
==1749== Memcheck, a memory error detector
==1749== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==1749== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==1749== Command: ./test/test_mem_leak_section
==1749==
==1749==
==1749== HEAP SUMMARY:
==1749== in use at exit: 0 bytes in 0 blocks
==1749== total heap usage: 392 allocs, 392 frees, 23,574 bytes allocated
==1749==
==1749== All heap blocks were freed -- no leaks are possible
==1749==
==1749== For counts of detected and suppressed errors, rerun with: -v
==1749== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
One or more files have been attached.
More information can be found at the following URL:
https://bugs.lede-project.org/index.php?do=details&task_id=1321
More information about the lede-bugs
mailing list