[PATCH net] packet: avoid panic in packet_getsockopt()

* [PATCH net] packet: avoid panic in packet_getsockopt()
@ 2017-10-18 22:08 Eric Dumazet
  2017-10-18 23:14 ` [PATCH v2 " Eric Dumazet
  2017-10-21  0:50 ` [PATCH " David Miller
  0 siblings, 2 replies; 7+ messages in thread
From: Eric Dumazet @ 2017-10-18 22:08 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Willem de Bruijn, Eric Dumazet

From: Eric Dumazet <edumazet@google.com>

syzkaller got crashes in packet_getsockopt() processing
PACKET_ROLLOVER_STATS command while another thread was managing
to change po->rollover

Using RCU will fix this bug. We might later add proper RCU annotations
for sparse sake.

Fixes: a9b6391814d5 ("packet: rollover statistics")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Willem de Bruijn <willemb@google.com>
---
 net/packet/af_packet.c |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index bec01a3daf5b02bd716dbff5c9efef8d6a7982be..1d8a7add86b4f29880e11c6f4971d79319dcb426 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1796,8 +1796,10 @@ static struct packet_fanout *fanout_release(struct sock *sk)
 		else
 			f = NULL;
 
-		if (po->rollover)
+		if (po->rollover) {
 			kfree_rcu(po->rollover, rcu);
+			po->rollover = NULL;
+		}
 	}
 	mutex_unlock(&fanout_mutex);
 
@@ -3851,6 +3853,7 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
 	void *data = &val;
 	union tpacket_stats_u st;
 	struct tpacket_rollover_stats rstats;
+	struct packet_rollover *rollover;
 
 	if (level != SOL_PACKET)
 		return -ENOPROTOOPT;
@@ -3929,13 +3932,18 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
 		       0);
 		break;
 	case PACKET_ROLLOVER_STATS:
-		if (!po->rollover)
+		rcu_read_lock();
+		rollover = rcu_dereference(po->rollover);
+		if (rollover) {
+			rstats.tp_all = atomic_long_read(&rollover->num);
+			rstats.tp_huge = atomic_long_read(&rollover->num_huge);
+			rstats.tp_failed = atomic_long_read(&rollover->num_failed);
+			data = &rstats;
+			lv = sizeof(rstats);
+		}
+		rcu_read_unlock();
+		if (!rollover)
 			return -EINVAL;
-		rstats.tp_all = atomic_long_read(&po->rollover->num);
-		rstats.tp_huge = atomic_long_read(&po->rollover->num_huge);
-		rstats.tp_failed = atomic_long_read(&po->rollover->num_failed);
-		data = &rstats;
-		lv = sizeof(rstats);
 		break;
 	case PACKET_TX_HAS_OFF:
 		val = po->tp_tx_has_off;

^ permalink raw reply related	[flat|nested] 7+ messages in thread