Re: [PATCH] net: wwan: qcom_bam_dmux: fix TX DMA channel use-after-free
Stephan Gerhold <[email protected]>
| Newsgroups | org.kernel.vger.linux-arm-msm,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
On Thu, Aug 06, 2026 at 02:06:42PM +0800, Hongyan Xu wrote:
> The modem-driven power-control IRQ can release the TX DMA channel
> independently of the runtime PM callbacks. It can therefore race with the
> command, netdev transmit, and wakeup-work paths while they prepare and
> submit descriptors through dmux->tx. A runtime PM reference prevents
> runtime suspend, but does not serialize those paths with the external IRQ.
>
> Protect TX channel users with SRCU. Clear the published channel pointer and
> wait for existing readers before terminating and releasing the channel.
> Serialize channel replacement and power transitions with a mutex, and
> discard deferred packets if the channel disappears after runtime resume.
>
> Fixes: 21a0ffd9b38c ("net: wwan: Add Qualcomm BAM-DMUX WWAN network driver")
> Signed-off-by: Hongyan Xu <[email protected]>
Thanks for the report and patch!
Generally speaking, I would expect the modem never signals pc=false when
there is an active power vote from the host (which is modeled as the
runtime PM state in the driver). The BAM DMUX protocol is by design
quite fragile, so I think we could also just refuse to work with
non-conform firmwares, e.g. by never acknowledging such a broken IRQ
and keeping the TX channel allocated.
There is one rare situation however where this race condition could
occur even with a conformant firmware:
1. The RX path is active (pc_state = true), but the TX path is idle
(i.e. the host pc vote is false).
2. We try to transmit a packet and request runtime resume.
3. bam_dmux_runtime_resume() sends pc vote true.
4. The modem sends pc_state = false, but the bam_dmux_pc_irq() handler
gets delayed for some reason.
5. The modem acknowledges the new pc vote (which would be followed by
pc_state = true, but it will likely wait until the host has
acknowledges the previous pc_state = false IRQ).
6. bam_dmux_pc_irq() still hasn't run, so bam_dmux_runtime_resume()
still sees pc_state == true, finishes and goes ahead with
transmitting packets.
7. bam_dmux_pc_irq() runs and frees the TX channel that is in use.
Your patch solves that by dropping packets in this situation. This can
be problematic, e.g. in the following situation:
1. The RX path is active (pc_state = true), but the TX path is idle
(i.e. the host pc vote is false).
2. bam_dmux_netdev_open() runs and sends BAM_DMUX_CMD_OPEN.
3. The command is put into the DMA queue and bam_dmux_netdev_open()
returns success (it does not wait for a confirmation, at the moment).
4. In this moment, the packet is cancelled again by
dmaengine_terminate_sync() and the interface will never be opened.
5. (In addition, bam_dmux_tx_done() is never called anywhere for the
packet with your patch, so the buffer will leak...)
I think we need some approach that minimizes the overhead for handling
non-conform firmware, prevents the use-after-free, but also avoids
dropping queued commands where possible.
I'm not entirely sure yet what's the best way to do that. It may help
already to introduce some synchronization for power-related events (pc,
pc ack, runtime PM). For the situation above, I wouldn't expect the
modem to signal pc_state = false after it has already acknowledged the
new pc vote. This could theoretically happen with the current driver
though, since pc and pc ack are separate IRQs that are handled by
separate IRQ threads.
Suggestions welcome. I will also try to find some time to look into it
further.
Thanks,
Stephan