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]

 



On 2023-03-20, at 20:41:46 +0000, Kyuwon Shim wrote:
> 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?
>
> 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 /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)

I've seen these traces myself.  Never got round to diagnosing and fixing
them.

J.

> 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
> --------------------------------------------------------------------------------

Attachment: signature.asc
Description: PGP signature


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

  Powered by Linux