diff mbox

[3/5] batman-adv: Fix a potential bcast/ogm queue purging race condition (1)

Message ID 1363583397-4800-3-git-send-email-linus.luessing@web.de
State Superseded, archived
Delegated to: Marek Lindner
Headers

Commit Message

Linus Lüssing March 18, 2013, 5:09 a.m. UTC 
  - Perform queue list addition and work queueing atomically.

Otherwise we might potentially miss waiting for the re-armed task on
interface removal, leading to freeing packets with the re-armed tasks
trying to process them afterwards, therefore causing segmentation
faults.

Signed-off-by: Linus Lüssing <linus.luessing@web.de>
---
 bat_iv_ogm.c |   12 ++++++------
 send.c       |    6 ++----
 2 files changed, 8 insertions(+), 10 deletions(-)

Comments

Sven Eckelmann March 10, 2016, 5 p.m. UTC | #1 
On Monday 18 March 2013 06:09:55 Linus Lüssing wrote:
> - Perform queue list addition and work queueing atomically.
> 
> Otherwise we might potentially miss waiting for the re-armed task on
> interface removal, leading to freeing packets with the re-armed tasks
> trying to process them afterwards, therefore causing segmentation
> faults.
> 
> Signed-off-by: Linus Lüssing <linus.luessing@web.de>
> ---

It looks like this patch and the other two don't apply anymore. Can you please
resent them or mark them correctly in patchwork [1, 2, 3].

Thanks,
	Sven

[1] https://patchwork.open-mesh.org/patch/2798/
[2] https://patchwork.open-mesh.org/patch/2812/
[3] https://patchwork.open-mesh.org/patch/2786/
Sven Eckelmann April 9, 2016, 4:23 p.m. UTC | #2 
On Thursday 10 March 2016 18:00:58 Sven Eckelmann wrote:
[...]
> It looks like this patch and the other two don't apply anymore. Can you
> please resent them or mark them correctly in patchwork [1, 2, 3].
> 
> Thanks,
> 	Sven
> 
> [1] https://patchwork.open-mesh.org/patch/2798/
> [2] https://patchwork.open-mesh.org/patch/2812/
> [3] https://patchwork.open-mesh.org/patch/2786/

What is the state here?

Kind regards,
	Sven
Sven Eckelmann July 21, 2016, 10:27 p.m. UTC | #3 
On Samstag, 9. April 2016 18:23:56 CEST Sven Eckelmann wrote:
> On Thursday 10 March 2016 18:00:58 Sven Eckelmann wrote:
> [...]
> 
> > It looks like this patch and the other two don't apply anymore. Can you
> > please resent them or mark them correctly in patchwork [1, 2, 3].
> > 
> > Thanks,
> > 
> > 	Sven
> > 
> > [1] https://patchwork.open-mesh.org/patch/2798/
> > [2] https://patchwork.open-mesh.org/patch/2812/
> > [3] https://patchwork.open-mesh.org/patch/2786/
> 
> What is the state here?

What is the state of these three patches?

Kind regards,
	Sven
Linus Lüssing July 22, 2016, 11:46 a.m. UTC | #4 
On Fri, Jul 22, 2016 at 12:27:04AM +0200, Sven Eckelmann wrote:
> On Samstag, 9. April 2016 18:23:56 CEST Sven Eckelmann wrote:
> > On Thursday 10 March 2016 18:00:58 Sven Eckelmann wrote:
> > [...]
> > 
> > > It looks like this patch and the other two don't apply anymore. Can you
> > > please resent them or mark them correctly in patchwork [1, 2, 3].
> > > 
> > > Thanks,
> > > 
> > > 	Sven
> > > 
> > > [1] https://patchwork.open-mesh.org/patch/2798/
> > > [2] https://patchwork.open-mesh.org/patch/2812/
> > > [3] https://patchwork.open-mesh.org/patch/2786/
> > 
> > What is the state here?
> 
> What is the state of these three patches?
> 
> Kind regards,
> 	Sven

Hi Sven,

Looks like I'm still having this issue from time to time, even on
the master branch. Will try to refresh and rephrase them later.

Regards, Linus
diff mbox

Patch

diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index 77568ba..de3a4c1 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -422,17 +422,17 @@  static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff,
 	if (direct_link)
 		forw_packet_aggr->direct_link_flags |= 1;
 
-	/* add new packet to packet list */
-	spin_lock_bh(&bat_priv->forw_bat_list_lock);
-	hlist_add_head(&forw_packet_aggr->list, &bat_priv->forw_bat_list);
-	spin_unlock_bh(&bat_priv->forw_bat_list_lock);
-
-	/* start timer for this packet */
+	/* initialize job for this packet */
 	INIT_DELAYED_WORK(&forw_packet_aggr->delayed_work,
 			  batadv_send_outstanding_bat_ogm_packet);
+
+	/* add new packet to packet list and start its timer */
+	spin_lock_bh(&bat_priv->forw_bat_list_lock);
+	hlist_add_head(&forw_packet_aggr->list, &bat_priv->forw_bat_list);
 	queue_delayed_work(batadv_event_workqueue,
 			   &forw_packet_aggr->delayed_work,
 			   send_time - jiffies);
+	spin_unlock_bh(&bat_priv->forw_bat_list_lock);
 }
 
 /* aggregate a new packet into the existing ogm packet */
diff --git a/send.c b/send.c
index c151982..1ddfae7 100644
--- a/send.c
+++ b/send.c
@@ -217,14 +217,12 @@  _batadv_add_bcast_packet_to_list(struct batadv_priv *bat_priv,
 				 struct batadv_forw_packet *forw_packet,
 				 unsigned long send_time)
 {
-	/* add new packet to packet list */
+	/* add new packet to packet list and start its timer */
 	spin_lock_bh(&bat_priv->forw_bcast_list_lock);
 	hlist_add_head(&forw_packet->list, &bat_priv->forw_bcast_list);
-	spin_unlock_bh(&bat_priv->forw_bcast_list_lock);
-
-	/* start timer for this packet */
 	queue_delayed_work(batadv_event_workqueue, &forw_packet->delayed_work,
 			   send_time);
+	spin_unlock_bh(&bat_priv->forw_bcast_list_lock);
 }
 
 /* add a broadcast packet to the queue and setup timers. broadcast packets