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

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

[Hongyan Xu]

The modem power-control interrupt can release dmux->tx while the command,
netdev transmit, or deferred wakeup paths are preparing and submitting
DMA descriptors. A runtime PM reference alone does not order those paths
against the modem-driven interrupt.

The pc and pc-ack IRQ actions do not encode the modem protocol order. A
pc=false edge raised before a new host power vote is acknowledged can
therefore be handled after runtime resume observes the acknowledgment and
starts using TX. On the other hand, acknowledging a new pc=false transition
while the host vote remains active lets the modem power down the DMA engine
underneath the driver.

Serialize power-control state with a mutex. Before publishing a host-vote
acknowledgment, sample the actual pc line and process any delayed
transition.
When pc goes low while the host vote is active and the TX channel is
allocated, defer the transition without acknowledging it. Repeated pc=false
interrupts remain deferred and cannot release the channel. If pc returns
high, cancel the deferred transition.

Runtime suspend drops the host vote under the same lock. If a pc=false
transition was deferred, it then terminates and releases the DMA channels
before acknowledging the transition. If runtime suspend wins the race with
the pc interrupt, the interrupt observes the inactive host vote and can
safely perform the same shutdown. This orders channel release after the
last runtime PM user without relying on whether the modem acknowledges
the host vote before or after asserting pc.

Keep both state IRQs disabled until the initial probe state is committed,
and disable both before final remove cleanup. Use a device-managed mutex
and scoped guards for the serialized paths.

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>

---
Changes in v5:
- Drop the vote-ack state; PC-down safety does not depend on ACK ordering.
- Keep repeated PC-low interrupts deferred while TX can still be in use.
- Complete a deferred PC-down directly from runtime suspend after dropping
  the host vote.
- Move TX channel acquisition into a scoped helper and retain early returns.

Link: https://lore.kernel.org/netdev/20260822085308.1089-1-getshell@seu.edu.cn/

diff --git a/drivers/net/wwan/qcom_bam_dmux.c b/drivers/net/wwan/qcom_bam_dmux.c
index cc6ace8..429d34a 100644
--- a/drivers/net/wwan/qcom_bam_dmux.c
+++ b/drivers/net/wwan/qcom_bam_dmux.c
@@ -6,12 +6,14 @@
 
 #include <linux/atomic.h>
 #include <linux/bitops.h>
+#include <linux/cleanup.h>
 #include <linux/completion.h>
 #include <linux/dma-mapping.h>
 #include <linux/dmaengine.h>
 #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>
@@ -64,15 +66,23 @@ struct bam_dmux_skb_dma {
 	dma_addr_t addr;
 };
 
+enum bam_dmux_pc_vote {
+	BAM_DMUX_PC_VOTE_INACTIVE, /* pc_vote = false */
+	BAM_DMUX_PC_VOTE_ACTIVE, /* pc_vote = true */
+	BAM_DMUX_PC_VOTE_DOWN_PENDING, /* pc = false while host vote active */
+};
+
 struct bam_dmux {
 	struct device *dev;
 
-	int pc_irq;
+	int pc_irq, pc_ack_irq;
 	bool pc_state, pc_ack_state;
 	struct qcom_smem_state *pc, *pc_ack;
 	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 */
+	enum bam_dmux_pc_vote pc_vote;
 
 	struct dma_chan *rx, *tx;
 	struct bam_dmux_skb_dma rx_skbs[BAM_DMUX_NUM_SKB];
@@ -99,6 +109,24 @@ 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);
+
+	if (enable)
+		dmux->pc_vote = BAM_DMUX_PC_VOTE_ACTIVE;
+	else if (dmux->pc_vote != BAM_DMUX_PC_VOTE_DOWN_PENDING)
+		dmux->pc_vote = BAM_DMUX_PC_VOTE_INACTIVE;
+	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 +683,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);
@@ -670,10 +700,24 @@ static void bam_dmux_power_off(struct bam_dmux *dmux)
 	bam_dmux_free_skbs(dmux->rx_skbs, DMA_FROM_DEVICE);
 }
 
-static irqreturn_t bam_dmux_pc_irq(int irq, void *data)
+static void bam_dmux_complete_pc_down(struct bam_dmux *dmux)
 {
-	struct bam_dmux *dmux = data;
-	bool new_state = !dmux->pc_state;
+	lockdep_assert_held(&dmux->power_lock);
+
+	bam_dmux_power_off(dmux);
+	bam_dmux_pc_ack(dmux);
+	WRITE_ONCE(dmux->pc_state, false);
+}
+
+static bool bam_dmux_handle_pc(struct bam_dmux *dmux, bool new_state)
+{
+	lockdep_assert_held(&dmux->power_lock);
+
+	if (new_state == dmux->pc_state) {
+		if (new_state && dmux->pc_vote == BAM_DMUX_PC_VOTE_DOWN_PENDING)
+			dmux->pc_vote = BAM_DMUX_PC_VOTE_ACTIVE;
+		return false;
+	}
 
 	dev_dbg(dmux->dev, "pc: %u\n", new_state);
 
@@ -682,13 +726,40 @@ static irqreturn_t bam_dmux_pc_irq(int irq, void *data)
 			bam_dmux_pc_ack(dmux);
 		else
 			bam_dmux_power_off(dmux);
+	} else if (dmux->tx && dmux->pc_vote != BAM_DMUX_PC_VOTE_INACTIVE) {
+		/* The modem must keep the DMA engine on until pc is acked. */
+		dmux->pc_vote = BAM_DMUX_PC_VOTE_DOWN_PENDING;
+		dev_err_ratelimited(dmux->dev,
+				    "refusing pc down while host vote is active\n");
+		return false;
 	} else {
-		bam_dmux_power_off(dmux);
-		bam_dmux_pc_ack(dmux);
+		bam_dmux_complete_pc_down(dmux);
 	}
 
-	dmux->pc_state = new_state;
-	wake_up_all(&dmux->pc_wait);
+	if (new_state)
+		WRITE_ONCE(dmux->pc_state, true);
+	return true;
+}
+
+static irqreturn_t bam_dmux_pc_irq(int irq, void *data)
+{
+	struct bam_dmux *dmux = data;
+	bool new_state, wake;
+	int ret;
+
+	mutex_lock(&dmux->power_lock);
+	ret = irq_get_irqchip_state(dmux->pc_irq, IRQCHIP_STATE_LINE_LEVEL,
+				    &new_state);
+	if (ret) {
+		mutex_unlock(&dmux->power_lock);
+		dev_err_ratelimited(dmux->dev, "failed to read pc state: %d\n", ret);
+		return IRQ_HANDLED;
+	}
+
+	wake = bam_dmux_handle_pc(dmux, new_state);
+	mutex_unlock(&dmux->power_lock);
+	if (wake)
+		wake_up_all(&dmux->pc_wait);
 
 	return IRQ_HANDLED;
 }
@@ -696,9 +767,27 @@ static irqreturn_t bam_dmux_pc_irq(int irq, void *data)
 static irqreturn_t bam_dmux_pc_ack_irq(int irq, void *data)
 {
 	struct bam_dmux *dmux = data;
+	bool new_state, wake = false;
+	int ret;
 
 	dev_dbg(dmux->dev, "pc ack\n");
+
+	/* Process an earlier pc edge before publishing the host-vote ack. */
+	synchronize_irq(dmux->pc_irq);
+	mutex_lock(&dmux->power_lock);
+	ret = irq_get_irqchip_state(dmux->pc_irq, IRQCHIP_STATE_LINE_LEVEL,
+				    &new_state);
+	if (ret) {
+		mutex_unlock(&dmux->power_lock);
+		dev_err_ratelimited(dmux->dev, "failed to read pc state: %d\n", ret);
+		return IRQ_HANDLED;
+	}
+
+	wake = bam_dmux_handle_pc(dmux, new_state);
 	complete_all(&dmux->pc_ack_completion);
+	mutex_unlock(&dmux->power_lock);
+	if (wake)
+		wake_up_all(&dmux->pc_wait);
 
 	return IRQ_HANDLED;
 }
@@ -706,9 +795,47 @@ static irqreturn_t bam_dmux_pc_ack_irq(int irq, void *data)
 static int bam_dmux_runtime_suspend(struct device *dev)
 {
 	struct bam_dmux *dmux = dev_get_drvdata(dev);
+	bool wake = false;
 
 	dev_dbg(dev, "runtime suspend\n");
-	bam_dmux_pc_vote(dmux, false);
+
+	scoped_guard(mutex, &dmux->power_lock) {
+		bam_dmux_pc_vote_locked(dmux, false);
+		if (dmux->pc_vote == BAM_DMUX_PC_VOTE_DOWN_PENDING) {
+			dmux->pc_vote = BAM_DMUX_PC_VOTE_INACTIVE;
+			bam_dmux_complete_pc_down(dmux);
+			wake = true;
+		} else if (!dmux->pc_state) {
+			bam_dmux_power_off(dmux);
+		}
+	}
+	if (wake)
+		wake_up_all(&dmux->pc_wait);
+
+	return 0;
+}
+
+static int bam_dmux_request_tx(struct bam_dmux *dmux)
+{
+	struct device *dev = dmux->dev;
+
+	scoped_guard(mutex, &dmux->power_lock) {
+		/* Ensure that we actually initialized successfully */
+		if (!dmux->rx)
+			return -ENXIO;
+
+		/* Request TX channel if necessary */
+		if (dmux->tx)
+			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;
+			return -ENXIO;
+		}
+	}
 
 	return 0;
 }
@@ -716,6 +843,7 @@ static int bam_dmux_runtime_suspend(struct device *dev)
 static int __maybe_unused bam_dmux_runtime_resume(struct device *dev)
 {
 	struct bam_dmux *dmux = dev_get_drvdata(dev);
+	int ret;
 
 	dev_dbg(dev, "runtime resume\n");
 
@@ -724,50 +852,41 @@ 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;
 	}
 
-	/* Ensure that we actually initialized successfully */
-	if (!dmux->rx) {
-		bam_dmux_pc_vote(dmux, false);
-		return -ENXIO;
-	}
-
-	/* Request TX channel if necessary */
-	if (dmux->tx)
-		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;
+	ret = bam_dmux_request_tx(dmux);
+	if (ret)
 		bam_dmux_runtime_suspend(dev);
-		return -ENXIO;
-	}
 
-	return 0;
+	return ret;
 }
 
 static int bam_dmux_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct bam_dmux *dmux;
-	int ret, pc_ack_irq, i;
 	unsigned int bit;
+	bool pc_state;
+	int ret, i;
 
 	dmux = devm_kzalloc(dev, sizeof(*dmux), GFP_KERNEL);
 	if (!dmux)
@@ -780,9 +899,9 @@ static int bam_dmux_probe(struct platform_device *pdev)
 	if (dmux->pc_irq < 0)
 		return dmux->pc_irq;
 
-	pc_ack_irq = platform_get_irq_byname(pdev, "pc-ack");
-	if (pc_ack_irq < 0)
-		return pc_ack_irq;
+	dmux->pc_ack_irq = platform_get_irq_byname(pdev, "pc-ack");
+	if (dmux->pc_ack_irq < 0)
+		return dmux->pc_ack_irq;
 
 	dmux->pc = devm_qcom_smem_state_get(dev, "pc", &bit);
 	if (IS_ERR(dmux->pc))
@@ -799,6 +918,9 @@ 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);
+	ret = devm_mutex_init(dev, &dmux->power_lock);
+	if (ret)
+		return ret;
 
 	spin_lock_init(&dmux->tx_lock);
 	INIT_WORK(&dmux->tx_wakeup_work, bam_dmux_tx_wakeup_work);
@@ -817,31 +939,40 @@ static int bam_dmux_probe(struct platform_device *pdev)
 	pm_runtime_use_autosuspend(dev);
 	pm_runtime_enable(dev);
 
-	ret = devm_request_threaded_irq(dev, pc_ack_irq, NULL, bam_dmux_pc_ack_irq,
-					IRQF_ONESHOT, NULL, dmux);
+	ret = devm_request_threaded_irq(dev, dmux->pc_ack_irq, NULL,
+					bam_dmux_pc_ack_irq,
+					IRQF_ONESHOT | IRQF_NO_AUTOEN,
+					NULL, dmux);
 	if (ret)
 		goto err_disable_pm;
 
 	ret = devm_request_threaded_irq(dev, dmux->pc_irq, NULL, bam_dmux_pc_irq,
-					IRQF_ONESHOT, NULL, dmux);
+					IRQF_ONESHOT | IRQF_NO_AUTOEN, NULL, dmux);
 	if (ret)
-		goto err_disable_pm;
+		goto err_power_off;
 
+	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;
+				    &pc_state);
+	if (ret) {
+		mutex_unlock(&dmux->power_lock);
+		goto err_power_off;
+	}
 
 	/* Check if remote finished initialization before us */
-	if (dmux->pc_state) {
-		if (bam_dmux_power_on(dmux))
-			bam_dmux_pc_ack(dmux);
-		else
-			bam_dmux_power_off(dmux);
-	}
+	bam_dmux_handle_pc(dmux, pc_state);
+	mutex_unlock(&dmux->power_lock);
+
+	/* Start handling state changes after the initial state is processed. */
+	enable_irq(dmux->pc_ack_irq);
+	enable_irq(dmux->pc_irq);
 
 	return 0;
 
+err_power_off:
+	mutex_lock(&dmux->power_lock);
+	bam_dmux_power_off(dmux);
+	mutex_unlock(&dmux->power_lock);
 err_disable_pm:
 	pm_runtime_disable(dev);
 	pm_runtime_dont_use_autosuspend(dev);
@@ -872,12 +1003,16 @@ 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_ack_irq);
 	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 v5] net: wwan: qcom_bam_dmux: fix TX DMA channel use-after-free

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 1 (2) · Medium: 1 · Low: 0

- [High] The new design (live level re-read via
  irq_get_irqchip_state(IRQCHIP_STATE_LINE_LEVEL) as the only state…
- [Medium] bam_dmux_runtime_suspend() turns a possibly stale
  BAM_DMUX_PC_VOTE_DOWN_PENDING into an irreversible power-down…

Pre-existing issues:
- [High] bam_dmux_remove() cancels register_netdev_work and unregisters
  all netdevs while the RX DMA producer and the pc IRQ are still live;…
- [High] bam_dmux_rx_callback() can run concurrently with
  bam_dmux_power_off() and dereference a released/NULL RX channel.

--

net: wwan: qcom_bam_dmux: fix TX DMA channel use-after-free

This adds a power_lock mutex to serialize the modem power-control state,
re-reads the pc line level at each decision point instead of toggling a
cached bool, defers a pc=false transition while the host vote is active and
TX is allocated, and keeps both state IRQs masked with IRQF_NO_AUTOEN until
probe has committed the initial state.

Link: https://lore.kernel.org/netdev/20260822085308.1089-1-getshell@seu.edu.cn/

> diff --git a/drivers/net/wwan/qcom_bam_dmux.c b/drivers/net/wwan/qcom_bam_dmux.c
> index cc6ace8d64371..429d34a3cfa07 100644
> --- a/drivers/net/wwan/qcom_bam_dmux.c
> +++ b/drivers/net/wwan/qcom_bam_dmux.c

[ ... ]

> @@ -655,6 +683,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);

[Severity: High]
This isn't a bug introduced by this patch, but now that power_lock
serializes the power-control paths, is the RX completion callback still
left outside that serialization?

bam_dmux_rx_callback() ends with an unconditional refill:

drivers/net/wwan/qcom_bam_dmux.c:bam_dmux_rx_callback() {
	...
out:
	if (bam_dmux_skb_dma_queue_rx(skb_dma, GFP_ATOMIC))
		dma_async_issue_pending(dmux->rx);
}

while bam_dmux_power_off() does:

	if (dmux->rx) {
		dmaengine_terminate_sync(dmux->rx);
		dma_release_channel(dmux->rx);
		dmux->rx = NULL;
	}

Can the refill see dmux->rx == NULL and oops? dmaengine_prep_config_single()
dereferences chan with no NULL check:

include/linux/dmaengine.h:
	if (chan->device->device_prep_config_sg)

Qualcomm BAM registers device_terminate_all but no device_synchronize:

drivers/dma/qcom/bam_dma.c:
	bdev->common.device_terminate_all = bam_dma_terminate_all;

so dmaengine_terminate_sync() -> dmaengine_synchronize() cannot wait for a
callback that vchan_complete() has already spliced off and started. Should
the RX callback also take power_lock, or check a shutdown flag, before
requeueing?

> @@ -670,10 +700,24 @@ static void bam_dmux_free_skbs(struct bam_dmux_skb_dma skbs[],
>  	bam_dmux_free_skbs(dmux->rx_skbs, DMA_FROM_DEVICE);
>  }
>  
> -static irqreturn_t bam_dmux_pc_irq(int irq, void *data)
> +static void bam_dmux_complete_pc_down(struct bam_dmux *dmux)
>  {
> -	struct bam_dmux *dmux = data;
> -	bool new_state = !dmux->pc_state;
> +	lockdep_assert_held(&dmux->power_lock);
> +
> +	bam_dmux_power_off(dmux);
> +	bam_dmux_pc_ack(dmux);
> +	WRITE_ONCE(dmux->pc_state, false);
> +}
> +
> +static bool bam_dmux_handle_pc(struct bam_dmux *dmux, bool new_state)
> +{
> +	lockdep_assert_held(&dmux->power_lock);
> +
> +	if (new_state == dmux->pc_state) {
> +		if (new_state && dmux->pc_vote == BAM_DMUX_PC_VOTE_DOWN_PENDING)
> +			dmux->pc_vote = BAM_DMUX_PC_VOTE_ACTIVE;
> +		return false;
> +	}
>  
>  	dev_dbg(dmux->dev, "pc: %u\n", new_state);
>  

[ ... ]

> @@ -706,9 +795,47 @@ static irqreturn_t bam_dmux_pc_ack_irq(int irq, void *data)
>  static int bam_dmux_runtime_suspend(struct device *dev)
>  {
>  	struct bam_dmux *dmux = dev_get_drvdata(dev);
> +	bool wake = false;
>  
>  	dev_dbg(dev, "runtime suspend\n");
> -	bam_dmux_pc_vote(dmux, false);
> +
> +	scoped_guard(mutex, &dmux->power_lock) {
> +		bam_dmux_pc_vote_locked(dmux, false);
> +		if (dmux->pc_vote == BAM_DMUX_PC_VOTE_DOWN_PENDING) {
> +			dmux->pc_vote = BAM_DMUX_PC_VOTE_INACTIVE;
> +			bam_dmux_complete_pc_down(dmux);
> +			wake = true;
> +		} else if (!dmux->pc_state) {
> +			bam_dmux_power_off(dmux);
> +		}
> +	}

[Severity: Medium]
Is it safe for this to act on the cached BAM_DMUX_PC_VOTE_DOWN_PENDING
without re-sampling the pc line?

Every other new decision point in the patch calls
irq_get_irqchip_state(IRQCHIP_STATE_LINE_LEVEL) first, but this one
converts the cached flag straight into bam_dmux_complete_pc_down(), which
releases both channels, frees the RX skbs and toggles pc-ack:

	bam_dmux_power_off(dmux);
	bam_dmux_pc_ack(dmux);
	WRITE_ONCE(dmux->pc_state, false);

DOWN_PENDING is only cancelled from bam_dmux_handle_pc() when a later
invocation observes new_state == dmux->pc_state == true:

	if (new_state == dmux->pc_state) {
		if (new_state && dmux->pc_vote == BAM_DMUX_PC_VOTE_DOWN_PENDING)
			dmux->pc_vote = BAM_DMUX_PC_VOTE_ACTIVE;

So if the modem re-raises pc and the rising-edge handler is still blocked on
mutex_lock(&dmux->power_lock) when the autosuspend timer fires:

bam_dmux_runtime_suspend()
   scoped_guard(mutex, &dmux->power_lock)
      bam_dmux_complete_pc_down()   /* pc line is high again */

does the driver then acknowledge a pc=false transition the modem has already
revoked, and tear down RX while the peer believes the link is up? The commit
message describes this as the case to avoid:

    "acknowledging a new pc=false transition while the host vote remains
    active lets the modem power down the DMA engine underneath the driver"

Would reading the line level here before calling
bam_dmux_complete_pc_down() close that window?

[ ... ]

> @@ -817,31 +939,40 @@ static int bam_dmux_probe(struct platform_device *pdev)
>  	pm_runtime_use_autosuspend(dev);
>  	pm_runtime_enable(dev);
>  
> -	ret = devm_request_threaded_irq(dev, pc_ack_irq, NULL, bam_dmux_pc_ack_irq,
> -					IRQF_ONESHOT, NULL, dmux);
> +	ret = devm_request_threaded_irq(dev, dmux->pc_ack_irq, NULL,
> +					bam_dmux_pc_ack_irq,
> +					IRQF_ONESHOT | IRQF_NO_AUTOEN,
> +					NULL, dmux);
>  	if (ret)
>  		goto err_disable_pm;
>  
>  	ret = devm_request_threaded_irq(dev, dmux->pc_irq, NULL, bam_dmux_pc_irq,
> -					IRQF_ONESHOT, NULL, dmux);
> +					IRQF_ONESHOT | IRQF_NO_AUTOEN, NULL, dmux);
>  	if (ret)
> -		goto err_disable_pm;
> +		goto err_power_off;
>  
> +	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;
> +				    &pc_state);
> +	if (ret) {
> +		mutex_unlock(&dmux->power_lock);
> +		goto err_power_off;
> +	}
>  
>  	/* Check if remote finished initialization before us */
> -	if (dmux->pc_state) {
> -		if (bam_dmux_power_on(dmux))
> -			bam_dmux_pc_ack(dmux);
> -		else
> -			bam_dmux_power_off(dmux);
> -	}
> +	bam_dmux_handle_pc(dmux, pc_state);
> +	mutex_unlock(&dmux->power_lock);
> +
> +	/* Start handling state changes after the initial state is processed. */
> +	enable_irq(dmux->pc_ack_irq);
> +	enable_irq(dmux->pc_irq);

[Severity: High]
Can a pc transition be lost in the window created here between the level
snapshot and enable_irq(dmux->pc_irq)?

The in-tree DTs for qcom,bam-dmux (msm8916.dtsi, msm8939.dtsi) parent these
interrupts on SMSM:

	interrupt-parent = <&hexagon_smsm>;
	interrupts = <1 IRQ_TYPE_EDGE_BOTH>, <11 IRQ_TYPE_EDGE_BOTH>;

and smsm_intr() advances the cached level for all bits but only dispatches
the enabled ones:

drivers/soc/qcom/smsm.c:smsm_intr() {
	val = readl(entry->remote_state);
	changed = val ^ xchg(&entry->last_value, val);

	for_each_set_bit(i, entry->irq_enabled, 32) {
	...
}

and on unmask the cache is re-synced to the live line rather than replaying
the missed edge:

drivers/soc/qcom/smsm.c:smsm_unmask_irq() {
	/* Make sure our last cached state is up-to-date */
	if (readl(entry->remote_state) & BIT(irq))
		set_bit(irq, &entry->last_value);
	else
		clear_bit(irq, &entry->last_value);
}

The smp2p provider does the same thing in qcom_smp2p_notify_in().

So if the modem raises pc after irq_get_irqchip_state() but before
enable_irq(), is that edge simply consumed?  bam_dmux_power_on() would never
run, no pc-ack would be toggled, and the following falling edge would land in
bam_dmux_handle_pc() with new_state == dmux->pc_state == false:

	if (new_state == dmux->pc_state) {
		...
		return false;
	}

after which every bam_dmux_runtime_resume() would time out in

	wait_event_timeout(dmux->pc_wait, READ_ONCE(dmux->pc_state),
			   BAM_DMUX_REMOTE_TIMEOUT)

because the modem has no reason to produce another rising edge. Given that
probe already has a "Check if remote finished initialization before us" path,
a pc rising edge concurrent with probe looks like expected timing. Is there a
recovery path for that case, or does relying on a re-read of the line level
while the child IRQ is masked need a different approach?

>  
>  	return 0;
>  
> +err_power_off:
> +	mutex_lock(&dmux->power_lock);
> +	bam_dmux_power_off(dmux);
> +	mutex_unlock(&dmux->power_lock);
>  err_disable_pm:
>  	pm_runtime_disable(dev);
>  	pm_runtime_dont_use_autosuspend(dev);
> @@ -872,12 +1003,16 @@ 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_ack_irq);
>  	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);
>  }

[Severity: High]
This is a pre-existing ordering issue rather than something the patch
introduces, but since the remove path is being touched here: can
register_netdev_work be re-queued after its only cancellation?

bam_dmux_remove() runs cancel_work_sync(&dmux->register_netdev_work) and
unregister_netdevice_many() at the top, while the RX channel is still armed
and the pc IRQ is still enabled. Only afterwards does it reach
disable_irq()/bam_dmux_power_off().

A remote OPEN command arriving in that window does:

bam_dmux_rx_callback()
  bam_dmux_cmd_open()
     /* Cannot sleep here, schedule work to register the netdev */
     schedule_work(&dmux->register_netdev_work);

and there is no second cancel_work_sync() after the IRQs are disabled and the
DMA is terminated. bam_dmux_register_netdev_work() would then dereference the
devm-allocated dmux and publish a fresh netdev holding it:

	bndev = netdev_priv(netdev);
	bndev->dmux = dmux;
	...
	ret = register_netdev(netdev);

Once remove() returns, devm frees dmux, so any xmit on that interface would
touch freed memory. Would moving the cancel_work_sync() after
disable_irq()/bam_dmux_power_off(), or adding a second one there, be
appropriate?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903180057.1437-1-getshell%40seu.edu.cn

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

[Paolo Abeni]

On 9/3/26 8:00 PM, Hongyan Xu wrote:
> The modem power-control interrupt can release dmux->tx while the command,
> netdev transmit, or deferred wakeup paths are preparing and submitting
> DMA descriptors. A runtime PM reference alone does not order those paths
> against the modem-driven interrupt.
> 
> The pc and pc-ack IRQ actions do not encode the modem protocol order. A
> pc=false edge raised before a new host power vote is acknowledged can
> therefore be handled after runtime resume observes the acknowledgment and
> starts using TX. On the other hand, acknowledging a new pc=false transition
> while the host vote remains active lets the modem power down the DMA engine
> underneath the driver.
> 
> Serialize power-control state with a mutex. Before publishing a host-vote
> acknowledgment, sample the actual pc line and process any delayed
> transition.
> When pc goes low while the host vote is active and the TX channel is
> allocated, defer the transition without acknowledging it. Repeated pc=false
> interrupts remain deferred and cannot release the channel. If pc returns
> high, cancel the deferred transition.
> 
> Runtime suspend drops the host vote under the same lock. If a pc=false
> transition was deferred, it then terminates and releases the DMA channels
> before acknowledging the transition. If runtime suspend wins the race with
> the pc interrupt, the interrupt observes the inactive host vote and can
> safely perform the same shutdown. This orders channel release after the
> last runtime PM user without relying on whether the modem acknowledges
> the host vote before or after asserting pc.
> 
> Keep both state IRQs disabled until the initial probe state is committed,
> and disable both before final remove cleanup. Use a device-managed mutex
> and scoped guards for the serialized paths.
> 
> 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>
> 
> ---
> Changes in v5:
> - Drop the vote-ack state; PC-down safety does not depend on ACK ordering.
> - Keep repeated PC-low interrupts deferred while TX can still be in use.
> - Complete a deferred PC-down directly from runtime suspend after dropping
>   the host vote.
> - Move TX channel acquisition into a scoped helper and retain early returns.
> 
> Link: https://lore.kernel.org/netdev/20260822085308.1089-1-getshell@seu.edu.cn/
> 
> diff --git a/drivers/net/wwan/qcom_bam_dmux.c b/drivers/net/wwan/qcom_bam_dmux.c
> index cc6ace8..429d34a 100644
> --- a/drivers/net/wwan/qcom_bam_dmux.c
> +++ b/drivers/net/wwan/qcom_bam_dmux.c
> @@ -6,12 +6,14 @@
>  
>  #include <linux/atomic.h>
>  #include <linux/bitops.h>
> +#include <linux/cleanup.h>
>  #include <linux/completion.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/dmaengine.h>
>  #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>
> @@ -64,15 +66,23 @@ struct bam_dmux_skb_dma {
>  	dma_addr_t addr;
>  };
>  
> +enum bam_dmux_pc_vote {
> +	BAM_DMUX_PC_VOTE_INACTIVE, /* pc_vote = false */
> +	BAM_DMUX_PC_VOTE_ACTIVE, /* pc_vote = true */
> +	BAM_DMUX_PC_VOTE_DOWN_PENDING, /* pc = false while host vote active */
> +};
> +
>  struct bam_dmux {
>  	struct device *dev;
>  
> -	int pc_irq;
> +	int pc_irq, pc_ack_irq;
>  	bool pc_state, pc_ack_state;
>  	struct qcom_smem_state *pc, *pc_ack;
>  	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 */
> +	enum bam_dmux_pc_vote pc_vote;
>  
>  	struct dma_chan *rx, *tx;
>  	struct bam_dmux_skb_dma rx_skbs[BAM_DMUX_NUM_SKB];
> @@ -99,6 +109,24 @@ 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);
> +
> +	if (enable)
> +		dmux->pc_vote = BAM_DMUX_PC_VOTE_ACTIVE;
> +	else if (dmux->pc_vote != BAM_DMUX_PC_VOTE_DOWN_PENDING)
> +		dmux->pc_vote = BAM_DMUX_PC_VOTE_INACTIVE;
> +	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 +683,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);
> @@ -670,10 +700,24 @@ static void bam_dmux_power_off(struct bam_dmux *dmux)
>  	bam_dmux_free_skbs(dmux->rx_skbs, DMA_FROM_DEVICE);
>  }
>  
> -static irqreturn_t bam_dmux_pc_irq(int irq, void *data)
> +static void bam_dmux_complete_pc_down(struct bam_dmux *dmux)
>  {
> -	struct bam_dmux *dmux = data;
> -	bool new_state = !dmux->pc_state;
> +	lockdep_assert_held(&dmux->power_lock);
> +
> +	bam_dmux_power_off(dmux);
> +	bam_dmux_pc_ack(dmux);
> +	WRITE_ONCE(dmux->pc_state, false);
> +}
> +
> +static bool bam_dmux_handle_pc(struct bam_dmux *dmux, bool new_state)
> +{
> +	lockdep_assert_held(&dmux->power_lock);
> +
> +	if (new_state == dmux->pc_state) {
> +		if (new_state && dmux->pc_vote == BAM_DMUX_PC_VOTE_DOWN_PENDING)
> +			dmux->pc_vote = BAM_DMUX_PC_VOTE_ACTIVE;
> +		return false;
> +	}
>  
>  	dev_dbg(dmux->dev, "pc: %u\n", new_state);
>  
> @@ -682,13 +726,40 @@ static irqreturn_t bam_dmux_pc_irq(int irq, void *data)
>  			bam_dmux_pc_ack(dmux);
>  		else
>  			bam_dmux_power_off(dmux);
> +	} else if (dmux->tx && dmux->pc_vote != BAM_DMUX_PC_VOTE_INACTIVE) {
> +		/* The modem must keep the DMA engine on until pc is acked. */
> +		dmux->pc_vote = BAM_DMUX_PC_VOTE_DOWN_PENDING;
> +		dev_err_ratelimited(dmux->dev,
> +				    "refusing pc down while host vote is active\n");
> +		return false;
>  	} else {
> -		bam_dmux_power_off(dmux);
> -		bam_dmux_pc_ack(dmux);
> +		bam_dmux_complete_pc_down(dmux);
>  	}
>  
> -	dmux->pc_state = new_state;
> -	wake_up_all(&dmux->pc_wait);
> +	if (new_state)
> +		WRITE_ONCE(dmux->pc_state, true);
> +	return true;
> +}
> +
> +static irqreturn_t bam_dmux_pc_irq(int irq, void *data)
> +{
> +	struct bam_dmux *dmux = data;
> +	bool new_state, wake;
> +	int ret;
> +
> +	mutex_lock(&dmux->power_lock);
> +	ret = irq_get_irqchip_state(dmux->pc_irq, IRQCHIP_STATE_LINE_LEVEL,
> +				    &new_state);
> +	if (ret) {
> +		mutex_unlock(&dmux->power_lock);
> +		dev_err_ratelimited(dmux->dev, "failed to read pc state: %d\n", ret);
> +		return IRQ_HANDLED;
> +	}
> +
> +	wake = bam_dmux_handle_pc(dmux, new_state);
> +	mutex_unlock(&dmux->power_lock);
> +	if (wake)
> +		wake_up_all(&dmux->pc_wait);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -696,9 +767,27 @@ static irqreturn_t bam_dmux_pc_irq(int irq, void *data)
>  static irqreturn_t bam_dmux_pc_ack_irq(int irq, void *data)
>  {
>  	struct bam_dmux *dmux = data;
> +	bool new_state, wake = false;
> +	int ret;
>  
>  	dev_dbg(dmux->dev, "pc ack\n");
> +
> +	/* Process an earlier pc edge before publishing the host-vote ack. */
> +	synchronize_irq(dmux->pc_irq);
> +	mutex_lock(&dmux->power_lock);
> +	ret = irq_get_irqchip_state(dmux->pc_irq, IRQCHIP_STATE_LINE_LEVEL,
> +				    &new_state);
> +	if (ret) {
> +		mutex_unlock(&dmux->power_lock);
> +		dev_err_ratelimited(dmux->dev, "failed to read pc state: %d\n", ret);
> +		return IRQ_HANDLED;
> +	}
> +
> +	wake = bam_dmux_handle_pc(dmux, new_state);
>  	complete_all(&dmux->pc_ack_completion);
> +	mutex_unlock(&dmux->power_lock);
> +	if (wake)
> +		wake_up_all(&dmux->pc_wait);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -706,9 +795,47 @@ static irqreturn_t bam_dmux_pc_ack_irq(int irq, void *data)
>  static int bam_dmux_runtime_suspend(struct device *dev)
>  {
>  	struct bam_dmux *dmux = dev_get_drvdata(dev);
> +	bool wake = false;
>  
>  	dev_dbg(dev, "runtime suspend\n");
> -	bam_dmux_pc_vote(dmux, false);
> +
> +	scoped_guard(mutex, &dmux->power_lock) {
> +		bam_dmux_pc_vote_locked(dmux, false);
> +		if (dmux->pc_vote == BAM_DMUX_PC_VOTE_DOWN_PENDING) {
> +			dmux->pc_vote = BAM_DMUX_PC_VOTE_INACTIVE;
> +			bam_dmux_complete_pc_down(dmux);
> +			wake = true;
> +		} else if (!dmux->pc_state) {
> +			bam_dmux_power_off(dmux);
> +		}
> +	}
> +	if (wake)
> +		wake_up_all(&dmux->pc_wait);
> +
> +	return 0;
> +}
> +
> +static int bam_dmux_request_tx(struct bam_dmux *dmux)
> +{
> +	struct device *dev = dmux->dev;
> +
> +	scoped_guard(mutex, &dmux->power_lock) {
> +		/* Ensure that we actually initialized successfully */
> +		if (!dmux->rx)
> +			return -ENXIO;
> +
> +		/* Request TX channel if necessary */
> +		if (dmux->tx)
> +			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;
> +			return -ENXIO;
> +		}
> +	}
>  
>  	return 0;
>  }
> @@ -716,6 +843,7 @@ static int bam_dmux_runtime_suspend(struct device *dev)
>  static int __maybe_unused bam_dmux_runtime_resume(struct device *dev)
>  {
>  	struct bam_dmux *dmux = dev_get_drvdata(dev);
> +	int ret;
>  
>  	dev_dbg(dev, "runtime resume\n");
>  
> @@ -724,50 +852,41 @@ 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;
>  	}
>  
> -	/* Ensure that we actually initialized successfully */
> -	if (!dmux->rx) {
> -		bam_dmux_pc_vote(dmux, false);
> -		return -ENXIO;
> -	}
> -
> -	/* Request TX channel if necessary */
> -	if (dmux->tx)
> -		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;
> +	ret = bam_dmux_request_tx(dmux);
> +	if (ret)
>  		bam_dmux_runtime_suspend(dev);
> -		return -ENXIO;
> -	}
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int bam_dmux_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct bam_dmux *dmux;
> -	int ret, pc_ack_irq, i;
>  	unsigned int bit;
> +	bool pc_state;
> +	int ret, i;
>  
>  	dmux = devm_kzalloc(dev, sizeof(*dmux), GFP_KERNEL);
>  	if (!dmux)
> @@ -780,9 +899,9 @@ static int bam_dmux_probe(struct platform_device *pdev)
>  	if (dmux->pc_irq < 0)
>  		return dmux->pc_irq;
>  
> -	pc_ack_irq = platform_get_irq_byname(pdev, "pc-ack");
> -	if (pc_ack_irq < 0)
> -		return pc_ack_irq;
> +	dmux->pc_ack_irq = platform_get_irq_byname(pdev, "pc-ack");
> +	if (dmux->pc_ack_irq < 0)
> +		return dmux->pc_ack_irq;
>  
>  	dmux->pc = devm_qcom_smem_state_get(dev, "pc", &bit);
>  	if (IS_ERR(dmux->pc))
> @@ -799,6 +918,9 @@ 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);
> +	ret = devm_mutex_init(dev, &dmux->power_lock);
> +	if (ret)
> +		return ret;
>  
>  	spin_lock_init(&dmux->tx_lock);
>  	INIT_WORK(&dmux->tx_wakeup_work, bam_dmux_tx_wakeup_work);
> @@ -817,31 +939,40 @@ static int bam_dmux_probe(struct platform_device *pdev)
>  	pm_runtime_use_autosuspend(dev);
>  	pm_runtime_enable(dev);
>  
> -	ret = devm_request_threaded_irq(dev, pc_ack_irq, NULL, bam_dmux_pc_ack_irq,
> -					IRQF_ONESHOT, NULL, dmux);
> +	ret = devm_request_threaded_irq(dev, dmux->pc_ack_irq, NULL,
> +					bam_dmux_pc_ack_irq,
> +					IRQF_ONESHOT | IRQF_NO_AUTOEN,
> +					NULL, dmux);
>  	if (ret)
>  		goto err_disable_pm;
>  
>  	ret = devm_request_threaded_irq(dev, dmux->pc_irq, NULL, bam_dmux_pc_irq,
> -					IRQF_ONESHOT, NULL, dmux);
> +					IRQF_ONESHOT | IRQF_NO_AUTOEN, NULL, dmux);
>  	if (ret)
> -		goto err_disable_pm;
> +		goto err_power_off;
>  
> +	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;
> +				    &pc_state);
> +	if (ret) {
> +		mutex_unlock(&dmux->power_lock);
> +		goto err_power_off;
> +	}
>  
>  	/* Check if remote finished initialization before us */
> -	if (dmux->pc_state) {
> -		if (bam_dmux_power_on(dmux))
> -			bam_dmux_pc_ack(dmux);
> -		else
> -			bam_dmux_power_off(dmux);
> -	}
> +	bam_dmux_handle_pc(dmux, pc_state);
> +	mutex_unlock(&dmux->power_lock);
> +
> +	/* Start handling state changes after the initial state is processed. */
> +	enable_irq(dmux->pc_ack_irq);
> +	enable_irq(dmux->pc_irq);
>  
>  	return 0;
>  
> +err_power_off:
> +	mutex_lock(&dmux->power_lock);
> +	bam_dmux_power_off(dmux);
> +	mutex_unlock(&dmux->power_lock);
>  err_disable_pm:
>  	pm_runtime_disable(dev);
>  	pm_runtime_dont_use_autosuspend(dev);
> @@ -872,12 +1003,16 @@ 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_ack_irq);

Beyond all the sashiko comments, this patch is quite big and should
be split in smaller chunck to help reviewers.

I.e. the above line looks like a separate fix, deserving
it's own patch.

/P