[4/6] dmaengine: sun6i: track paused virtual channels
Commit Message
sun6i_dma_tx_status() only returns the cookie state, so a paused but
incomplete transfer is reported as DMA_IN_PROGRESS. The 8250 RX flush
path pauses the channel and calls its completion helper before terminating
it. That helper ignores an in-progress transfer to avoid consuming a new
RX operation. With sun6i it consequently drops the partial buffer and
leaves rx_running set, preventing another receive from being submitted.
Track pause state under the channel-assignment lock and report DMA_PAUSED
for incomplete cookies, including status queries without a residue output.
Flush the hardware pause request before returning. Completed cookies
remain DMA_COMPLETE.
Keep paused channels out of the pending queue and do not start a queued
descriptor when a paused channel's previous descriptor completes. Resume
requeues an unassigned channel only once and schedules the allocator, so
an idle controller does not require an unrelated completion to resume it.
Clear the pause state when terminating the channel.
Fixes: 555859308723 ("dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
drivers/dma/sun6i-dma.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
Comments
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dmaengine: sun6i: UAF and pchan starvation in sun6i_dma_tasklet() when paused
--
commit 668628da200a577f2488b0d26796212d9a34a3f9
Author: James Hilliard <james.hilliard1@gmail.com>
dmaengine: sun6i: track paused virtual channels
This commit adds pause state tracking to virtual channels in the sun6i DMA
driver so sun6i_dma_tx_status() reports DMA_PAUSED for incomplete transfers
and paused channels do not start new descriptors or enter the pending queue.
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index cd0562de7def1..7ac6693a041eb 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
[ ... ]
> @@ -538,7 +539,7 @@ static void sun6i_dma_tasklet(struct tasklet_struct *t)
>
> pchan = vchan->phy;
>
> - if (pchan && pchan->done) {
> + if (pchan && pchan->done && !vchan->paused) {
[Severity: High]
What happens if a non-cyclic descriptor completes in sun6i_dma_interrupt()
just before or concurrently with sun6i_dma_pause()?
When a non-cyclic transfer finishes, sun6i_dma_interrupt() marks the cookie
complete and sets pchan->done:
drivers/dma/sun6i-dma.c:sun6i_dma_interrupt() {
...
} else {
vchan_cookie_complete(&pchan->desc->vd);
pchan->done = pchan->desc;
}
...
}
vchan_cookie_complete() places &pchan->desc->vd on vc->desc_completed and
schedules vc->task (vchan_complete()). That tasklet runs independently of
vchan->paused and frees pchan->desc along with its LLI pool entries via
vchan_complete() -> vchan_vdesc_fini() -> sun6i_dma_free_desc_virt() ->
sun6i_dma_free_desc().
Previously, sun6i_dma_tasklet() always entered the pchan && pchan->done block
and called sun6i_dma_start_desc(), which either replaced pchan->desc with the
next issued descriptor or cleared pchan->desc and pchan->done to NULL before
detaching pchan from vchan.
With the added !vchan->paused check, sun6i_dma_tasklet() skips this cleanup
completely while the channel is paused. Doesn't this leave pchan->desc and
pchan->done holding dangling pointers to the freed struct sun6i_desc, while
also keeping the idle physical channel pchan pinned to the paused vchan and
starving other pending virtual channels in sdev->pending?
If a caller then queries sun6i_dma_tx_status() with a non-NULL state for a
submitted-but-unissued cookie (on vc.desc_submitted, which vchan_find_desc()
does not search), vd is NULL while pchan->desc is still non-NULL:
drivers/dma/sun6i-dma.c:sun6i_dma_tx_status() {
...
vd = vchan_find_desc(&vchan->vc, cookie);
txd = to_sun6i_desc(&vd->tx);
if (vd) {
for (lli = txd->v_lli; lli != NULL; lli = lli->v_lli_next)
bytes += lli->len;
} else if (!pchan || !pchan->desc) {
bytes = 0;
} else {
bytes = sun6i_get_chan_size(pchan);
}
...
}
Won't sun6i_get_chan_size(pchan) then dereference the freed pchan->desc and
walk its freed txd->v_lli chain?
@@ -209,6 +209,7 @@ struct sun6i_vchan {
u8 port;
u8 irq_type;
bool cyclic;
+ bool paused;
};
struct sun6i_dma_dev {
@@ -538,7 +539,7 @@ static void sun6i_dma_tasklet(struct tasklet_struct *t)
pchan = vchan->phy;
- if (pchan && pchan->done) {
+ if (pchan && pchan->done && !vchan->paused) {
if (sun6i_dma_start_desc(vchan)) {
/*
* No current txd associated with this channel
@@ -934,10 +935,12 @@ static int sun6i_dma_pause(struct dma_chan *chan)
dev_dbg(dmaengine_chan_dev(chan), "vchan %p: pause\n", &vchan->vc);
spin_lock_irqsave(&sdev->lock, flags);
+ vchan->paused = true;
pchan = vchan->phy;
if (pchan) {
writel(DMA_CHAN_PAUSE_PAUSE,
pchan->base + DMA_CHAN_PAUSE);
+ readl(pchan->base + DMA_CHAN_PAUSE);
} else {
list_del_init(&vchan->node);
}
@@ -959,12 +962,14 @@ static int sun6i_dma_resume(struct dma_chan *chan)
spin_lock(&vchan->vc.lock);
pchan = vchan->phy;
+ vchan->paused = false;
if (pchan) {
writel(DMA_CHAN_PAUSE_RESUME,
pchan->base + DMA_CHAN_PAUSE);
- } else if (!list_empty(&vchan->vc.desc_issued)) {
+ } else if (!list_empty(&vchan->vc.desc_issued) && list_empty(&vchan->node)) {
list_add_tail(&vchan->node, &sdev->pending);
}
+ tasklet_schedule(&sdev->task);
spin_unlock(&vchan->vc.lock);
spin_unlock_irqrestore(&sdev->lock, flags);
@@ -993,6 +998,7 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
}
vchan->cyclic = false;
+ vchan->paused = false;
vchan_get_all_descriptors(&vchan->vc, &head);
if (pchan) {
@@ -1040,6 +1046,8 @@ static enum dma_status sun6i_dma_tx_status(struct dma_chan *chan,
spin_lock_irqsave(&sdev->lock, flags);
spin_lock(&vchan->vc.lock);
ret = dma_cookie_status(chan, cookie, state);
+ if (ret == DMA_IN_PROGRESS && vchan->paused)
+ ret = DMA_PAUSED;
if (ret == DMA_COMPLETE || !state)
goto out;
pchan = vchan->phy;
@@ -1074,7 +1082,7 @@ static void sun6i_dma_issue_pending(struct dma_chan *chan)
spin_lock(&vchan->vc.lock);
if (vchan_issue_pending(&vchan->vc)) {
- if (!vchan->phy && list_empty(&vchan->node)) {
+ if (!vchan->paused && !vchan->phy && list_empty(&vchan->node)) {
list_add_tail(&vchan->node, &sdev->pending);
tasklet_schedule(&sdev->task);
dev_dbg(dmaengine_chan_dev(chan), "vchan %p: issued\n",