netdev - Re: [PATCH] neigh: Explicitly declare RCU-bh read side critical section in neigh

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] [day] [month] [year] [list]

Message-Id: <20160629.080119.236337033632160248.davem@davemloft.net>
Date:	Wed, 29 Jun 2016 08:01:19 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	buytenh@...tstofly.org
Cc:	netdev@...r.kernel.org, dsa@...ulusnetworks.com,
	roopa@...ulusnetworks.com, rshearma@...cade.com,
	ebiederm@...ssion.com
Subject: Re: [PATCH] neigh: Explicitly declare RCU-bh read side critical
 section in neigh_xmit()

From: Lennert Buytenhek <buytenh@...tstofly.org>
Date: Tue, 28 Jun 2016 11:16:43 +0300

> From: David Barroso <dbarroso@...tly.com>
> 
> neigh_xmit() expects to be called inside an RCU-bh read side critical
> section, and while one of its two current callers gets this right, the
> other one doesn't.
> 
> More specifically, neigh_xmit() has two callers, mpls_forward() and
> mpls_output(), and while both callers call neigh_xmit() under
> rcu_read_lock(), this provides sufficient protection for neigh_xmit()
> only in the case of mpls_forward(), as that is always called from
> softirq context and therefore doesn't need explicit BH protection,
> while mpls_output() can be called from process context with softirqs
> enabled.
> 
> When mpls_output() is called from process context, with softirqs
> enabled, we can be preempted by a softirq at any time, and RCU-bh
> considers the completion of a softirq as signaling the end of any
> pending read-side critical sections, so if we do get a softirq
> while we are in the part of neigh_xmit() that expects to be run inside
> an RCU-bh read side critical section, we can end up with an unexpected
> RCU grace period running right in the middle of that critical section,
> making things go boom.
> 
> This patch fixes this impedance mismatch in the callee, by making
> neigh_xmit() always take rcu_read_{,un}lock_bh() around the code that
> expects to be treated as an RCU-bh read side critical section, as this
> seems a safer option than fixing it in the callers.
> 
> Fixes: 4fd3d7d9e868f ("neigh: Add helper function neigh_xmit")
> Signed-off-by: David Barroso <dbarroso@...tly.com>
> Signed-off-by: Lennert Buytenhek <lbuytenhek@...tly.com>
> Acked-by: David Ahern <dsa@...ulusnetworks.com>
> Acked-by: Robert Shearman <rshearma@...cade.com>

Applied.