CVE-2017-6346: Use After Free (CWE-416)
value: Race condition in net/packet/af_packet.c in the Linux kernel before 4.9.13 allows local users to cause a denial of service (use-after-free) or possibly have unspecified other impact via a multithreaded application that makes PACKET_FANOUT setsockopt system calls.
CWE description: The product reuses or references memory after it has been freed. At some point afterward, the memory may be allocated again and saved in another pointer, while the original pointer references a location somewhere within the new allocation. Any operations using the original pointer are no longer valid because the memory "belongs" to the code that operates on the new pointer.
CVSS v3 base score: 7
Published: 2017-03-01T20:59Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: net/packet/af_packet.c
Commit date: 2017-02-14 15:05:12 -0500
Commit message: packet: fix races in fanout_add()

Multiple threads can call fanout_add() at the same time.

We need to grab fanout_mutex earlier to avoid races that could
lead to one thread freeing po->rollover that was set by another thread.

Do the same in fanout_release(), for peace of mind, and to help us
finding lockdep issues earlier.

Fixes: dc99f600698d ("packet: Add fanout support.")
Fixes: 0648ab70afe6 ("packet: rollover prepare: per-socket state")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Willem de Bruijn <willemb@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

Diff: diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index d56ee46b11fc..0f03f6a53b4d 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1617,10 +1617,11 @@ static void fanout_release_data(struct packet_fanout *f)
 	};
 }
 
 static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
 {
+	struct packet_rollover *rollover = NULL;
 	struct packet_sock *po = pkt_sk(sk);
 	struct packet_fanout *f, *match;
 	u8 type = type_flags & 0xff;
 	u8 flags = type_flags >> 8;
 	int err;
@@ -1639,27 +1640,32 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
 		break;
 	default:
 		return -EINVAL;
 	}
 
+	mutex_lock(&fanout_mutex);
+
+	err = -EINVAL;
 	if (!po->running)
-		return -EINVAL;
+		goto out;
 
+	err = -EALREADY;
 	if (po->fanout)
-		return -EALREADY;
+		goto out;
 
 	if (type == PACKET_FANOUT_ROLLOVER ||
 	    (type_flags & PACKET_FANOUT_FLAG_ROLLOVER)) {
-		po->rollover = kzalloc(sizeof(*po->rollover), GFP_KERNEL);
-		if (!po->rollover)
-			return -ENOMEM;
-		atomic_long_set(&po->rollover->num, 0);
-		atomic_long_set(&po->rollover->num_huge, 0);
-		atomic_long_set(&po->rollover->num_failed, 0);
+		err = -ENOMEM;
+		rollover = kzalloc(sizeof(*rollover), GFP_KERNEL);
+		if (!rollover)
+			goto out;
+		atomic_long_set(&rollover->num, 0);
+		atomic_long_set(&rollover->num_huge, 0);
+		atomic_long_set(&rollover->num_failed, 0);
+		po->rollover = rollover;
 	}
 
-	mutex_lock(&fanout_mutex);
 	match = NULL;
 	list_for_each_entry(f, &fanout_list, list) {
 		if (f->id == id &&
 		    read_pnet(&f->net) == sock_net(sk)) {
 			match = f;
@@ -1702,40 +1708,39 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
 			__fanout_link(sk, po);
 			err = 0;
 		}
 	}
 out:
-	mutex_unlock(&fanout_mutex);
-	if (err) {
-		kfree(po->rollover);
+	if (err && rollover) {
+		kfree(rollover);
 		po->rollover = NULL;
 	}
+	mutex_unlock(&fanout_mutex);
 	return err;
 }
 
 static void fanout_release(struct sock *sk)
 {
 	struct packet_sock *po = pkt_sk(sk);
 	struct packet_fanout *f;
 
-	f = po->fanout;
-	if (!f)
-		return;
-
 	mutex_lock(&fanout_mutex);
-	po->fanout = NULL;
+	f = po->fanout;
+	if (f) {
+		po->fanout = NULL;
+
+		if (atomic_dec_and_test(&f->sk_ref)) {
+			list_del(&f->list);
+			dev_remove_pack(&f->prot_hook);
+			fanout_release_data(f);
+			kfree(f);
+		}
 
-	if (atomic_dec_and_test(&f->sk_ref)) {
-		list_del(&f->list);
-		dev_remove_pack(&f->prot_hook);
-		fanout_release_data(f);
-		kfree(f);
+		if (po->rollover)
+			kfree_rcu(po->rollover, rcu);
 	}
 	mutex_unlock(&fanout_mutex);
-
-	if (po->rollover)
-		kfree_rcu(po->rollover, rcu);
 }
 
 static bool packet_extra_vlan_len_allowed(const struct net_device *dev,
 					  struct sk_buff *skb)
 {

Vulnerable code: 		return -EINVAL;
		return -EALREADY;
		po->rollover = kzalloc(sizeof(*po->rollover), GFP_KERNEL);
		if (!po->rollover)
			return -ENOMEM;
		atomic_long_set(&po->rollover->num, 0);
		atomic_long_set(&po->rollover->num_huge, 0);
		atomic_long_set(&po->rollover->num_failed, 0);
	mutex_lock(&fanout_mutex);
	mutex_unlock(&fanout_mutex);
	if (err) {
		kfree(po->rollover);
	f = po->fanout;
	if (!f)
		return;

	po->fanout = NULL;
	if (atomic_dec_and_test(&f->sk_ref)) {
		list_del(&f->list);
		dev_remove_pack(&f->prot_hook);
		fanout_release_data(f);
		kfree(f);

	if (po->rollover)
		kfree_rcu(po->rollover, rcu);
Fixed code: 	struct packet_rollover *rollover = NULL;
	mutex_lock(&fanout_mutex);

	err = -EINVAL;
		goto out;
	err = -EALREADY;
		goto out;
		err = -ENOMEM;
		rollover = kzalloc(sizeof(*rollover), GFP_KERNEL);
		if (!rollover)
			goto out;
		atomic_long_set(&rollover->num, 0);
		atomic_long_set(&rollover->num_huge, 0);
		atomic_long_set(&rollover->num_failed, 0);
		po->rollover = rollover;
	if (err && rollover) {
		kfree(rollover);
	mutex_unlock(&fanout_mutex);
	f = po->fanout;
	if (f) {
		po->fanout = NULL;

		if (atomic_dec_and_test(&f->sk_ref)) {
			list_del(&f->list);
			dev_remove_pack(&f->prot_hook);
			fanout_release_data(f);
			kfree(f);
		}
		if (po->rollover)
			kfree_rcu(po->rollover, rcu);