Skip to content

Commit d552c8c

Browse files
edumazetgregkh
authored andcommitted
packet: avoid panic in packet_getsockopt()
[ Upstream commit 509c7a1ecc8601f94ffba8a00889fefb239c00c6 ] 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. In v2: I replaced kfree(rollover) in fanout_add() to kfree_rcu() variant, as spotted by John. Fixes: a9b6391 ("packet: rollover statistics") Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Willem de Bruijn <willemb@google.com> Cc: John Sperbeck <jsperbeck@google.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent ef3a12f commit d552c8c

1 file changed

Lines changed: 16 additions & 8 deletions

File tree

net/packet/af_packet.c

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1724,7 +1724,7 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
17241724

17251725
out:
17261726
if (err && rollover) {
1727-
kfree(rollover);
1727+
kfree_rcu(rollover, rcu);
17281728
po->rollover = NULL;
17291729
}
17301730
mutex_unlock(&fanout_mutex);
@@ -1751,8 +1751,10 @@ static struct packet_fanout *fanout_release(struct sock *sk)
17511751
else
17521752
f = NULL;
17531753

1754-
if (po->rollover)
1754+
if (po->rollover) {
17551755
kfree_rcu(po->rollover, rcu);
1756+
po->rollover = NULL;
1757+
}
17561758
}
17571759
mutex_unlock(&fanout_mutex);
17581760

@@ -3769,6 +3771,7 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
37693771
void *data = &val;
37703772
union tpacket_stats_u st;
37713773
struct tpacket_rollover_stats rstats;
3774+
struct packet_rollover *rollover;
37723775

37733776
if (level != SOL_PACKET)
37743777
return -ENOPROTOOPT;
@@ -3847,13 +3850,18 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
38473850
0);
38483851
break;
38493852
case PACKET_ROLLOVER_STATS:
3850-
if (!po->rollover)
3853+
rcu_read_lock();
3854+
rollover = rcu_dereference(po->rollover);
3855+
if (rollover) {
3856+
rstats.tp_all = atomic_long_read(&rollover->num);
3857+
rstats.tp_huge = atomic_long_read(&rollover->num_huge);
3858+
rstats.tp_failed = atomic_long_read(&rollover->num_failed);
3859+
data = &rstats;
3860+
lv = sizeof(rstats);
3861+
}
3862+
rcu_read_unlock();
3863+
if (!rollover)
38513864
return -EINVAL;
3852-
rstats.tp_all = atomic_long_read(&po->rollover->num);
3853-
rstats.tp_huge = atomic_long_read(&po->rollover->num_huge);
3854-
rstats.tp_failed = atomic_long_read(&po->rollover->num_failed);
3855-
data = &rstats;
3856-
lv = sizeof(rstats);
38573865
break;
38583866
case PACKET_TX_HAS_OFF:
38593867
val = po->tp_tx_has_off;

0 commit comments

Comments
 (0)