Netdev List
 help / color / mirror / Atom feed
From: Stephan Gerhold <stephan.gerhold@linaro.org>
To: Hongyan Xu <getshell@seu.edu.cn>
Cc: Stephan Gerhold <stephan@gerhold.net>,
	Loic Poulain <loic.poulain@oss.qualcomm.com>,
	Sergey Ryazanov <ryazanov.s.a@gmail.com>,
	Johannes Berg <johannes@sipsolutions.net>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	jianhao.xu@seu.edu.cn
Subject: Re: [PATCH] net: wwan: qcom_bam_dmux: fix TX DMA channel use-after-free
Date: Thu, 6 Aug 2026 18:15:08 +0200	[thread overview]
Message-ID: <anSzDNYf0AMW7U9Y@linaro.org> (raw)
In-Reply-To: <20260806060642.1281-1-getshell@seu.edu.cn>

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 <getshell@seu.edu.cn>

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

      reply	other threads:[~2026-08-06 16:15 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-06  6:06 [PATCH] net: wwan: qcom_bam_dmux: fix TX DMA channel use-after-free Hongyan Xu
2026-08-06 16:15 ` Stephan Gerhold [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=anSzDNYf0AMW7U9Y@linaro.org \
    --to=stephan.gerhold@linaro.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=getshell@seu.edu.cn \
    --cc=jianhao.xu@seu.edu.cn \
    --cc=johannes@sipsolutions.net \
    --cc=kuba@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=loic.poulain@oss.qualcomm.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=ryazanov.s.a@gmail.com \
    --cc=stephan@gerhold.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox