Commit aab4f8d4 authored by Johannes Berg's avatar Johannes Berg Committed by Greg Kroah-Hartman

genetlink: fix family dump race

commit 58ad436f upstream.

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.
Reported-by: default avatarAndrei Otcheretianski <andrei.otcheretianski@intel.com>
Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent d00ff4f2
...@@ -789,6 +789,10 @@ static int ctrl_dumpfamily(struct sk_buff *skb, struct netlink_callback *cb) ...@@ -789,6 +789,10 @@ static int ctrl_dumpfamily(struct sk_buff *skb, struct netlink_callback *cb)
struct net *net = sock_net(skb->sk); struct net *net = sock_net(skb->sk);
int chains_to_skip = cb->args[0]; int chains_to_skip = cb->args[0];
int fams_to_skip = cb->args[1]; 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++) { for (i = chains_to_skip; i < GENL_FAM_TAB_SIZE; i++) {
n = 0; n = 0;
...@@ -810,6 +814,9 @@ static int ctrl_dumpfamily(struct sk_buff *skb, struct netlink_callback *cb) ...@@ -810,6 +814,9 @@ static int ctrl_dumpfamily(struct sk_buff *skb, struct netlink_callback *cb)
cb->args[0] = i; cb->args[0] = i;
cb->args[1] = n; cb->args[1] = n;
if (need_locking)
genl_unlock();
return skb->len; return skb->len;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment