Skip to content

Commit

Permalink
genetlink: fix family dump race
Browse files Browse the repository at this point in the history
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: stable@vger.kernel.org
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>
  • Loading branch information
Johannes Berg authored and David S. Miller committed Aug 13, 2013
1 parent 771085d commit 58ad436
Showing 1 changed file with 7 additions and 0 deletions.
7 changes: 7 additions & 0 deletions net/netlink/genetlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,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;
Expand All @@ -810,6 +814,9 @@ static int ctrl_dumpfamily(struct sk_buff *skb, struct netlink_callback *cb)
cb->args[0] = i;
cb->args[1] = n;

if (need_locking)
genl_unlock();

return skb->len;
}

Expand Down

0 comments on commit 58ad436

Please sign in to comment.