[PATCH] Fix ipt_ULOG panics on SMP kernels

[PATCH] Fix ipt_ULOG panics on SMP kernels

[Mark Huang]

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

I've run into the same kernel panic as these reports:

https://lists.gnumonks.org/pipermail/ulogd/2005-August/000776.html
http://lists.netfilter.org/pipermail/netfilter/2006-January/064509.html
https://lists.gnumonks.org/pipermail/ulogd/2006-April/000853.html

On various SMP machines. The culprit is a null ub->skb in ulog_send(). I believe
that this can occur for the following reason. If ulog_timer() has already been
scheduled on one CPU and is spinning on the lock, and ipt_ulog_packet() flushes
the queue on another CPU by calling ulog_send() right before it exits (because
the threshold is reached), there will be no skbuff when ulog_timer() acquires
the lock and calls ulog_send(). Cancelling the timer in ulog_send() doesn't help
   because it has already been scheduled and is running on the first CPU.

There are two solutions that I can see: re-allocate ub->skb at the end of
ipt_ulog_packet(), just like it does toward the beginning of the function. But
the problem will still happen if the allocation fails. The second solution,
implemented by the attached patch, is to just return from ulog_send() if ub->skb
is null.

Regards,

--Mark


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

Index: linux-2.6/net/ipv4/netfilter/ipt_ULOG.c
===================================================================
RCS file: /cvs/linux-2.6/net/ipv4/netfilter/ipt_ULOG.c,v
retrieving revision 1.6
diff -u -r1.6 ipt_ULOG.c
--- linux-2.6/net/ipv4/netfilter/ipt_ULOG.c	27 Jul 2006 22:00:49 -0000	1.6
+++ linux-2.6/net/ipv4/netfilter/ipt_ULOG.c	10 Aug 2006 16:30:42 -0000
@@ -120,6 +120,11 @@
 	if (ub->qlen > 1)
 		ub->lastnlh->nlmsg_type = NLMSG_DONE;
 
+	if (!ub->skb) {
+		DEBUGP("ipt_ULOG: ulog_send: nothing to send\n");
+		return;
+	}
+
 	NETLINK_CB(ub->skb).dst_group = nlgroupnum + 1;
 	DEBUGP("ipt_ULOG: throwing %d packets to netlink group %u\n",
 		ub->qlen, nlgroupnum + 1);

Re: [PATCH] Fix ipt_ULOG panics on SMP kernels

[Patrick McHardy]

Mark Huang wrote:
> I've run into the same kernel panic as these reports:
> 
> https://lists.gnumonks.org/pipermail/ulogd/2005-August/000776.html
> http://lists.netfilter.org/pipermail/netfilter/2006-January/064509.html
> https://lists.gnumonks.org/pipermail/ulogd/2006-April/000853.html
> 
> On various SMP machines. The culprit is a null ub->skb in ulog_send(). I
> believe
> that this can occur for the following reason. If ulog_timer() has
> already been
> scheduled on one CPU and is spinning on the lock, and ipt_ulog_packet()
> flushes
> the queue on another CPU by calling ulog_send() right before it exits
> (because
> the threshold is reached), there will be no skbuff when ulog_timer()
> acquires
> the lock and calls ulog_send(). Cancelling the timer in ulog_send()
> doesn't help
>   because it has already been scheduled and is running on the first CPU.
> 
> There are two solutions that I can see: re-allocate ub->skb at the end of
> ipt_ulog_packet(), just like it does toward the beginning of the
> function. But
> the problem will still happen if the allocation fails. The second solution,
> implemented by the attached patch, is to just return from ulog_send() if
> ub->skb
> is null.

Very nice catch, thank you. The second solution is perfectly fine I
think, if the skb has already been sent there is no need to do
anything, a new allocation could be useless if no further traffic
arrives. If you could add a similar fix to
net/bridge/netfilter/ebt_ulog.c and net/netfilter/nfnetlink_log.c
and send me a Signed-off-by: line I'll push it in 2.6.18. Thanks.

Re: [PATCH] Fix ipt_ULOG panics on SMP kernels

[Mark Huang]

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

Patrick McHardy wrote:
> If you could add a similar fix to
> net/bridge/netfilter/ebt_ulog.c and net/netfilter/nfnetlink_log.c
> and send me a Signed-off-by: line I'll push it in 2.6.18. Thanks.

Done.

Regards,

--Mark

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

Fix kernel panic on various SMP machines. The culprit is a null
ub->skb in ulog_send(). If ulog_timer() has already been scheduled on
one CPU and is spinning on the lock, and ipt_ulog_packet() flushes the
queue on another CPU by calling ulog_send() right before it exits,
there will be no skbuff when ulog_timer() acquires the lock and calls
ulog_send(). Cancelling the timer in ulog_send() doesn't help because
it has already been scheduled and is running on the first CPU.

Similar problem exists in ebt_ulog.c and nfnetlink_log.c.

Signed-off-by: Mark Huang <mlhuang@cs.princeton.edu>

Index: linux-2.6/net/ipv4/netfilter/ipt_ULOG.c
===================================================================
RCS file: /cvs/linux-2.6/net/ipv4/netfilter/ipt_ULOG.c,v
retrieving revision 1.1.3.6
retrieving revision 1.7
diff -u -r1.1.3.6 -r1.7
--- linux-2.6/net/ipv4/netfilter/ipt_ULOG.c	27 Jul 2006 20:49:21 -0000	1.1.3.6
+++ linux-2.6/net/ipv4/netfilter/ipt_ULOG.c	10 Aug 2006 17:50:14 -0000	1.7
@@ -120,6 +120,11 @@
 	if (ub->qlen > 1)
 		ub->lastnlh->nlmsg_type = NLMSG_DONE;
 
+	if (!ub->skb) {
+		DEBUGP("ipt_ULOG: ulog_send: nothing to send\n");
+		return;
+	}
+
 	NETLINK_CB(ub->skb).dst_group = nlgroupnum + 1;
 	DEBUGP("ipt_ULOG: throwing %d packets to netlink group %u\n",
 		ub->qlen, nlgroupnum + 1);
Index: linux-2.6/net/bridge/netfilter/ebt_ulog.c
===================================================================
RCS file: /cvs/linux-2.6/net/bridge/netfilter/ebt_ulog.c,v
retrieving revision 1.1.3.2
diff -u -r1.1.3.2 ebt_ulog.c
--- linux-2.6/net/bridge/netfilter/ebt_ulog.c	27 Jul 2006 20:49:20 -0000	1.1.3.2
+++ linux-2.6/net/bridge/netfilter/ebt_ulog.c	11 Aug 2006 16:40:16 -0000
@@ -79,6 +79,9 @@
 	if (ub->qlen > 1)
 		ub->lastnlh->nlmsg_type = NLMSG_DONE;
 
+	if (!ub->skb)
+		return;
+
 	NETLINK_CB(ub->skb).dst_group = nlgroup + 1;
 	netlink_broadcast(ebtulognl, ub->skb, 0, nlgroup + 1, GFP_ATOMIC);
 
Index: linux-2.6/net/netfilter/nfnetlink_log.c
===================================================================
RCS file: /cvs/linux-2.6/net/netfilter/nfnetlink_log.c,v
retrieving revision 1.1.3.1
diff -u -r1.1.3.1 nfnetlink_log.c
--- linux-2.6/net/netfilter/nfnetlink_log.c	27 Jul 2006 20:49:21 -0000	1.1.3.1
+++ linux-2.6/net/netfilter/nfnetlink_log.c	11 Aug 2006 16:40:16 -0000
@@ -369,6 +369,9 @@
 	if (inst->qlen > 1)
 		inst->lastnlh->nlmsg_type = NLMSG_DONE;
 
+	if (!inst->skb)
+		return 0;
+
 	status = nfnetlink_unicast(inst->skb, inst->peer_pid, MSG_DONTWAIT);
 	if (status < 0) {
 		UDEBUG("netlink_unicast() failed\n");

Re: [PATCH] Fix ipt_ULOG panics on SMP kernels

[Patrick McHardy]

Mark Huang wrote:
> Fix kernel panic on various SMP machines. The culprit is a null
> ub->skb in ulog_send(). If ulog_timer() has already been scheduled on
> one CPU and is spinning on the lock, and ipt_ulog_packet() flushes the
> queue on another CPU by calling ulog_send() right before it exits,
> there will be no skbuff when ulog_timer() acquires the lock and calls
> ulog_send(). Cancelling the timer in ulog_send() doesn't help because
> it has already been scheduled and is running on the first CPU.
> 
> Similar problem exists in ebt_ulog.c and nfnetlink_log.c.

Applied, thanks Mark.