netdev - Re: [patch 11/11] netfilter warning fix

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20070205.184408.34759062.davem@davemloft.net>
Date:	Mon, 05 Feb 2007 18:44:08 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	akpm@...ux-foundation.org
Cc:	netdev@...r.kernel.org, kaber@...sh.net, dipankar@...ibm.com,
	paulmck@...ibm.com, mingo@...e.hu
Subject: Re: [patch 11/11] netfilter warning fix

From: Andrew Morton <akpm@...ux-foundation.org>
Date: Mon, 5 Feb 2007 18:18:10 -0800

> I think the finger was pointed at preemptible rcu, in -mm.  iirc,
> the net stats code is assuming that rcu_read_lock() disables
> preemption as a side-effect, which rcu-preempt makes no-longer-true.
>
> Not sure what to do there.  Perhaps add a new
> rcu_read_lock_with_preempt_disable() thing which will dtrt with
> either config.

Hmmm, really?

Let's audit NF_CT_STAT_INC() usage to make sure :-)

net/netfilter/nf_conntrack_core.c:

	destroy_conntrack: Inside write_{lock,unlock}_bh().
	death_by_timeout: Ditto.
	__nf_conntrack_find: Inside read_{lock,unlock}_bh() via callers.
	__nf_conntrack_confirm: Inside write_{lock,unlock}_bh().
	early_drop: This one looks like it could be unprotected.
	init_conntrack: Inside of write_{lock,unlock}_bh().
	nf_conntrack_in: Packet receive path, softints disabled.

net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c:

	ipv6_prepare: Packet input path, BH's disabled.

net/netfilter/nf_conntrack_expect.c:

	nf_ct_unlink_expect: Inside if write_{lock,unlock}_bh() via callers.
	nf_conntrack_expect_insert: Ditto.

So that leaves early_drop() as the only suspicious case that might
not run inside of disabled BH's.

And in fact that case is a bug regardless of the preemptible rcu
changes because this allows the counter bump to be corrupted by
software interrupt context.

And OK, I see in the lockdep trace that it's the packet transmit
path...   In fact, this assumption of preemption being disabled
by the netfilter top-level dispatch is very deep.

For example, several bits besides the NF_CT_STATIC_INC of
nf_conntrack_in() (where the lockdep trigger backtrace hits) assume
that preemption is enabled by that rcu_read_lock() in the top-level
netfilter dispatch.

The __nf_ct_l{3,4}proto_find() calls there are just two examples.

I imagine this assumption is quite pervasive throughout the
netfilter code, so just patching up this NF_CT_STAT_INC() case
will merely shut up lockdep and paper over the issue.

I bet this rcu_read_lock()-implies-preempt_disable() assumption has
spread into other areas of the tree as well.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html