2013-09-11 04:58:32

by Steven Rostedt

[permalink] [raw]
Subject: [199/251] genetlink: fix family dump race

3.6.11.9-rc1 stable review patch.
If anyone has any objections, please let me know.

------------------

From: Johannes Berg <[email protected]>

[ Upstream commit 58ad436fcf49810aa006016107f494c9ac9013db ]

When dumping generic netlink families, only the first dump call
is locked with genl_lock(), which protects the list of families,
and thus subsequent calls can access the data without locking,
racing against family addition/removal. This can cause a crash.
Fix it - the locking needs to be conditional because the first
time around it's already locked.

A similar bug was reported to me on an old kernel (3.4.47) but
the exact scenario that happened there is no longer possible,
on those kernels the first round wasn't locked either. Looking
at the current code I found the race described above, which had
also existed on the old kernel.

Cc: [email protected]
Reported-by: Andrei Otcheretianski <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
net/netlink/genetlink.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 42556ce..17e7104 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -749,6 +749,10 @@ static int ctrl_dumpfamily(struct sk_buff *skb, struct netlink_callback *cb)
struct net *net = sock_net(skb->sk);
int chains_to_skip = cb->args[0];
int fams_to_skip = cb->args[1];
+ bool need_locking = chains_to_skip || fams_to_skip;
+
+ if (need_locking)
+ genl_lock();

for (i = chains_to_skip; i < GENL_FAM_TAB_SIZE; i++) {
n = 0;
@@ -770,6 +774,9 @@ errout:
cb->args[0] = i;
cb->args[1] = n;

+ if (need_locking)
+ genl_unlock();
+
return skb->len;
}

--
1.7.10.4


2013-09-11 06:50:33

by Berg, Johannes

[permalink] [raw]
Subject: RE: [199/251] genetlink: fix family dump race

> 3.6.11.9-rc1 stable review patch.
> If anyone has any objections, please let me know.

Yes, this patch is broken and we reverted it upstream.

johannes
--

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

2013-09-11 12:25:52

by Steven Rostedt

[permalink] [raw]
Subject: Re: [199/251] genetlink: fix family dump race

On Wed, 11 Sep 2013 06:50:10 +0000
"Berg, Johannes" <[email protected]> wrote:

> > 3.6.11.9-rc1 stable review patch.
> > If anyone has any objections, please let me know.
>
> Yes, this patch is broken and we reverted it upstream.

Thanks! I'll revert it.

-- Steve