diff mbox

[net] macvlan: fix a race on port dismantle and possible skb leaks

Message ID 1414032226.2094.14.camel@edumazet-glaptop2.roam.corp.google.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Dumazet Oct. 23, 2014, 2:43 a.m. UTC 
From: Eric Dumazet <edumazet@google.com>

We need to cancel the work queue after rcu grace period,
otherwise it can be rescheduled by incoming packets.

We need to purge queue if some skbs are still in it.

We can use __skb_queue_head_init() variant in
macvlan_process_broadcast()

Signed-off-by: Eric Dumazet <edumazet@google.com>
Fixes: 412ca1550cbec ("macvlan: Move broadcasts into a work queue")
Cc: Herbert Xu <herbert@gondor.apana.org.au>
---
 drivers/net/macvlan.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Herbert Xu Oct. 23, 2014, 3:28 a.m. UTC | #1 
On Wed, Oct 22, 2014 at 07:43:46PM -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> We need to cancel the work queue after rcu grace period,
> otherwise it can be rescheduled by incoming packets.
> 
> We need to purge queue if some skbs are still in it.
> 
> We can use __skb_queue_head_init() variant in
> macvlan_process_broadcast()
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Fixes: 412ca1550cbec ("macvlan: Move broadcasts into a work queue")
> Cc: Herbert Xu <herbert@gondor.apana.org.au>

Good catch! Your fix looks good to me.

Thanks,
David Miller Oct. 25, 2014, 8:24 p.m. UTC | #2 
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 23 Oct 2014 11:28:58 +0800

> On Wed, Oct 22, 2014 at 07:43:46PM -0700, Eric Dumazet wrote:
>> From: Eric Dumazet <edumazet@google.com>
>> 
>> We need to cancel the work queue after rcu grace period,
>> otherwise it can be rescheduled by incoming packets.
>> 
>> We need to purge queue if some skbs are still in it.
>> 
>> We can use __skb_queue_head_init() variant in
>> macvlan_process_broadcast()
>> 
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Fixes: 412ca1550cbec ("macvlan: Move broadcasts into a work queue")
>> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> 
> Good catch! Your fix looks good to me.

Applied and queued up for -stable, thanks everyone.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 29b3bb410781..98c2732755ea 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -272,7 +272,7 @@  static void macvlan_process_broadcast(struct work_struct *w)
 	struct sk_buff *skb;
 	struct sk_buff_head list;
 
-	skb_queue_head_init(&list);
+	__skb_queue_head_init(&list);
 
 	spin_lock_bh(&port->bc_queue.lock);
 	skb_queue_splice_tail_init(&port->bc_queue, &list);
@@ -1082,9 +1082,15 @@  static void macvlan_port_destroy(struct net_device *dev)
 {
 	struct macvlan_port *port = macvlan_port_get_rtnl(dev);
 
-	cancel_work_sync(&port->bc_work);
 	dev->priv_flags &= ~IFF_MACVLAN_PORT;
 	netdev_rx_handler_unregister(dev);
+
+	/* After this point, no packet can schedule bc_work anymore,
+	 * but we need to cancel it and purge left skbs if any.
+	 */
+	cancel_work_sync(&port->bc_work);
+	__skb_queue_purge(&port->bc_queue);
+
 	kfree_rcu(port, rcu);
 }