All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	netfilter@vger.kernel.org,
	"Paul E. McKenney" <paulmck@us.ibm.com>,
	Stephen Hemminger <shemminger@vyatta.com>
Cc: Eric Dumazet <dada1@cosmosbay.com>,
	"David S. Miller" <davem@davemloft.net>,
	Patrick McHardy <kaber@trash.net>,
	Rusty Russell <rusty@rustcorp.com.au>,
	coreteam@netfilter.org
Subject: [PATCH] netfilter: iptables: lock free counters, PREEMPT_RCU=y fix
Date: Thu, 2 Apr 2009 22:12:45 +0200	[thread overview]
Message-ID: <20090402201245.GA29904@elte.hu> (raw)
In-Reply-To: <20090402200128.GA21805@elte.hu>


Impact: fix log spam under CONFIG_DEBUG_PREEMPT=y

This recent commit:

   7845447: netfilter: iptables: lock free counters

Converted a couple of netfilter codepaths from read_lock() critical 
sections to lockless rcu_read_lock(). What it forgot about is that 
under CONFIG_PREEMPT=y and CONFIG_PREEMPT_RCU=y these sections can 
be preempted.

Under CONFIG_DEBUG_PREEMPT=y this produces such warnings:

BUG: using smp_processor_id() in preemptible [00000000] code: ssh/9115
caller is ipt_do_table+0xc8/0x559
Pid: 9115, comm: ssh Tainted: G        W  2.6.29-tip-08646-g45ef7c3-dirty #26231
Call Trace:
 [<c0c0dacf>] ? printk+0x14/0x16
 [<c048c2e6>] debug_smp_processor_id+0xa6/0xbc
 [<c0adbd03>] ipt_do_table+0xc8/0x559
 [<c0c10277>] ? _read_unlock+0x3d/0x49
 [<c0ad68c0>] ? fn_hash_lookup+0x94/0xa0
 [<c0ad330e>] ? __inet_dev_addr_type+0x56/0x8d
 [<c0a87b02>] ? neigh_lookup+0xe5/0x108
 [<c0adc2bc>] ipt_local_hook+0x40/0x50
 [<c0a93e57>] nf_iterate+0x34/0x80
 [<c0aad4b8>] ? dst_output+0x0/0x10
 [<c0a93eea>] nf_hook_slow+0x47/0xa4
 [<c0aad4b8>] ? dst_output+0x0/0x10
 [<c0aaec04>] __ip_local_out+0x78/0x7f
 [<c0aad4b8>] ? dst_output+0x0/0x10
 [<c0aaec1b>] ip_local_out+0x10/0x20
 [<c0aaf416>] ip_queue_xmit+0x2bc/0x332
 [<c0aa8fdf>] ? __ip_route_output_key+0x112/0x77b
 [<c01397fe>] ? local_bh_enable+0x10/0x12
 [<c0abf0e5>] ? tcp_connect+0x32a/0x3bb
 [<c0ab15a7>] ? __inet_hash_nolisten+0x97/0xaf
 [<c0a78dde>] ? __copy_skb_header+0xe/0x13a
 [<c0abf0e5>] ? tcp_connect+0x32a/0x3bb
 [<c0abe955>] ? tcp_transmit_skb+0x5a5/0x61c
 [<c0abe995>] tcp_transmit_skb+0x5e5/0x61c
 [<c0a7b6c0>] ? __alloc_skb+0x54/0x120
 [<c0abefca>] ? tcp_connect+0x20f/0x3bb
 [<c0abf0e5>] tcp_connect+0x32a/0x3bb
 [<c0ac4b68>] tcp_v4_connect+0x466/0x4be
 [<c0acf380>] inet_stream_connect+0x8f/0x212
 [<c018f4e6>] ? might_fault+0x75/0x77
 [<c047f198>] ? copy_from_user+0x2f/0x117
BUG: using smp_processor_id() in preemptible [00000000] code: ssh/9114

Since it appears that the tables are RCU freed, and there are no 
non-preempt assumptions in the code, the using of 
raw_smp_processor_id() is safe.

[ I also audited all of net/netfilter/*.c for smp_processor_id() use,
  and fixed all places that used them unsafely. ]

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 net/ipv4/netfilter/arp_tables.c |    2 +-
 net/ipv4/netfilter/ip_tables.c  |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 35c5f6a..30baf3e 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -255,7 +255,7 @@ unsigned int arpt_do_table(struct sk_buff *skb,
 
 	rcu_read_lock();
 	private = rcu_dereference(table->private);
-	table_base = rcu_dereference(private->entries[smp_processor_id()]);
+	table_base = rcu_dereference(private->entries[raw_smp_processor_id()]);
 
 	e = get_entry(table_base, private->hook_entry[hook]);
 	back = get_entry(table_base, private->underflow[hook]);
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 82ee7c9..eff124e 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -280,7 +280,7 @@ static void trace_packet(struct sk_buff *skb,
 	char *hookname, *chainname, *comment;
 	unsigned int rulenum = 0;
 
-	table_base = (void *)private->entries[smp_processor_id()];
+	table_base = (void *)private->entries[raw_smp_processor_id()];
 	root = get_entry(table_base, private->hook_entry[hook]);
 
 	hookname = chainname = (char *)hooknames[hook];
@@ -341,7 +341,7 @@ ipt_do_table(struct sk_buff *skb,
 
 	rcu_read_lock();
 	private = rcu_dereference(table->private);
-	table_base = rcu_dereference(private->entries[smp_processor_id()]);
+	table_base = rcu_dereference(private->entries[raw_smp_processor_id()]);
 
 	e = get_entry(table_base, private->hook_entry[hook]);
 

  reply	other threads:[~2009-04-02 20:12 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-02 20:01 [netfilter bug] BUG: using smp_processor_id() in preemptible [00000000] code: ssh/9115, caller is ipt_do_table+0xc8/0x559 Ingo Molnar
2009-04-02 20:12 ` Ingo Molnar [this message]
2009-04-02 20:38   ` [PATCH] netfilter: iptables: lock free counters, PREEMPT_RCU=y fix Stephen Hemminger
2009-04-02 20:52     ` Ingo Molnar
2009-04-02 20:18 ` [netfilter bug] BUG: using smp_processor_id() in preemptible [00000000] code: ssh/9115, caller is ipt_do_table+0xc8/0x559 Eric Dumazet
2009-04-02 20:22   ` Ingo Molnar
2009-04-02 20:32   ` Ingo Molnar
2009-04-02 20:32     ` Ingo Molnar
2009-04-02 21:16     ` Ingo Molnar
2009-04-04 17:23       ` Paul E. McKenney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090402201245.GA29904@elte.hu \
    --to=mingo@elte.hu \
    --cc=coreteam@netfilter.org \
    --cc=dada1@cosmosbay.com \
    --cc=davem@davemloft.net \
    --cc=kaber@trash.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter@vger.kernel.org \
    --cc=paulmck@us.ibm.com \
    --cc=rusty@rustcorp.com.au \
    --cc=shemminger@vyatta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.