* [PATCH v2] ulogd2: Avoid use after free in unregister on global ulogd_fds linked list
[not found] <1678233154187.35009@alliedtelesis.co.nz>
@ 2023-03-09 1:24 ` Kyuwon Shim
2023-03-16 11:28 ` Florian Westphal
0 siblings, 1 reply; 6+ messages in thread
From: Kyuwon Shim @ 2023-03-09 1:24 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, Kyuwon Shim
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().
Signed-off-by: Kyuwon Shim <kyuwon.shim@alliedtelesis.co.nz>
---
Notes:
Add new patch revision in plain-text that applies cleanly to master
src/ulogd.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/ulogd.c b/src/ulogd.c
index 8ea9793ec..944637e0d 100644
--- a/src/ulogd.c
+++ b/src/ulogd.c
@@ -1334,6 +1334,11 @@ static void stop_pluginstances()
(*pi->plugin->stop)(pi);
pi->private[0] = 0;
}
+ }
+ }
+
+ llist_for_each_entry(stack, &ulogd_pi_stacks, stack_list) {
+ llist_for_each_entry_safe(pi, npi, &stack->list, list) {
free(pi);
}
}
--
2.39.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ulogd2: Avoid use after free in unregister on global ulogd_fds linked list
2023-03-09 1:24 ` [PATCH v2] ulogd2: Avoid use after free in unregister on global ulogd_fds linked list Kyuwon Shim
@ 2023-03-16 11:28 ` Florian Westphal
2023-03-20 20:41 ` Kyuwon Shim
0 siblings, 1 reply; 6+ messages in thread
From: Florian Westphal @ 2023-03-16 11:28 UTC (permalink / raw)
To: Kyuwon Shim; +Cc: pablo, netfilter-devel
Kyuwon Shim <kyuwon.shim@alliedtelesis.co.nz> 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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ulogd2: Avoid use after free in unregister on global ulogd_fds linked list
2023-03-16 11:28 ` Florian Westphal
@ 2023-03-20 20:41 ` Kyuwon Shim
2023-03-20 21:43 ` Jeremy Sowden
2023-03-20 22:10 ` Florian Westphal
0 siblings, 2 replies; 6+ messages in thread
From: Kyuwon Shim @ 2023-03-20 20:41 UTC (permalink / raw)
To: fw@strlen.de; +Cc: pablo@netfilter.org, netfilter-devel@vger.kernel.org
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@alliedtelesis.co.nz> 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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ulogd2: Avoid use after free in unregister on global ulogd_fds linked list
2023-03-20 20:41 ` Kyuwon Shim
@ 2023-03-20 21:43 ` Jeremy Sowden
2023-03-20 22:10 ` Florian Westphal
1 sibling, 0 replies; 6+ messages in thread
From: Jeremy Sowden @ 2023-03-20 21:43 UTC (permalink / raw)
To: Kyuwon Shim
Cc: fw@strlen.de, pablo@netfilter.org,
netfilter-devel@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 13251 bytes --]
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@alliedtelesis.co.nz> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ulogd2: Avoid use after free in unregister on global ulogd_fds linked list
2023-03-20 20:41 ` Kyuwon Shim
2023-03-20 21:43 ` Jeremy Sowden
@ 2023-03-20 22:10 ` Florian Westphal
2023-03-20 23:35 ` Kyuwon Shim
1 sibling, 1 reply; 6+ messages in thread
From: Florian Westphal @ 2023-03-20 22:10 UTC (permalink / raw)
To: Kyuwon Shim
Cc: fw@strlen.de, pablo@netfilter.org,
netfilter-devel@vger.kernel.org
Kyuwon Shim <Kyuwon.Shim@alliedtelesis.co.nz> wrote:
> 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
Yuck, thanks for the backtrace. I've applied the patch with an amended
changelog and a comment wrt. ::stop doing such things.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ulogd2: Avoid use after free in unregister on global ulogd_fds linked list
2023-03-20 22:10 ` Florian Westphal
@ 2023-03-20 23:35 ` Kyuwon Shim
0 siblings, 0 replies; 6+ messages in thread
From: Kyuwon Shim @ 2023-03-20 23:35 UTC (permalink / raw)
To: fw@strlen.de; +Cc: pablo@netfilter.org, netfilter-devel@vger.kernel.org
Thank you Florian!
Have a great day!
On Mon, 2023-03-20 at 23:10 +0100, Florian Westphal wrote:
> Kyuwon Shim <Kyuwon.Shim@alliedtelesis.co.nz> wrote:
> > 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
>
> Yuck, thanks for the backtrace. I've applied the patch with an
> amended
> changelog and a comment wrt. ::stop doing such things.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-20 23:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1678233154187.35009@alliedtelesis.co.nz>
2023-03-09 1:24 ` [PATCH v2] ulogd2: Avoid use after free in unregister on global ulogd_fds linked list Kyuwon Shim
2023-03-16 11:28 ` Florian Westphal
2023-03-20 20:41 ` Kyuwon Shim
2023-03-20 21:43 ` Jeremy Sowden
2023-03-20 22:10 ` Florian Westphal
2023-03-20 23:35 ` Kyuwon Shim
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.