diff options
| author | Eric Dumazet <edumazet@google.com> | 2022-01-07 10:39:53 -0800 |
|---|---|---|
| committer | Jakub Kicinski <kuba@kernel.org> | 2022-01-07 19:11:55 -0800 |
| commit | bf44077c1b3ae86668bce02d9466e7134a6569ec (patch) | |
| tree | c693e0915fe5444ecc81f982fbfbcbcb6ffe5db4 /net/packet | |
| parent | d8caa2ed47de0e55828a3bd0a81bbb81aa9e7e11 (diff) | |
af_packet: fix tracking issues in packet_do_bind()
It appears that my changes in packet_do_bind() were
slightly wrong.
syzbot found that calling bind() twice would trigger
a false positive.
Remove proto_curr/dev_curr variables and rewrite things
to be less confusing (like not having to use netdev_tracker_alloc(),
and instead use the standard dev_hold_track())
Fixes: f1d9268e0618 ("net: add net device refcount tracker to struct packet_type")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Link: https://lore.kernel.org/r/20220107183953.3886647-1-eric.dumazet@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Diffstat (limited to 'net/packet')
| -rw-r--r-- | net/packet/af_packet.c | 27 |
1 files changed, 8 insertions, 19 deletions
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 9bbe7282efb6..5bd409ab4cc2 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -3162,12 +3162,10 @@ static int packet_do_bind(struct sock *sk, const char *name, int ifindex, __be16 proto) { struct packet_sock *po = pkt_sk(sk); - struct net_device *dev_curr; - __be16 proto_curr; - bool need_rehook; struct net_device *dev = NULL; - int ret = 0; bool unlisted = false; + bool need_rehook; + int ret = 0; lock_sock(sk); spin_lock(&po->bind_lock); @@ -3192,14 +3190,10 @@ static int packet_do_bind(struct sock *sk, const char *name, int ifindex, } } - dev_hold(dev); - - proto_curr = po->prot_hook.type; - dev_curr = po->prot_hook.dev; - - need_rehook = proto_curr != proto || dev_curr != dev; + need_rehook = po->prot_hook.type != proto || po->prot_hook.dev != dev; if (need_rehook) { + dev_hold(dev); if (po->running) { rcu_read_unlock(); /* prevents packet_notifier() from calling @@ -3208,7 +3202,6 @@ static int packet_do_bind(struct sock *sk, const char *name, int ifindex, WRITE_ONCE(po->num, 0); __unregister_prot_hook(sk, true); rcu_read_lock(); - dev_curr = po->prot_hook.dev; if (dev) unlisted = !dev_get_by_index_rcu(sock_net(sk), dev->ifindex); @@ -3218,25 +3211,21 @@ static int packet_do_bind(struct sock *sk, const char *name, int ifindex, WRITE_ONCE(po->num, proto); po->prot_hook.type = proto; - dev_put_track(dev_curr, &po->prot_hook.dev_tracker); - dev_curr = NULL; + dev_put_track(po->prot_hook.dev, &po->prot_hook.dev_tracker); if (unlikely(unlisted)) { - dev_put(dev); po->prot_hook.dev = NULL; WRITE_ONCE(po->ifindex, -1); packet_cached_dev_reset(po); } else { - if (dev) - netdev_tracker_alloc(dev, - &po->prot_hook.dev_tracker, - GFP_ATOMIC); + dev_hold_track(dev, &po->prot_hook.dev_tracker, + GFP_ATOMIC); po->prot_hook.dev = dev; WRITE_ONCE(po->ifindex, dev ? dev->ifindex : 0); packet_cached_dev_assign(po, dev); } + dev_put(dev); } - dev_put_track(dev_curr, &po->prot_hook.dev_tracker); if (proto == 0 || !need_rehook) goto out_unlock; |