Re: XDMA crashes on zero-length sg entries

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Martin,

On 3/17/23 04:18, Martin Tůma wrote:
Hi

On 16. 03. 23 19:25, Lizhi Hou wrote:
Hi Martin,

Based on the dmaengine document

https://kernel.org/doc/html/v6.3-rc2/driver-api/dmaengine/client.html

The sg_len used for dmaengine_prep_slave_sg should be returned by dma_map_sg.

This implies there should not be zero segment for the first sg_len segments.


I do get the mapping from vb2_dma_sg_plane_desc() which internally
uses dma_map_sg_attrs() and yet in some special cases sg_dma_len()
returns zero. So this is a "real-life" scenario. I'm still investigating
This sounds odd to me. dma_map_sg() is supposed to return number of Non-zero sg because the zero length sg will be merged.
what the root cause is for getting such empty mappings from v4l2, but
the DMA driver should IMHO not crash the kernel in such case anyway.

I checked some dma driver and I did not see obvious code to check the sg len. e.g.

https://elixir.bootlin.com/linux/v6.3-rc2/source/drivers/dma/k3dma.c#L531

I have looked into some randomly chosen kernel code that uses
sg_dma_len() and all usages that I have seen are prepared for the case
the length is zero. All but one driver simply stop the iteration while
the remaining one reported this case as an error.

Are those dma drivers?

Adding a check sounds fine to me. And I am going to create a patch anyway.  :)


Thanks,

Lizhi


M.

I think that is why we need to provide 'sg_len' for dmaengine_prep_slave_sg().

With 'sg_len', we do not need to worry about zero length sg in the driver callback.


Thanks,

Lizhi

On 3/16/23 10:03, Martin Tůma wrote:
Hi,
The Xilinx XDMA driver crashes when the scatterlist provided to
xdma_prep_device_sg() contains an empty entry, i.e. sg_dma_len()
returns zero. As I do get such sgl from v4l2 I suppose that this
is a valid scenario and not a bug in our parent mgb4 driver. Also
the documentation for sg_dma_len() suggests that there may be
zero-length entries.

The following trivial patch fixes the crash:

diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
index 462109c61653..cd5fcd911c50 100644
--- a/drivers/dma/xilinx/xdma.c
+++ b/drivers/dma/xilinx/xdma.c
@@ -487,6 +487,8 @@ xdma_prep_device_sg(struct dma_chan *chan, struct scatterlist *sgl,
        for_each_sg(sgl, sg, sg_len, i) {
                addr = sg_dma_address(sg);
                rest = sg_dma_len(sg);
+               if (!rest)
+                       break;

                do {
                        len = min_t(u32, rest, XDMA_DESC_BLEN_MAX);

M.




[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux