[NETFILTER]: Fix deadlock with ip_queue and tcp local input path.
authorHarald Welte <laforge@netfilter.org>
Mon, 30 May 2005 22:35:26 +0000 (15:35 -0700)
committerDavid S. Miller <davem@davemloft.net>
Mon, 30 May 2005 22:35:26 +0000 (15:35 -0700)
When we have ip_queue being used from LOCAL_IN, then we end up with a
situation where the verdicts coming back from userspace traverse the TCP
input path from syscall context.  While this seems to work most of the
time, there's an ugly deadlock:

syscall context is interrupted by the timer interrupt.  When the timer
interrupt leaves, the timer softirq get's scheduled and calls
tcp_delack_timer() and alike.  They themselves do bh_lock_sock(sk),
which is already held from somewhere else -> boom.

I've now tested the suggested solution by Patrick McHardy and Herbert Xu to
simply use local_bh_{en,dis}able().

Signed-off-by: Harald Welte <laforge@netfilter.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/ipv4/netfilter/ip_queue.c

index e5746b6..eda1fba 100644 (file)
@@ -3,6 +3,7 @@
  * communicating with userspace via netlink.
  *
  * (C) 2000-2002 James Morris <jmorris@intercode.com.au>
+ * (C) 2003-2005 Netfilter Core Team <coreteam@netfilter.org>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -17,6 +18,7 @@
  * 2005-01-10: Added /proc counter for dropped packets; fixed so
  *             packets aren't delivered to user space if they're going 
  *             to be dropped. 
+ * 2005-05-26: local_bh_{disable,enable} around nf_reinject (Harald Welte)
  *
  */
 #include <linux/module.h>
@@ -71,7 +73,15 @@ static DECLARE_MUTEX(ipqnl_sem);
 static void
 ipq_issue_verdict(struct ipq_queue_entry *entry, int verdict)
 {
+       /* TCP input path (and probably other bits) assume to be called
+        * from softirq context, not from syscall, like ipq_issue_verdict is
+        * called.  TCP input path deadlocks with locks taken from timer
+        * softirq, e.g.  We therefore emulate this by local_bh_disable() */
+
+       local_bh_disable();
        nf_reinject(entry->skb, entry->info, verdict);
+       local_bh_enable();
+
        kfree(entry);
 }