Fix ipt_ULOG panics on SMP kernels

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

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
@@ -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);

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.

Done.

Regards,

--Mark

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.

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
@@ -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
@@ -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
@@ -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");

Applied, thanks Mark.