| From 58ad436fcf49810aa006016107f494c9ac9013db Mon Sep 17 00:00:00 2001 |
| From: Johannes Berg <johannes.berg@intel.com> |
| Date: Tue, 13 Aug 2013 09:04:05 +0200 |
| Subject: genetlink: fix family dump race |
| |
| From: Johannes Berg <johannes.berg@intel.com> |
| |
| 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> |
| |
| --- |
| net/netlink/genetlink.c | 7 +++++++ |
| 1 file changed, 7 insertions(+) |
| |
| --- a/net/netlink/genetlink.c |
| +++ b/net/netlink/genetlink.c |
| @@ -789,6 +789,10 @@ static int ctrl_dumpfamily(struct sk_buf |
| 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; |
| @@ -810,6 +814,9 @@ errout: |
| cb->args[0] = i; |
| cb->args[1] = n; |
| |
| + if (need_locking) |
| + genl_unlock(); |
| + |
| return skb->len; |
| } |
| |
