diff mbox

net/802/mrp: fix possible race condition when calling mrp_pdu_queue()

Message ID 1365724035-30220-1-git-send-email-david.ward@ll.mit.edu
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

David Ward April 11, 2013, 11:47 p.m. UTC
(Adapted from a very similar change to net/802/garp.c by Cong Wang.)

mrp_pdu_queue() should ways be called with the applicant spin lock.
mrp_uninit_applicant() only holds the rtnl lock which is not enough;
a race is possible because mrp_rcv() is called in BH context:

	mrp_rcv()
	  |->mrp_pdu_parse_msg()
	    |->mrp_pdu_parse_vecattr()
	      |->mrp_pdu_parse_vecattr_event()
	        |-> mrp_attr_event()
	          |-> mrp_pdu_append_vecattr_event()
	            |-> mrp_pdu_queue()

Cc: Cong Wang <amwang@redhat.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
 net/802/mrp.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

Comments

Amerigo Wang April 12, 2013, 8:18 a.m. UTC | #1
----- Original Message -----
> (Adapted from a very similar change to net/802/garp.c by Cong Wang.)
> 
> mrp_pdu_queue() should ways be called with the applicant spin lock.
> mrp_uninit_applicant() only holds the rtnl lock which is not enough;
> a race is possible because mrp_rcv() is called in BH context:
> 
> 	mrp_rcv()
> 	  |->mrp_pdu_parse_msg()
> 	    |->mrp_pdu_parse_vecattr()
> 	      |->mrp_pdu_parse_vecattr_event()
> 	        |-> mrp_attr_event()
> 	          |-> mrp_pdu_append_vecattr_event()
> 	            |-> mrp_pdu_queue()
> 
> Cc: Cong Wang <amwang@redhat.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: David Ward <david.ward@ll.mit.edu>

Acked-by: Cong Wang <amwang@redhat.com>

Thanks.
--
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
David Miller April 12, 2013, 7:11 p.m. UTC | #2
From: David Ward <david.ward@ll.mit.edu>
Date: Thu, 11 Apr 2013 19:47:15 -0400

> (Adapted from a very similar change to net/802/garp.c by Cong Wang.)
> 
> mrp_pdu_queue() should ways be called with the applicant spin lock.
> mrp_uninit_applicant() only holds the rtnl lock which is not enough;
> a race is possible because mrp_rcv() is called in BH context:
> 
> 	mrp_rcv()
> 	  |->mrp_pdu_parse_msg()
> 	    |->mrp_pdu_parse_vecattr()
> 	      |->mrp_pdu_parse_vecattr_event()
> 	        |-> mrp_attr_event()
> 	          |-> mrp_pdu_append_vecattr_event()
> 	            |-> mrp_pdu_queue()
> 
> Cc: Cong Wang <amwang@redhat.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: David Ward <david.ward@ll.mit.edu>

Applied and queued up for -stable, thanks.
--
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/net/802/mrp.c b/net/802/mrp.c
index a4cc322..e085bcc 100644
--- a/net/802/mrp.c
+++ b/net/802/mrp.c
@@ -870,8 +870,12 @@  void mrp_uninit_applicant(struct net_device *dev, struct mrp_application *appl)
 	 * all pending messages before the applicant is gone.
 	 */
 	del_timer_sync(&app->join_timer);
+
+	spin_lock(&app->lock);
 	mrp_mad_event(app, MRP_EVENT_TX);
 	mrp_pdu_queue(app);
+	spin_unlock(&app->lock);
+
 	mrp_queue_xmit(app);
 
 	dev_mc_del(dev, appl->group_address);