Re: [PATCH v2] ulogd2: Avoid use after free in unregister on global ulogd_fds linked list

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Florian
This is valgrind logs.

==4797== Memcheck, a memory error detector
==4797== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et
al.
==4797== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright
info
==4797== Command: ulogd -v -c /etc/ulogd.conf
==4797== Invalid read of size 4
==4797==    at 0x405F60: ulogd_unregister_fd (select.c:74)
==4797==    by 0x4E4E3DF: ??? (in /usr/lib/ulogd/ulogd_inppkt_NFLOG.so)
==4797==    by 0x405003: stop_pluginstances (ulogd.c:1335)
==4797==    by 0x405003: sigterm_handler_task (ulogd.c:1383)
==4797==    by 0x405153: call_signal_handler_tasks (ulogd.c:424)
==4797==    by 0x405153: signal_channel_callback (ulogd.c:443)
==4797==    by 0x406163: ulogd_select_main (select.c:105)
==4797==    by 0x403CF3: ulogd_main_loop (ulogd.c:1070)
==4797==    by 0x403CF3: main (ulogd.c:1649)
==4797==  Address 0x4a84f40 is 160 bytes inside a block of size 4,848
free'd
==4797==    at 0x4847551: free (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4797==    by 0x40500E: stop_pluginstances (ulogd.c:1338)
==4797==    by 0x40500E: sigterm_handler_task (ulogd.c:1383)
==4797==    by 0x405153: call_signal_handler_tasks (ulogd.c:424)
==4797==    by 0x405153: signal_channel_callback (ulogd.c:443)
==4797==    by 0x406163: ulogd_select_main (select.c:105)
==4797==    by 0x403CF3: ulogd_main_loop (ulogd.c:1070)
==4797==    by 0x403CF3: main (ulogd.c:1649)
==4797==  Block was alloc'd at
==4797==    at 0x4849D82: calloc (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4797==    by 0x405504: pluginstance_alloc_init (ulogd.c:664)
==4797==    by 0x405504: create_stack (ulogd.c:1014)
==4797==    by 0x406FCE: config_parse_file (conffile.c:225)
==4797==    by 0x403949: parse_conffile (ulogd.c:1107)
==4797==    by 0x403949: main (ulogd.c:1589)
==4797== 
==4797== Invalid read of size 8
==4797==    at 0x405F6E: ulogd_unregister_fd (select.c:73)
==4797==    by 0x4E4E3DF: ??? (in /usr/lib/ulogd/ulogd_inppkt_NFLOG.so)
==4797==    by 0x405003: stop_pluginstances (ulogd.c:1335)
==4797==    by 0x405003: sigterm_handler_task (ulogd.c:1383)
==4797==    by 0x405153: call_signal_handler_tasks (ulogd.c:424)
==4797==    by 0x405153: signal_channel_callback (ulogd.c:443)
==4797==    by 0x406163: ulogd_select_main (select.c:105)
==4797==    by 0x403CF3: ulogd_main_loop (ulogd.c:1070)
==4797==    by 0x403CF3: main (ulogd.c:1649)
==4797==  Address 0x4a84f30 is 144 bytes inside a block of size 4,848
free'd
==4797==    at 0x4847551: free (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4797==    by 0x40500E: stop_pluginstances (ulogd.c:1338)
==4797==    by 0x40500E: sigterm_handler_task (ulogd.c:1383)
==4797==    by 0x405153: call_signal_handler_tasks (ulogd.c:424)
==4797==    by 0x405153: signal_channel_callback (ulogd.c:443)
==4797==    by 0x406163: ulogd_select_main (select.c:105)
==4797==    by 0x403CF3: ulogd_main_loop (ulogd.c:1070)
==4797==    by 0x403CF3: main (ulogd.c:1649)
==4797==  Block was alloc'd at
==4797==    at 0x4849D82: calloc (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4797==    by 0x405504: pluginstance_alloc_init (ulogd.c:664)
==4797==    by 0x405504: create_stack (ulogd.c:1014)
==4797==    by 0x406FCE: config_parse_file (conffile.c:225)
==4797==    by 0x403949: parse_conffile (ulogd.c:1107)
==4797==    by 0x403949: main (ulogd.c:1589)
==4797== 
Tue Mar 14 11:18:20 2023 <1> ulogd.c:1333 calling stop for IFINDEX
==4797== Invalid write of size 8
==4797==    at 0x405F31: __llist_del (linuxlist.h:107)
==4797==    by 0x405F31: llist_del (linuxlist.h:119)
==4797==    by 0x405F31: ulogd_unregister_fd (select.c:69)
==4797==    by 0x4E6427F: ??? (in
/usr/lib/ulogd/ulogd_filter_IFINDEX.so)
==4797==    by 0x405003: stop_pluginstances (ulogd.c:1335)
==4797==    by 0x405003: sigterm_handler_task (ulogd.c:1383)
==4797==    by 0x405153: call_signal_handler_tasks (ulogd.c:424)
==4797==    by 0x405153: signal_channel_callback (ulogd.c:443)
==4797==    by 0x406163: ulogd_select_main (select.c:105)
==4797==    by 0x403CF3: ulogd_main_loop (ulogd.c:1070)
==4797==    by 0x403CF3: main (ulogd.c:1649)
==4797==  Address 0x4a84f38 is 152 bytes inside a block of size 4,848
free'd
==4797==    at 0x4847551: free (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4797==    by 0x40500E: stop_pluginstances (ulogd.c:1338)
==4797==    by 0x40500E: sigterm_handler_task (ulogd.c:1383)
==4797==    by 0x405153: call_signal_handler_tasks (ulogd.c:424)
==4797==    by 0x405153: signal_channel_callback (ulogd.c:443)
==4797==    by 0x406163: ulogd_select_main (select.c:105)
==4797==    by 0x403CF3: ulogd_main_loop (ulogd.c:1070)
==4797==    by 0x403CF3: main (ulogd.c:1649)
==4797==  Block was alloc'd at
==4797==    at 0x4849D82: calloc (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4797==    by 0x405504: pluginstance_alloc_init (ulogd.c:664)
==4797==    by 0x405504: create_stack (ulogd.c:1014)
==4797==    by 0x406FCE: config_parse_file (conffile.c:225)
==4797==    by 0x403949: parse_conffile (ulogd.c:1107)
==4797==    by 0x403949: main (ulogd.c:1589)
==4797== 
==4797== Invalid read of size 4
==4797==    at 0x405F60: ulogd_unregister_fd (select.c:74)
==4797==    by 0x4E6427F: ??? (in
/usr/lib/ulogd/ulogd_filter_IFINDEX.so)
==4797==    by 0x405003: stop_pluginstances (ulogd.c:1335)
==4797==    by 0x405003: sigterm_handler_task (ulogd.c:1383)
==4797==    by 0x405153: call_signal_handler_tasks (ulogd.c:424)
==4797==    by 0x405153: signal_channel_callback (ulogd.c:443)
==4797==    by 0x406163: ulogd_select_main (select.c:105)
==4797==    by 0x403CF3: ulogd_main_loop (ulogd.c:1070)
==4797==    by 0x403CF3: main (ulogd.c:1649)
==4797==  Address 0x4a84f40 is 160 bytes inside a block of size 4,848
free'd
==4797==    at 0x4847551: free (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4797==    by 0x40500E: stop_pluginstances (ulogd.c:1338)
==4797==    by 0x40500E: sigterm_handler_task (ulogd.c:1383)
==4797==    by 0x405153: call_signal_handler_tasks (ulogd.c:424)
==4797==    by 0x405153: signal_channel_callback (ulogd.c:443)
==4797==    by 0x406163: ulogd_select_main (select.c:105)
==4797==    by 0x403CF3: ulogd_main_loop (ulogd.c:1070)
==4797==    by 0x403CF3: main (ulogd.c:1649)
==4797==  Block was alloc'd at
==4797==    at 0x4849D82: calloc (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4797==    by 0x405504: pluginstance_alloc_init (ulogd.c:664)
==4797==    by 0x405504: create_stack (ulogd.c:1014)
==4797==    by 0x406FCE: config_parse_file (conffile.c:225)
==4797==    by 0x403949: parse_conffile (ulogd.c:1107)
==4797==    by 0x403949: main (ulogd.c:1589)
==4797== 
==4797== Invalid read of size 8
==4797==    at 0x405F6E: ulogd_unregister_fd (select.c:73)
==4797==    by 0x4E6427F: ??? (in
/usr/lib/ulogd/ulogd_filter_IFINDEX.so)
==4797==    by 0x405003: stop_pluginstances (ulogd.c:1335)
==4797==    by 0x405003: sigterm_handler_task (ulogd.c:1383)
==4797==    by 0x405153: call_signal_handler_tasks (ulogd.c:424)
==4797==    by 0x405153: signal_channel_callback (ulogd.c:443)
==4797==    by 0x406163: ulogd_select_main (select.c:105)
==4797==    by 0x403CF3: ulogd_main_loop (ulogd.c:1070)
==4797==    by 0x403CF3: main (ulogd.c:1649)
==4797==  Address 0x4a84f30 is 144 bytes inside a block of size 4,848
free'd
==4797==    at 0x4847551: free (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4797==    by 0x40500E: stop_pluginstances (ulogd.c:1338)
==4797==    by 0x40500E: sigterm_handler_task (ulogd.c:1383)
==4797==    by 0x405153: call_signal_handler_tasks (ulogd.c:424)
==4797==    by 0x405153: signal_channel_callback (ulogd.c:443)
==4797==    by 0x406163: ulogd_select_main (select.c:105)
==4797==    by 0x403CF3: ulogd_main_loop (ulogd.c:1070)
==4797==    by 0x403CF3: main (ulogd.c:1649)
==4797==  Block was alloc'd at
==4797==    at 0x4849D82: calloc (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4797==    by 0x405504: pluginstance_alloc_init (ulogd.c:664)
==4797==    by 0x405504: create_stack (ulogd.c:1014)
==4797==    by 0x406FCE: config_parse_file (conffile.c:225)
==4797==    by 0x403949: parse_conffile (ulogd.c:1107)
==4797==    by 0x403949: main (ulogd.c:1589)
==4797== 
Tue Mar 14 11:18:20 2023 <1> ulogd.c:1333 calling stop for SYSLOG
==4797== 
==4797== HEAP SUMMARY:
==4797==     in use at exit: 301,152 bytes in 10 blocks
==4797==   total heap usage: 1,133 allocs, 1,123 frees, 1,852,378 bytes
allocated
==4797== 
==4797== LEAK SUMMARY:
==4797==    definitely lost: 300,928 bytes in 4 blocks
==4797==    indirectly lost: 224 bytes in 6 blocks
==4797==      possibly lost: 0 bytes in 0 blocks
==4797==    still reachable: 0 bytes in 0 blocks
==4797==         suppressed: 0 bytes in 0 blocks
==4797== Rerun with --leak-check=full to see details of leaked memory
==4797== 
==4797== Use --track-origins=yes to see where uninitialised values come
from
==4797== For lists of detected and suppressed errors, rerun with: -s
==4797== ERROR SUMMARY: 12 errors from 6 contexts (suppressed: 0 from
0)

This is coredump backtrace.

---------------------------------------------------------------------
-----------
Thread Information:
  Id   Target Id         Frame 
* 1    LWP 170           0x00007fa985225d40 in main_arena ()
   from /tmp/tmp1fbxh6bj/vfw_x86_64/output/awplus-
vfw_x86_64/image/rootfs/staging/lib64/libc.so.6
---------------------------------------------------------------------
-----------
Current Thread Local Variables:
No symbol table info available.
---------------------------------------------------------------------
-----------
Thread Backtraces:
Thread 1 (LWP 170):
#0  0x00007fa985225d40 in main_arena () from
/tmp/tmp1fbxh6bj/vfw_x86_64/output/awplus-
vfw_x86_64/image/rootfs/staging/lib64/libc.so.6
#1  0x0000000000405ca7 in ulogd_propagate_results (pi=pi@entry=0x8ebc50
) at ulogd.c:617
#2  0x00007fa9850376b9 in interp_packet (upi=upi@entry=0x8ebc50,
pf_family=<optimized out>, ldata=ldata@entry=0x7ffe805c2648) at
ulogd_inppkt_NFLOG.c:400
#3  0x00007fa985037c55 in msg_cb (gh=<optimized out>,
nfmsg=0x7ffe805c2770, nfa=0x7ffe805c2648, data=0x8e2d90) at
ulogd_inppkt_NFLOG.c:479
#4  0x00007fa98503023e in __nflog_rcv_pkt (nlh=<optimized out>,
nfa=<optimized out>, data=<optimized out>) at libnetfilter_log.c:162
#5  0x00007fa9850288fc in nfnl_step (h=h@entry=0x8e6f80, nlh=nlh@entry=
0x7ffe805c2760) at libnfnetlink.c:1365
#6  0x00007fa985028ff8 in nfnl_process (h=h@entry=0x8e6f80, 
buf=buf@entry=0x7ffe805c2760 "\214", len=len@entry=140) at
libnfnetlink.c:1410
#7  0x00007fa985029344 in nfnl_catch (h=<optimized out>) at
libnfnetlink.c:1564
#8  nfnl_catch (h=0x8e6f80) at libnfnetlink.c:1546
#9  0x00007fa985030612 in __build_send_cfg_msg (pf=0 '\000',
groupnum=<optimized out>, command=2 '\002', h=0x8dfbc0) at
libnetfilter_log.c:143
#10 nflog_unbind_group (gh=0x8e71a0) at libnetfilter_log.c:439
#11 0x00007fa9850373ec in stop (pi=0x8e2d90) at
ulogd_inppkt_NFLOG.c:638
#12 0x0000000000405004 in stop_pluginstances () at ulogd.c:1335
#13 sigterm_handler_task (signal=signal@entry=15) at ulogd.c:1383
#14 0x0000000000405154 in call_signal_handler_tasks (sig=15) at
ulogd.c:423
#15 signal_channel_callback (fd=4, what=<optimized out>,
data=<optimized out>) at ulogd.c:442
#16 0x0000000000406184 in ulogd_select_main (tv=tv@entry=0x0) at
select.c:105
#17 0x0000000000403cf4 in ulogd_main_loop () at ulogd.c:1070
#18 main (argc=<optimized out>, argv=<optimized out>) at ulogd.c:1649
---------------------------------------------------------------------
-----------
On Thu, 2023-03-16 at 12:28 +0100, Florian Westphal wrote:
> Kyuwon Shim <kyuwon.shim@xxxxxxxxxxxxxxxxxxx> wrote:
> > The issue "core dumped" occurred  from
> > ulogd_unregister_fd(). One of the processes is unlink
> > from list and remove, but some struct 'pi' values
> > freed without ulogd_unregister_fd().
> > Unlink process needs to access the previous pointer
> > value of struct 'pi', but it was already freed.
> > 
> > Therefore, the free() process moved location
> > after finishing all ulogd_unregister_fd().
> 
> I don't understand this patch.
> 
> llist_for_each_entry_safe() doesn't dereference 'pi' after its
> free'd.
> 
> Where does this deref happen?  Can you share a backtrace?
> 
> > +		}
> > +	}
> > +
> > +	llist_for_each_entry(stack, &ulogd_pi_stacks, stack_list) {
> > +		llist_for_each_entry_safe(pi, npi, &stack->list, list)
> > {
> >  			free(pi);
> 
> Perhaps there should be a 'llist_del' before pi gets free'd instead?




[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux