2024-03-27 10:49:51

by Joao Pinto

[permalink] [raw]
Subject: [PATCH RESEND] Avoid hw_desc array overrun in dw-axi-dmac

I have a use case where nr_buffers = 3 and in which each descriptor is composed by 3
segments, resulting in the DMA channel descs_allocated to be 9. Since axi_desc_put()
handles the hw_desc considering the descs_allocated, this scenario would result in a
kernel panic (hw_desc array will be overrun).

To fix this, the proposal is to add a new member to the axi_dma_desc structure,
where we keep the number of allocated hw_descs (axi_desc_alloc()) and use it in
axi_desc_put() to handle the hw_desc array correctly.

Additionally I propose to remove the axi_chan_start_first_queued() call after completing
the transfer, since it was identified that unbalance can occur (started descriptors can
be interrupted and transfer ignored due to DMA channel not being enabled).

Signed-off-by: Joao Pinto <[email protected]>
---
drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 7 ++-----
drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 1 +
2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
index a86a81f..b39f37a 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
@@ -302,6 +302,7 @@ static struct axi_dma_desc *axi_desc_alloc(u32 num)
kfree(desc);
return NULL;
}
+ desc->nr_hw_descs = num;

return desc;
}
@@ -328,7 +329,7 @@ static struct axi_dma_lli *axi_desc_get(struct axi_dma_chan *chan,
static void axi_desc_put(struct axi_dma_desc *desc)
{
struct axi_dma_chan *chan = desc->chan;
- int count = atomic_read(&chan->descs_allocated);
+ int count = desc->nr_hw_descs;
struct axi_dma_hw_desc *hw_desc;
int descs_put;

@@ -1139,9 +1139,6 @@ static void axi_chan_block_xfer_complete(struct axi_dma_chan *chan)
/* Remove the completed descriptor from issued list before completing */
list_del(&vd->node);
vchan_cookie_complete(vd);
-
- /* Submit queued descriptors after processing the completed ones */
- axi_chan_start_first_queued(chan);
}

out:
diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
index 454904d..ac571b4 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac.h
@@ -104,6 +104,7 @@ struct axi_dma_desc {
u32 completed_blocks;
u32 length;
u32 period_len;
+ u32 nr_hw_descs;
};

struct axi_dma_chan_config {
--
1.8.3.1



2024-04-07 16:41:29

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH RESEND] Avoid hw_desc array overrun in dw-axi-dmac


On Wed, 27 Mar 2024 10:49:24 +0000, Joao Pinto wrote:
> I have a use case where nr_buffers = 3 and in which each descriptor is composed by 3
> segments, resulting in the DMA channel descs_allocated to be 9. Since axi_desc_put()
> handles the hw_desc considering the descs_allocated, this scenario would result in a
> kernel panic (hw_desc array will be overrun).
>
> To fix this, the proposal is to add a new member to the axi_dma_desc structure,
> where we keep the number of allocated hw_descs (axi_desc_alloc()) and use it in
> axi_desc_put() to handle the hw_desc array correctly.
>
> [...]

Applied, thanks!

[1/1] Avoid hw_desc array overrun in dw-axi-dmac
commit: 333e11bf47fa8d477db90e2900b1ed3c9ae9b697

Best regards,
--
~Vinod