Home Home > GIT Browse
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJohannes Berg <johannes.berg@intel.com>2013-08-13 09:04:05 +0200
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2013-08-20 08:21:01 -0700
commitbba2a9f0d381e510ba32f2f984e5ae1e705c90d1 (patch)
tree6569924a75e8979620029aa14dc4347b647f4cc9
parente00f3fdc81f21fe6de883c647aff2efcd6945de4 (diff)
genetlink: fix family dump race
commit 58ad436fcf49810aa006016107f494c9ac9013db 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: Andrei Otcheretianski <andrei.otcheretianski@intel.com> Signed-off-by: Johannes Berg <johannes.berg@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--net/netlink/genetlink.c7
1 files changed, 7 insertions, 0 deletions
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 874f8ffbe423..409dd4056659 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -700,6 +700,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;
@@ -721,6 +725,9 @@ errout:
cb->args[0] = i;
cb->args[1] = n;
+ if (need_locking)
+ genl_unlock();
+
return skb->len;
}