[PATCH net v2] mac802154: lock rx_mac_cmd_list and drain it before freeing sdata

@@ -74,6 +74,10 @@ struct ieee802154_local {
 	struct work_struct rx_beacon_work;
 	struct list_head rx_mac_cmd_list;
 	struct work_struct rx_mac_cmd_work;
+	/* protects rx_mac_cmd_list against the RX softirq producer, the
+	 * mac_wq worker and the teardown flush running concurrently
+	 */
+	spinlock_t rx_mac_cmd_lock;
 
 	/* Association */
 	struct ieee802154_pan_device *assoc_dev;

@@ -300,6 +304,8 @@ static inline bool mac802154_is_beaconing(struct ieee802154_local *local)
 }
 
 void mac802154_rx_mac_cmd_worker(struct work_struct *work);
+void mac802154_flush_queued_mac_cmds(struct ieee802154_local *local,
+				     struct ieee802154_sub_if_data *sdata);
 
 int mac802154_perform_association(struct ieee802154_sub_if_data *sdata,
 				  struct ieee802154_pan_device *coord,

@@ -694,6 +694,13 @@ void ieee802154_if_remove(struct ieee802154_sub_if_data *sdata)
 	mutex_unlock(&sdata->local->iflist_mtx);
 
 	synchronize_rcu();
+
+	/* After the grace period no new rx_mac_cmd_list entry can point at
+	 * @sdata; drop the ones already queued before it is freed below, as
+	 * the mac_wq worker derefs mac_pkt->sdata with no liveness check.
+	 */
+	mac802154_flush_queued_mac_cmds(sdata->local, sdata);
+
 	unregister_netdevice(sdata->dev);
 }
 

@@ -91,6 +91,7 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
 	INIT_LIST_HEAD(&local->interfaces);
 	INIT_LIST_HEAD(&local->rx_beacon_list);
 	INIT_LIST_HEAD(&local->rx_mac_cmd_list);
+	spin_lock_init(&local->rx_mac_cmd_lock);
 	mutex_init(&local->iflist_mtx);
 
 	tasklet_setup(&local->tasklet, ieee802154_tasklet_handler);

@@ -277,6 +278,12 @@ void ieee802154_unregister_hw(struct ieee802154_hw *hw)
 	tasklet_kill(&local->tasklet);
 	flush_workqueue(local->workqueue);
 
+	/* Drain rx_mac_cmd_list before ieee802154_remove_interfaces()
+	 * frees every sdata: the mac_wq worker derefs mac_pkt->sdata and
+	 * is not covered by the flush above (that is the data workqueue).
+	 */
+	mac802154_flush_queued_mac_cmds(local, NULL);
+
 	rtnl_lock();
 
 	ieee802154_remove_interfaces(local);

@@ -76,8 +76,12 @@ void mac802154_rx_mac_cmd_worker(struct work_struct *work)
 	u8 mac_cmd;
 	int rc;
 
+	spin_lock_bh(&local->rx_mac_cmd_lock);
 	mac_pkt = list_first_entry_or_null(&local->rx_mac_cmd_list,
 					   struct cfg802154_mac_pkt, node);
+	if (mac_pkt)
+		list_del(&mac_pkt->node);
+	spin_unlock_bh(&local->rx_mac_cmd_lock);
 	if (!mac_pkt)
 		return;
 

@@ -123,11 +127,48 @@ void mac802154_rx_mac_cmd_worker(struct work_struct *work)
 	}
 
 out:
-	list_del(&mac_pkt->node);
 	kfree_skb(mac_pkt->skb);
 	kfree(mac_pkt);
 }
 
+/**
+ * mac802154_flush_queued_mac_cmds - drop pending rx_mac_cmd_list work
+ * @local: the mac802154 device the queue belongs to
+ * @sdata: interface being torn down, or %NULL to flush unconditionally
+ *
+ * Each queued &struct cfg802154_mac_pkt stashes a raw pointer to the
+ * interface it was received on, which mac802154_rx_mac_cmd_worker() later
+ * dereferences without checking whether that interface is still alive.
+ * Callers must invoke this before freeing @sdata (or before freeing every
+ * interface, when @sdata is %NULL) so the worker never runs against freed
+ * memory. The list is drained under rx_mac_cmd_lock, then cancel_work_sync()
+ * waits out a run that already dequeued a matching packet before the drain.
+ */
+void mac802154_flush_queued_mac_cmds(struct ieee802154_local *local,
+				     struct ieee802154_sub_if_data *sdata)
+{
+	struct cfg802154_mac_pkt *mac_pkt, *tmp;
+
+	spin_lock_bh(&local->rx_mac_cmd_lock);
+	list_for_each_entry_safe(mac_pkt, tmp, &local->rx_mac_cmd_list, node) {
+		if (sdata && mac_pkt->sdata != sdata)
+			continue;
+
+		list_del(&mac_pkt->node);
+		kfree_skb(mac_pkt->skb);
+		kfree(mac_pkt);
+	}
+	spin_unlock_bh(&local->rx_mac_cmd_lock);
+
+	cancel_work_sync(&local->rx_mac_cmd_work);
+
+	/* Other interfaces on @local may still have entries pending. */
+	spin_lock_bh(&local->rx_mac_cmd_lock);
+	if (!list_empty(&local->rx_mac_cmd_list))
+		queue_work(local->mac_wq, &local->rx_mac_cmd_work);
+	spin_unlock_bh(&local->rx_mac_cmd_lock);
+}
+
 static int
 ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
 		       struct sk_buff *skb, const struct ieee802154_hdr *hdr)

@@ -233,7 +274,11 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
 
 		mac_pkt->skb = skb_get(skb);
 		mac_pkt->sdata = sdata;
+
+		spin_lock(&sdata->local->rx_mac_cmd_lock);
 		list_add_tail(&mac_pkt->node, &sdata->local->rx_mac_cmd_list);
+		spin_unlock(&sdata->local->rx_mac_cmd_lock);
+
 		queue_work(sdata->local->mac_wq, &sdata->local->rx_mac_cmd_work);
 		return NET_RX_SUCCESS;