* [PATCH] Revert "af-packet: Use existing netdev reference for bound sockets."
@ 2013-11-18 0:28 Salam Noureddine
2013-11-18 2:28 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Salam Noureddine @ 2013-11-18 0:28 UTC (permalink / raw)
To: David S. Miller, Daniel Borkmann, Willem de Bruijn, Phil Sutter,
Eric Dumazet, netdev
Cc: Salam Noureddine
This reverts commit 827d978037d7d0bf0860481948c6d26ead10042f.
The patch introduced a race condition between packet_snd and packet_notifier
when a net_device is being unregistered. In the case of a bound socket,
packet_notifier can drop the last reference to the net_device and packet_snd
might be sending a packet over a freed net_device.
---
net/packet/af_packet.c | 27 ++++++++++++---------------
1 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2e8286b..a7335ae 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2057,8 +2057,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
struct sk_buff *skb;
struct net_device *dev;
__be16 proto;
- bool need_rls_dev = false;
- int err, reserve = 0;
+ int ifindex, err, reserve = 0;
void *ph;
struct sockaddr_ll *saddr = (struct sockaddr_ll *)msg->msg_name;
int tp_len, size_max;
@@ -2070,7 +2069,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
mutex_lock(&po->pg_vec_lock);
if (saddr == NULL) {
- dev = po->prot_hook.dev;
+ ifindex = po->ifindex;
proto = po->num;
addr = NULL;
} else {
@@ -2081,12 +2080,12 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
+ offsetof(struct sockaddr_ll,
sll_addr)))
goto out;
+ ifindex = saddr->sll_ifindex;
proto = saddr->sll_protocol;
addr = saddr->sll_addr;
- dev = dev_get_by_index(sock_net(&po->sk), saddr->sll_ifindex);
- need_rls_dev = true;
}
+ dev = dev_get_by_index(sock_net(&po->sk), ifindex);
err = -ENXIO;
if (unlikely(dev == NULL))
goto out;
@@ -2173,8 +2172,7 @@ out_status:
__packet_set_status(po, ph, status);
kfree_skb(skb);
out_put:
- if (need_rls_dev)
- dev_put(dev);
+ dev_put(dev);
out:
mutex_unlock(&po->pg_vec_lock);
return err;
@@ -2212,9 +2210,8 @@ static int packet_snd(struct socket *sock,
struct sk_buff *skb;
struct net_device *dev;
__be16 proto;
- bool need_rls_dev = false;
unsigned char *addr;
- int err, reserve = 0;
+ int ifindex, err, reserve = 0;
struct virtio_net_hdr vnet_hdr = { 0 };
int offset = 0;
int vnet_hdr_len;
@@ -2228,7 +2225,7 @@ static int packet_snd(struct socket *sock,
*/
if (saddr == NULL) {
- dev = po->prot_hook.dev;
+ ifindex = po->ifindex;
proto = po->num;
addr = NULL;
} else {
@@ -2237,12 +2234,13 @@ static int packet_snd(struct socket *sock,
goto out;
if (msg->msg_namelen < (saddr->sll_halen + offsetof(struct sockaddr_ll, sll_addr)))
goto out;
+ ifindex = saddr->sll_ifindex;
proto = saddr->sll_protocol;
addr = saddr->sll_addr;
- dev = dev_get_by_index(sock_net(sk), saddr->sll_ifindex);
- need_rls_dev = true;
}
+
+ dev = dev_get_by_index(sock_net(sk), ifindex);
err = -ENXIO;
if (dev == NULL)
goto out_unlock;
@@ -2386,15 +2384,14 @@ static int packet_snd(struct socket *sock,
if (err > 0 && (err = net_xmit_errno(err)) != 0)
goto out_unlock;
- if (need_rls_dev)
- dev_put(dev);
+ dev_put(dev);
return len;
out_free:
kfree_skb(skb);
out_unlock:
- if (dev && need_rls_dev)
+ if (dev)
dev_put(dev);
out:
return err;
--
1.7.4.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Revert "af-packet: Use existing netdev reference for bound sockets."
2013-11-18 0:28 [PATCH] Revert "af-packet: Use existing netdev reference for bound sockets." Salam Noureddine
@ 2013-11-18 2:28 ` David Miller
2013-11-18 5:06 ` Salam Noureddine
0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2013-11-18 2:28 UTC (permalink / raw)
To: noureddine; +Cc: dborkman, willemb, phil, edumazet, netdev
From: Salam Noureddine <noureddine@aristanetworks.com>
Date: Sun, 17 Nov 2013 16:28:42 -0800
> This reverts commit 827d978037d7d0bf0860481948c6d26ead10042f.
>
> The patch introduced a race condition between packet_snd and packet_notifier
> when a net_device is being unregistered. In the case of a bound socket,
> packet_notifier can drop the last reference to the net_device and packet_snd
> might be sending a packet over a freed net_device.
You need to provide a proper "Signed-off-by: ", you need to refer to
commits not just buy SHA1 ID but also by the commit header text in
parenthesis and double quotes.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Revert "af-packet: Use existing netdev reference for bound sockets."
2013-11-18 2:28 ` David Miller
@ 2013-11-18 5:06 ` Salam Noureddine
0 siblings, 0 replies; 3+ messages in thread
From: Salam Noureddine @ 2013-11-18 5:06 UTC (permalink / raw)
To: David Miller
Cc: Daniel Borkmann, Willem de Bruijn, Phil Sutter, Eric Dumazet,
netdev
Will fix it. Thanks.
On Sun, Nov 17, 2013 at 6:28 PM, David Miller <davem@davemloft.net> wrote:
> From: Salam Noureddine <noureddine@aristanetworks.com>
> Date: Sun, 17 Nov 2013 16:28:42 -0800
>
>> This reverts commit 827d978037d7d0bf0860481948c6d26ead10042f.
>>
>> The patch introduced a race condition between packet_snd and packet_notifier
>> when a net_device is being unregistered. In the case of a bound socket,
>> packet_notifier can drop the last reference to the net_device and packet_snd
>> might be sending a packet over a freed net_device.
>
> You need to provide a proper "Signed-off-by: ", you need to refer to
> commits not just buy SHA1 ID but also by the commit header text in
> parenthesis and double quotes.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-11-18 5:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-18 0:28 [PATCH] Revert "af-packet: Use existing netdev reference for bound sockets." Salam Noureddine
2013-11-18 2:28 ` David Miller
2013-11-18 5:06 ` Salam Noureddine
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).