[PATCH net v2] net: wwan: qcom_bam_dmux: fix TX DMA channel use-after-free

[PATCH net v2] net: wwan: qcom_bam_dmux: fix TX DMA channel use-after-free

[Hongyan Xu]

The modem power-control interrupt can currently call bam_dmux_power_off()
and release dmux->tx while the host side still has an active runtime PM
vote and is preparing or issuing TX DMA descriptors. Runtime PM prevents
the runtime suspend callback from running in that window, but it does not
serialize the modem-driven pc interrupt with the command, netdev transmit,
or deferred wakeup paths that use dmux->tx.

Serialize power-control state with a mutex and track the host pc vote under
that lock. If the modem reports pc=false while the host vote is still
active, acknowledge the pc transition but keep the DMA channels allocated.
This avoids releasing the TX channel underneath active users and avoids
terminating already queued commands such as BAM_DMUX_CMD_OPEN.

Synchronize the pc interrupt around the runtime-resume pc-ack sequence.
This makes a delayed pc=false interrupt run before runtime resume decides
that the remote side is up and requests the TX channel.

This issue was found by the author's in-house static analysis tool.
The patch was reviewed by the author.

Fixes: 21a0ffd9b38c ("net: wwan: Add Qualcomm BAM-DMUX WWAN network driver")
Cc: stable@vger.kernel.org
Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
---
v2:
- Drop the SRCU-based TX channel publishing from v1.
- Keep DMA channels allocated while the host pc vote is active, so a
  delayed pc=false IRQ cannot terminate queued TX commands or descriptors.
- Serialize pc IRQ handling with runtime PM power transitions.
- Post as a new netdev thread, as requested by
  Documentation/process/maintainer-netdev.rst.

v1: https://lore.kernel.org/netdev/20260806060642.1281-1-getshell@seu.edu.cn/

 drivers/net/wwan/qcom_bam_dmux.c | 81 +++++++++++++++++++++++++++-----
 1 file changed, 68 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wwan/qcom_bam_dmux.c b/drivers/net/wwan/qcom_bam_dmux.c
index cc6ace8..91b13b9 100644
--- a/drivers/net/wwan/qcom_bam_dmux.c
+++ b/drivers/net/wwan/qcom_bam_dmux.c
@@ -12,6 +12,7 @@
 #include <linux/if_arp.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/netdevice.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
@@ -73,6 +74,8 @@ struct bam_dmux {
 	u32 pc_mask, pc_ack_mask;
 	wait_queue_head_t pc_wait;
 	struct completion pc_ack_completion;
+	struct mutex power_lock; /* Protect power-control state */
+	bool pc_vote;
 
 	struct dma_chan *rx, *tx;
 	struct bam_dmux_skb_dma rx_skbs[BAM_DMUX_NUM_SKB];
@@ -99,6 +102,21 @@ static void bam_dmux_pc_vote(struct bam_dmux *dmux, bool enable)
 				    enable ? dmux->pc_mask : 0);
 }
 
+static void bam_dmux_pc_vote_locked(struct bam_dmux *dmux, bool enable)
+{
+	lockdep_assert_held(&dmux->power_lock);
+
+	dmux->pc_vote = enable;
+	bam_dmux_pc_vote(dmux, enable);
+}
+
+static void bam_dmux_pc_vote_protected(struct bam_dmux *dmux, bool enable)
+{
+	mutex_lock(&dmux->power_lock);
+	bam_dmux_pc_vote_locked(dmux, enable);
+	mutex_unlock(&dmux->power_lock);
+}
+
 static void bam_dmux_pc_ack(struct bam_dmux *dmux)
 {
 	qcom_smem_state_update_bits(dmux->pc_ack, dmux->pc_ack_mask,
@@ -655,6 +673,8 @@ static void bam_dmux_free_skbs(struct bam_dmux_skb_dma skbs[],
 
 static void bam_dmux_power_off(struct bam_dmux *dmux)
 {
+	lockdep_assert_held(&dmux->power_lock);
+
 	if (dmux->tx) {
 		dmaengine_terminate_sync(dmux->tx);
 		dma_release_channel(dmux->tx);
@@ -673,21 +693,28 @@ static void bam_dmux_power_off(struct bam_dmux *dmux)
 static irqreturn_t bam_dmux_pc_irq(int irq, void *data)
 {
 	struct bam_dmux *dmux = data;
-	bool new_state = !dmux->pc_state;
+	bool new_state;
+
+	mutex_lock(&dmux->power_lock);
+	new_state = !dmux->pc_state;
 
 	dev_dbg(dmux->dev, "pc: %u\n", new_state);
 
 	if (new_state) {
-		if (bam_dmux_power_on(dmux))
+		if (dmux->rx || bam_dmux_power_on(dmux))
 			bam_dmux_pc_ack(dmux);
 		else
 			bam_dmux_power_off(dmux);
+	} else if (dmux->pc_vote) {
+		dev_dbg(dmux->dev, "pc down while host vote is active\n");
+		bam_dmux_pc_ack(dmux);
 	} else {
 		bam_dmux_power_off(dmux);
 		bam_dmux_pc_ack(dmux);
 	}
 
-	dmux->pc_state = new_state;
+	WRITE_ONCE(dmux->pc_state, new_state);
+	mutex_unlock(&dmux->power_lock);
 	wake_up_all(&dmux->pc_wait);
 
 	return IRQ_HANDLED;
@@ -708,7 +735,12 @@ static int bam_dmux_runtime_suspend(struct device *dev)
 	struct bam_dmux *dmux = dev_get_drvdata(dev);
 
 	dev_dbg(dev, "runtime suspend\n");
-	bam_dmux_pc_vote(dmux, false);
+
+	mutex_lock(&dmux->power_lock);
+	bam_dmux_pc_vote_locked(dmux, false);
+	if (!dmux->pc_state)
+		bam_dmux_power_off(dmux);
+	mutex_unlock(&dmux->power_lock);
 
 	return 0;
 }
@@ -724,40 +756,55 @@ static int __maybe_unused bam_dmux_runtime_resume(struct device *dev)
 					 BAM_DMUX_REMOTE_TIMEOUT))
 		return -ETIMEDOUT;
 
+	synchronize_irq(dmux->pc_irq);
+
 	/* Vote for power state */
-	bam_dmux_pc_vote(dmux, true);
+	bam_dmux_pc_vote_protected(dmux, true);
 
 	/* Wait for ack */
 	if (!wait_for_completion_timeout(&dmux->pc_ack_completion,
 					 BAM_DMUX_REMOTE_TIMEOUT)) {
-		bam_dmux_pc_vote(dmux, false);
+		bam_dmux_runtime_suspend(dev);
 		return -ETIMEDOUT;
 	}
 
+	synchronize_irq(dmux->pc_irq);
+
 	/* Wait until we're up */
-	if (!wait_event_timeout(dmux->pc_wait, dmux->pc_state,
+	if (!wait_event_timeout(dmux->pc_wait, READ_ONCE(dmux->pc_state),
 				BAM_DMUX_REMOTE_TIMEOUT)) {
-		bam_dmux_pc_vote(dmux, false);
+		bam_dmux_runtime_suspend(dev);
 		return -ETIMEDOUT;
 	}
 
+	mutex_lock(&dmux->power_lock);
+
 	/* Ensure that we actually initialized successfully */
 	if (!dmux->rx) {
-		bam_dmux_pc_vote(dmux, false);
+		bam_dmux_pc_vote_locked(dmux, false);
+		if (!dmux->pc_state)
+			bam_dmux_power_off(dmux);
+		mutex_unlock(&dmux->power_lock);
 		return -ENXIO;
 	}
 
 	/* Request TX channel if necessary */
-	if (dmux->tx)
+	if (dmux->tx) {
+		mutex_unlock(&dmux->power_lock);
 		return 0;
+	}
 
 	dmux->tx = dma_request_chan(dev, "tx");
 	if (IS_ERR(dmux->tx)) {
 		dev_err(dev, "Failed to request TX DMA channel: %pe\n", dmux->tx);
 		dmux->tx = NULL;
-		bam_dmux_runtime_suspend(dev);
+		bam_dmux_pc_vote_locked(dmux, false);
+		if (!dmux->pc_state)
+			bam_dmux_power_off(dmux);
+		mutex_unlock(&dmux->power_lock);
 		return -ENXIO;
 	}
+	mutex_unlock(&dmux->power_lock);
 
 	return 0;
 }
@@ -799,6 +846,7 @@ static int bam_dmux_probe(struct platform_device *pdev)
 	init_waitqueue_head(&dmux->pc_wait);
 	init_completion(&dmux->pc_ack_completion);
 	complete_all(&dmux->pc_ack_completion);
+	mutex_init(&dmux->power_lock);
 
 	spin_lock_init(&dmux->tx_lock);
 	INIT_WORK(&dmux->tx_wakeup_work, bam_dmux_tx_wakeup_work);
@@ -827,10 +875,11 @@ static int bam_dmux_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_disable_pm;
 
+	mutex_lock(&dmux->power_lock);
 	ret = irq_get_irqchip_state(dmux->pc_irq, IRQCHIP_STATE_LINE_LEVEL,
 				    &dmux->pc_state);
 	if (ret)
-		goto err_disable_pm;
+		goto err_unlock_power;
 
 	/* Check if remote finished initialization before us */
 	if (dmux->pc_state) {
@@ -839,9 +888,12 @@ static int bam_dmux_probe(struct platform_device *pdev)
 		else
 			bam_dmux_power_off(dmux);
 	}
+	mutex_unlock(&dmux->power_lock);
 
 	return 0;
 
+err_unlock_power:
+	mutex_unlock(&dmux->power_lock);
 err_disable_pm:
 	pm_runtime_disable(dev);
 	pm_runtime_dont_use_autosuspend(dev);
@@ -872,12 +924,15 @@ static void bam_dmux_remove(struct platform_device *pdev)
 	pm_runtime_set_suspended(dev);
 
 	/* Try to wait for remote side to drop power vote */
-	if (!wait_event_timeout(dmux->pc_wait, !dmux->rx, BAM_DMUX_REMOTE_TIMEOUT))
+	if (!wait_event_timeout(dmux->pc_wait, !READ_ONCE(dmux->rx),
+				BAM_DMUX_REMOTE_TIMEOUT))
 		dev_err(dev, "Timed out waiting for remote side to suspend\n");
 
 	/* Make sure everything is cleaned up before we return */
 	disable_irq(dmux->pc_irq);
+	mutex_lock(&dmux->power_lock);
 	bam_dmux_power_off(dmux);
+	mutex_unlock(&dmux->power_lock);
 	bam_dmux_free_skbs(dmux->tx_skbs, DMA_TO_DEVICE);
 }
 
-- 
2.50.1.windows.1

Re: [PATCH net v2] net: wwan: qcom_bam_dmux: fix TX DMA channel use-after-free

[Stephan Gerhold]

On Sat, Aug 08, 2026 at 04:34:57PM +0800, Hongyan Xu wrote:
> The modem power-control interrupt can currently call bam_dmux_power_off()
> and release dmux->tx while the host side still has an active runtime PM
> vote and is preparing or issuing TX DMA descriptors. Runtime PM prevents
> the runtime suspend callback from running in that window, but it does not
> serialize the modem-driven pc interrupt with the command, netdev transmit,
> or deferred wakeup paths that use dmux->tx.
> 
> Serialize power-control state with a mutex and track the host pc vote under
> that lock. If the modem reports pc=false while the host vote is still
> active, acknowledge the pc transition but keep the DMA channels allocated.
> This avoids releasing the TX channel underneath active users and avoids
> terminating already queued commands such as BAM_DMUX_CMD_OPEN.
> 

Unfortunately, this version won't work in practice because the modem
will power down the DMA engine as soon as we acknowledge the pc
transition. We need to release the TX channel (and bring the DMA engine
into clean reset state) before sending the pc-ack (or refuse sending the
pc-ack if the modem firmware is broken). The whole state management is
unfortunately very tricky as I wrote in v1 [1].

Thanks,
Stephan

[1]: https://lore.kernel.org/linux-arm-msm/anSzDNYf0AMW7U9Y@linaro.org/