[PATCH] mac802154: synchronize pending transmissions before stopping the hardware

[Adi Prasan]

From: Adi <itsadi2409@xxxxxxxxx>

syzbot reports a WARN_ON(current_phy->suspended) splat in
mac80211_hwsim's mac802154 counterpart, hwsim_hw_xmit():

  WARNING: CPU: 0 PID: 6038 at drivers/net/ieee802154/mac802154_hwsim.c:266
  hwsim_hw_xmit+0xcbf/0x1540
  Call Trace:
   drv_xmit_async
   ieee802154_tx
   ieee802154_subif_start_xmit
   __netdev_start_xmit
   ...
   packet_sendmsg

mac802154_slave_close() only calls netif_stop_queue() on the closing
netdev before tearing the hardware down via ieee802154_stop_device(),
which calls straight into drv_stop(). netif_stop_queue() merely marks
the queue as stopped; it does not wait for a transmission that a
different CPU already passed the "is the queue stopped" check for and
is currently executing inside ndo_start_xmit(). That thread can still
be inside drv_xmit_async() -> hwsim_hw_xmit() by the time drv_stop()
runs and marks the phy suspended, which trips the WARN_ON() (and, on
real hardware, could still start a transmission after ->stop() has
torn down the transceiver).

ieee802154_suspend() already knows about this and synchronizes with
any in-flight transmission via ieee802154_sync_and_hold_queue() plus
synchronize_net() before calling ieee802154_stop_device(), releasing
the queue again afterwards. mac802154_slave_close() is missing the
same synchronization when the last open interface on a phy is closed.

Do the same hold/sync/release dance in mac802154_slave_close() so
drv_stop() cannot race with a transmission that is already in flight.

Reported-by: syzbot+adb0bf16414b8bb2d799@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=adb0bf16414b8bb2d799
Signed-off-by: Adi <itsadi2409@xxxxxxxxx>
---
 net/mac802154/iface.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 31353795fa24..e29f225c0dd1 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -313,8 +313,19 @@ static int mac802154_slave_close(struct net_device *dev)
 
 	clear_bit(SDATA_STATE_RUNNING, &sdata->state);
 
-	if (!local->open_count)
+	if (!local->open_count) {
+		/* Ensure any asynchronous transmission already accepted by
+		 * the driver (i.e. past the netif_stop_queue() above) has
+		 * fully completed before calling into ->stop(). Otherwise
+		 * drv_xmit_async() can race with drv_stop(), which some
+		 * drivers (e.g. mac802154_hwsim) do not expect. This mirrors
+		 * the synchronization already done in ieee802154_suspend().
+		 */
+		ieee802154_sync_and_hold_queue(local);
+		synchronize_net();
 		ieee802154_stop_device(local);
+		ieee802154_release_queue(local);
+	}
 
 	return 0;
 }
-- 
2.43.0