Skip to content

Commit d199fab

Browse files
Eric Dumazetdavem330
Eric Dumazet
authored andcommitted
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: dc99f60 ("packet: Add fanout support.") Fixes: 0648ab7 ("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>
1 parent f39f0d1 commit d199fab

File tree

1 file changed

+30
-25
lines changed

1 file changed

+30
-25
lines changed

Diff for: net/packet/af_packet.c

+30-25
Original file line numberDiff line numberDiff line change
@@ -1619,6 +1619,7 @@ static void fanout_release_data(struct packet_fanout *f)
16191619

16201620
static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
16211621
{
1622+
struct packet_rollover *rollover = NULL;
16221623
struct packet_sock *po = pkt_sk(sk);
16231624
struct packet_fanout *f, *match;
16241625
u8 type = type_flags & 0xff;
@@ -1641,23 +1642,28 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
16411642
return -EINVAL;
16421643
}
16431644

1645+
mutex_lock(&fanout_mutex);
1646+
1647+
err = -EINVAL;
16441648
if (!po->running)
1645-
return -EINVAL;
1649+
goto out;
16461650

1651+
err = -EALREADY;
16471652
if (po->fanout)
1648-
return -EALREADY;
1653+
goto out;
16491654

16501655
if (type == PACKET_FANOUT_ROLLOVER ||
16511656
(type_flags & PACKET_FANOUT_FLAG_ROLLOVER)) {
1652-
po->rollover = kzalloc(sizeof(*po->rollover), GFP_KERNEL);
1653-
if (!po->rollover)
1654-
return -ENOMEM;
1655-
atomic_long_set(&po->rollover->num, 0);
1656-
atomic_long_set(&po->rollover->num_huge, 0);
1657-
atomic_long_set(&po->rollover->num_failed, 0);
1657+
err = -ENOMEM;
1658+
rollover = kzalloc(sizeof(*rollover), GFP_KERNEL);
1659+
if (!rollover)
1660+
goto out;
1661+
atomic_long_set(&rollover->num, 0);
1662+
atomic_long_set(&rollover->num_huge, 0);
1663+
atomic_long_set(&rollover->num_failed, 0);
1664+
po->rollover = rollover;
16581665
}
16591666

1660-
mutex_lock(&fanout_mutex);
16611667
match = NULL;
16621668
list_for_each_entry(f, &fanout_list, list) {
16631669
if (f->id == id &&
@@ -1704,11 +1710,11 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
17041710
}
17051711
}
17061712
out:
1707-
mutex_unlock(&fanout_mutex);
1708-
if (err) {
1709-
kfree(po->rollover);
1713+
if (err && rollover) {
1714+
kfree(rollover);
17101715
po->rollover = NULL;
17111716
}
1717+
mutex_unlock(&fanout_mutex);
17121718
return err;
17131719
}
17141720

@@ -1717,23 +1723,22 @@ static void fanout_release(struct sock *sk)
17171723
struct packet_sock *po = pkt_sk(sk);
17181724
struct packet_fanout *f;
17191725

1720-
f = po->fanout;
1721-
if (!f)
1722-
return;
1723-
17241726
mutex_lock(&fanout_mutex);
1725-
po->fanout = NULL;
1727+
f = po->fanout;
1728+
if (f) {
1729+
po->fanout = NULL;
1730+
1731+
if (atomic_dec_and_test(&f->sk_ref)) {
1732+
list_del(&f->list);
1733+
dev_remove_pack(&f->prot_hook);
1734+
fanout_release_data(f);
1735+
kfree(f);
1736+
}
17261737

1727-
if (atomic_dec_and_test(&f->sk_ref)) {
1728-
list_del(&f->list);
1729-
dev_remove_pack(&f->prot_hook);
1730-
fanout_release_data(f);
1731-
kfree(f);
1738+
if (po->rollover)
1739+
kfree_rcu(po->rollover, rcu);
17321740
}
17331741
mutex_unlock(&fanout_mutex);
1734-
1735-
if (po->rollover)
1736-
kfree_rcu(po->rollover, rcu);
17371742
}
17381743

17391744
static bool packet_extra_vlan_len_allowed(const struct net_device *dev,

0 commit comments

Comments
 (0)