Skip to content

Commit 57f015f

Browse files
mjamaloneydavem330
authored andcommitted
packet: fix crash in fanout_demux_rollover()
syzkaller found a race condition fanout_demux_rollover() while removing a packet socket from a fanout group. po->rollover is read and operated on during packet_rcv_fanout(), via fanout_demux_rollover(), but the pointer is currently cleared before the synchronization in packet_release(). It is safer to delay the cleanup until after synchronize_net() has been called, ensuring all calls to packet_rcv_fanout() for this socket have finished. To further simplify synchronization around the rollover structure, set po->rollover in fanout_add() only if there are no errors. This removes the need for rcu in the struct and in the call to packet_getsockopt(..., PACKET_ROLLOVER_STATS, ...). Crashing stack trace: fanout_demux_rollover+0xb6/0x4d0 net/packet/af_packet.c:1392 packet_rcv_fanout+0x649/0x7c8 net/packet/af_packet.c:1487 dev_queue_xmit_nit+0x835/0xc10 net/core/dev.c:1953 xmit_one net/core/dev.c:2975 [inline] dev_hard_start_xmit+0x16b/0xac0 net/core/dev.c:2995 __dev_queue_xmit+0x17a4/0x2050 net/core/dev.c:3476 dev_queue_xmit+0x17/0x20 net/core/dev.c:3509 neigh_connected_output+0x489/0x720 net/core/neighbour.c:1379 neigh_output include/net/neighbour.h:482 [inline] ip6_finish_output2+0xad1/0x22a0 net/ipv6/ip6_output.c:120 ip6_finish_output+0x2f9/0x920 net/ipv6/ip6_output.c:146 NF_HOOK_COND include/linux/netfilter.h:239 [inline] ip6_output+0x1f4/0x850 net/ipv6/ip6_output.c:163 dst_output include/net/dst.h:459 [inline] NF_HOOK.constprop.35+0xff/0x630 include/linux/netfilter.h:250 mld_sendpack+0x6a8/0xcc0 net/ipv6/mcast.c:1660 mld_send_initial_cr.part.24+0x103/0x150 net/ipv6/mcast.c:2072 mld_send_initial_cr net/ipv6/mcast.c:2056 [inline] ipv6_mc_dad_complete+0x99/0x130 net/ipv6/mcast.c:2079 addrconf_dad_completed+0x595/0x970 net/ipv6/addrconf.c:4039 addrconf_dad_work+0xac9/0x1160 net/ipv6/addrconf.c:3971 process_one_work+0xbf0/0x1bc0 kernel/workqueue.c:2113 worker_thread+0x223/0x1990 kernel/workqueue.c:2247 kthread+0x35e/0x430 kernel/kthread.c:231 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:432 Fixes: 0648ab7 ("packet: rollover prepare: per-socket state") Fixes: 509c7a1 ("packet: avoid panic in packet_getsockopt()") Reported-by: syzbot <[email protected]> Signed-off-by: Mike Maloney <[email protected]> Reviewed-by: Eric Dumazet <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent a51a40b commit 57f015f

File tree

2 files changed

+10
-23
lines changed

2 files changed

+10
-23
lines changed

net/packet/af_packet.c

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1687,7 +1687,6 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
16871687
atomic_long_set(&rollover->num, 0);
16881688
atomic_long_set(&rollover->num_huge, 0);
16891689
atomic_long_set(&rollover->num_failed, 0);
1690-
po->rollover = rollover;
16911690
}
16921691

16931692
if (type_flags & PACKET_FANOUT_FLAG_UNIQUEID) {
@@ -1745,6 +1744,8 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
17451744
if (refcount_read(&match->sk_ref) < PACKET_FANOUT_MAX) {
17461745
__dev_remove_pack(&po->prot_hook);
17471746
po->fanout = match;
1747+
po->rollover = rollover;
1748+
rollover = NULL;
17481749
refcount_set(&match->sk_ref, refcount_read(&match->sk_ref) + 1);
17491750
__fanout_link(sk, po);
17501751
err = 0;
@@ -1758,10 +1759,7 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
17581759
}
17591760

17601761
out:
1761-
if (err && rollover) {
1762-
kfree_rcu(rollover, rcu);
1763-
po->rollover = NULL;
1764-
}
1762+
kfree(rollover);
17651763
mutex_unlock(&fanout_mutex);
17661764
return err;
17671765
}
@@ -1785,11 +1783,6 @@ static struct packet_fanout *fanout_release(struct sock *sk)
17851783
list_del(&f->list);
17861784
else
17871785
f = NULL;
1788-
1789-
if (po->rollover) {
1790-
kfree_rcu(po->rollover, rcu);
1791-
po->rollover = NULL;
1792-
}
17931786
}
17941787
mutex_unlock(&fanout_mutex);
17951788

@@ -3029,6 +3022,7 @@ static int packet_release(struct socket *sock)
30293022
synchronize_net();
30303023

30313024
if (f) {
3025+
kfree(po->rollover);
30323026
fanout_release_data(f);
30333027
kfree(f);
30343028
}
@@ -3843,7 +3837,6 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
38433837
void *data = &val;
38443838
union tpacket_stats_u st;
38453839
struct tpacket_rollover_stats rstats;
3846-
struct packet_rollover *rollover;
38473840

38483841
if (level != SOL_PACKET)
38493842
return -ENOPROTOOPT;
@@ -3922,18 +3915,13 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
39223915
0);
39233916
break;
39243917
case PACKET_ROLLOVER_STATS:
3925-
rcu_read_lock();
3926-
rollover = rcu_dereference(po->rollover);
3927-
if (rollover) {
3928-
rstats.tp_all = atomic_long_read(&rollover->num);
3929-
rstats.tp_huge = atomic_long_read(&rollover->num_huge);
3930-
rstats.tp_failed = atomic_long_read(&rollover->num_failed);
3931-
data = &rstats;
3932-
lv = sizeof(rstats);
3933-
}
3934-
rcu_read_unlock();
3935-
if (!rollover)
3918+
if (!po->rollover)
39363919
return -EINVAL;
3920+
rstats.tp_all = atomic_long_read(&po->rollover->num);
3921+
rstats.tp_huge = atomic_long_read(&po->rollover->num_huge);
3922+
rstats.tp_failed = atomic_long_read(&po->rollover->num_failed);
3923+
data = &rstats;
3924+
lv = sizeof(rstats);
39373925
break;
39383926
case PACKET_TX_HAS_OFF:
39393927
val = po->tp_tx_has_off;

net/packet/internal.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@ struct packet_fanout {
9595

9696
struct packet_rollover {
9797
int sock;
98-
struct rcu_head rcu;
9998
atomic_long_t num;
10099
atomic_long_t num_huge;
101100
atomic_long_t num_failed;

0 commit comments

Comments
 (0)