ulogd2 segfault

ulogd2 segfault

[Salih Gönüllü]

Hi,

ulogd2 is segfaulting upon receipt of sigint or sigterm


Program received signal SIGTERM, Terminated.
0xb78c4430 in __kernel_vsyscall ()
(gdb) cont
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0xb78b71ba in main_arena () from /lib/libc.so.6
(gdb) bt
#0  0xb78b71ba in main_arena () from /lib/libc.so.6
#1  0x0804a35d in ulogd_propagate_results ()
#2  0xb777256c in interp_packet (upi=0x80630f0, pf_family=2 '\002', 
ldata=0xbff01390)
     at ulogd_inppkt_NFLOG.c:400
#3  0xb7772149 in msg_cb (gh=0x8062f58, nfmsg=0xbff01470, 
nfa=0xbff01390, data=0x805f270)
     at ulogd_inppkt_NFLOG.c:472
#4  0xb78bf7d5 in __nflog_rcv_pkt () from /usr/lib/libnetfilter_log.so.1
#5  0xb7767b00 in nfnl_step () from /usr/lib/libnfnetlink.so.0
#6  0xbff013b0 in ?? ()
#7  0x08062f28 in ?? ()
#8  0x00000001 in ?? ()
#9  0xbff01474 in ?? ()
#10 0x00000000 in ?? ()

I have tried with beta4 and with git 
4f652cc32aebeac20f46009f146ad973a1ed0e99


Is there a way to avoid this ?

#ulogd.conf

stack=log2:NFLOG,base1:BASE,ifi1:IFINDEX,ip2str1:IP2STR,print1:PRINTPKT,emu0:LOGEMU


[log2]
group=42

emu0]
file="/var/log/ulogd_sysemu.log"


Regards,

     -salih


-- 
salih goenuellue
security engineer

open systems ag
raeffelstrasse 29
ch-8045 zurich
t +41 44 455 74 00
f +41 44 455 74 01
sag@open.ch

http://www.open.ch

Re: ulogd2 segfault

[Salih Gönüllü]

Hi,

my last ulogd.conf was not complete. And I think I found out the cause:

stack=log2:NFLOG,base1:BASE,ifi1:IFINDEX,ip2str1:IP2STR,print1:PRINTPKT,emu0:LOGEMU
stack=log2:NFLOG,base1:BASE,ifi1:IFINDEX,ip2str1:IP2STR,print1:PRINTPKT,syslog1:SYSLOG 


having other stacks is not a problem as long as the log2:NFLOG is not 
present, could it be that stop_pluginstances() in this case is doing a 
double free() ?

Thanks,

    -salih


On 12/31/2010 05:23 PM, Salih Gönüllü wrote:
>
> Hi,
>
> ulogd2 is segfaulting upon receipt of sigint or sigterm
>
>
> Program received signal SIGTERM, Terminated.
> 0xb78c4430 in __kernel_vsyscall ()
> (gdb) cont
> Continuing.
>
> Program received signal SIGSEGV, Segmentation fault.
> 0xb78b71ba in main_arena () from /lib/libc.so.6
> (gdb) bt
> #0 0xb78b71ba in main_arena () from /lib/libc.so.6
> #1 0x0804a35d in ulogd_propagate_results ()
> #2 0xb777256c in interp_packet (upi=0x80630f0, pf_family=2 '\002',
> ldata=0xbff01390)
> at ulogd_inppkt_NFLOG.c:400
> #3 0xb7772149 in msg_cb (gh=0x8062f58, nfmsg=0xbff01470, nfa=0xbff01390,
> data=0x805f270)
> at ulogd_inppkt_NFLOG.c:472
> #4 0xb78bf7d5 in __nflog_rcv_pkt () from /usr/lib/libnetfilter_log.so.1
> #5 0xb7767b00 in nfnl_step () from /usr/lib/libnfnetlink.so.0
> #6 0xbff013b0 in ?? ()
> #7 0x08062f28 in ?? ()
> #8 0x00000001 in ?? ()
> #9 0xbff01474 in ?? ()
> #10 0x00000000 in ?? ()
>
> I have tried with beta4 and with git
> 4f652cc32aebeac20f46009f146ad973a1ed0e99
>
>
> Is there a way to avoid this ?
>
> #ulogd.conf
>
> stack=log2:NFLOG,base1:BASE,ifi1:IFINDEX,ip2str1:IP2STR,print1:PRINTPKT,emu0:LOGEMU
>
>
>
> [log2]
> group=42
>
> emu0]
> file="/var/log/ulogd_sysemu.log"
>
>
> Regards,
>
> -salih
>
>

Re: ulogd2 segfault

[Pablo Neira Ayuso]

On 03/01/11 19:12, Salih Gönüllü wrote:
> Hi,
> 
> my last ulogd.conf was not complete. And I think I found out the cause:
> 
> stack=log2:NFLOG,base1:BASE,ifi1:IFINDEX,ip2str1:IP2STR,print1:PRINTPKT,emu0:LOGEMU
> 
> stack=log2:NFLOG,base1:BASE,ifi1:IFINDEX,ip2str1:IP2STR,print1:PRINTPKT,syslog1:SYSLOG
> 
> having other stacks is not a problem as long as the log2:NFLOG is not
> present, could it be that stop_pluginstances() in this case is doing a
> double free() ?

Let me check this, if the problem is what you're pointing (which makes
sense at first look at the code) I guess that we need some refcounting
for plugins to avoid double freeing.

Re: ulogd2 segfault

[Salih Gönüllü]

On 01/06/2011 02:53 PM, Pablo Neira Ayuso wrote:

> Let me check this, if the problem is what you're pointing (which makes
> sense at first look at the code) I guess that we need some refcounting
> for plugins to avoid double freeing.


When I comment out nflog_unbind_group(ui->nful_gh) in  stop() inside 
input/packet/ulogd_inppkt_NFLOG.c,  ulogd does not segfault anymore.

Here I have question:

Does  *pi in
static int stop(struct ulogd_pluginstance *pi)

represent the NFLOG plugin or the individual log2:NFLOG ? I am asking 
because it get started only once with pluginstance_started(), and 
therefore should be stopped once (?)

Cheers,

-salih

Re: ulogd2 segfault

[Pablo Neira Ayuso]

[-- Attachment #1: Type: text/plain, Size: 800 bytes --]

On 07/01/11 11:24, Salih Gönüllü wrote:
> On 01/06/2011 02:53 PM, Pablo Neira Ayuso wrote:
> 
>> Let me check this, if the problem is what you're pointing (which makes
>> sense at first look at the code) I guess that we need some refcounting
>> for plugins to avoid double freeing.
> 
> 
> When I comment out nflog_unbind_group(ui->nful_gh) in  stop() inside
> input/packet/ulogd_inppkt_NFLOG.c,  ulogd does not segfault anymore.
> 
> Here I have question:
> 
> Does  *pi in
> static int stop(struct ulogd_pluginstance *pi)
> 
> represent the NFLOG plugin or the individual log2:NFLOG ? I am asking
> because it get started only once with pluginstance_started(), and
> therefore should be stopped once (?)

Indeed, accurate analysis. Would you give a try to the following patch?

[-- Attachment #2: refcount.patch --]
[-- Type: text/x-patch, Size: 2111 bytes --]

ulogd: fix double call of stop for reused input plugins

From: Pablo Neira Ayuso <pablo@netfilter.org>

This patch adds reference counting for plugins. This is used to fix
a double stop for input plugins that are reused.

This problem was reported by Salih Gonullu <sag@open.ch>:

http://marc.info/?l=netfilter&m=129439584700693&w=2

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/ulogd/ulogd.h |    2 ++
 src/ulogd.c           |   14 ++++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/include/ulogd/ulogd.h b/include/ulogd/ulogd.h
index 2d1b348..e48caf8 100644
--- a/include/ulogd/ulogd.h
+++ b/include/ulogd/ulogd.h
@@ -208,6 +208,8 @@ struct ulogd_plugin {
 	char name[ULOGD_MAX_KEYLEN+1];
 	/* ID for this plugin (dynamically assigned) */
 	unsigned int id;
+	/* how many stacks are using this plugin? initially set to zero. */
+	unsigned int usage;
 
 	struct ulogd_keyset input;
 	struct ulogd_keyset output;
diff --git a/src/ulogd.c b/src/ulogd.c
index f378c6f..a4b0ed1 100644
--- a/src/ulogd.c
+++ b/src/ulogd.c
@@ -762,6 +762,15 @@ static int pluginstance_started(struct ulogd_pluginstance *npi)
 	return 0;
 }
 
+static int pluginstance_stop(struct ulogd_pluginstance *npi)
+{
+	if (--npi->plugin->usage > 0 &&
+	    npi->plugin->input.type == ULOGD_DTYPE_SOURCE) {
+		return 0;
+	}
+	return 1;
+}
+
 static int create_stack_start_instances(struct ulogd_pluginstance_stack *stack)
 {
 	int ret;
@@ -839,6 +848,7 @@ static int create_stack(const char *option)
 			ret = -ENODEV;
 			goto out;
 		}
+		pl->usage++;
 
 		/* allocate */
 		pi = pluginstance_alloc_init(pl, pi_id, stack);
@@ -989,8 +999,8 @@ static void stop_pluginstances()
 
 	llist_for_each_entry(stack, &ulogd_pi_stacks, stack_list) {
 		llist_for_each_entry_safe(pi, npi, &stack->list, list) {
-			if (((pi->plugin->priv_size == 0) || pi->private[0])
-					&& *pi->plugin->stop) {
+			if ((pi->plugin->priv_size > 0 || *pi->plugin->stop) &&
+			    pluginstance_stop(pi)) {
 				ulogd_log(ULOGD_DEBUG, "calling stop for %s\n",
 					  pi->plugin->name);
 				(*pi->plugin->stop)(pi);

Re: ulogd2 segfault

[Salih Gönüllü]

Hi Pablo

On 01/07/2011 02:37 PM, Pablo Neira Ayuso wrote:

> Indeed, accurate analysis. Would you give a try to the following patch?

It works fine, thanks!

Cheers,

    -salih

Re: ulogd2 segfault

[Pablo Neira Ayuso]

On 07/01/11 17:40, Salih Gönüllü wrote:
> Hi Pablo
> 
> On 01/07/2011 02:37 PM, Pablo Neira Ayuso wrote:
> 
>> Indeed, accurate analysis. Would you give a try to the following patch?
> 
> It works fine, thanks!

I have applied the patch, thanks for the accurate report and for testing
it Salih.