linux-kernel - Re: Question: partial transfers of DMABUFs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 15 Feb 2023 13:52:24 +0000
From:   Paul Cercueil <paul@...pouillou.net>
To:     Christian König <christian.koenig@....com>,
        Sumit Semwal <sumit.semwal@...aro.org>,
        Vinod Koul <vkoul@...nel.org>
Cc:     linux-media@...r.kernel.org, dmaengine@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: Question: partial transfers of DMABUFs

Le mercredi 15 février 2023 à 14:46 +0100, Christian König a écrit :
> Am 15.02.23 um 14:24 schrieb Paul Cercueil:
> > Hi Christian,
> > 
> > Le mercredi 15 février 2023 à 13:58 +0100, Christian König a
> > écrit :
> > > Hi Paul,
> > > 
> > > Am 15.02.23 um 11:48 schrieb Paul Cercueil:
> > > > Hi,
> > > > 
> > > > I am working on adding support for DMABUFs in the IIO
> > > > subsystem.
> > > > 
> > > > One thing we want there, is the ability to specify the number
> > > > of
> > > > bytes
> > > > to transfer (while still defaulting to the DMABUF size).
> > > > 
> > > > Since dma_buf_map_attachment() returns a sg_table,
> > > Please don't assume that this is an sg_table. We just used it as
> > > container for DMA addresses, but this has proven to be a mistake.
> > TL/DR, why was it a mistake? Just curious.
> 
> The sg_table should have just contained DMA addresses, but we had 
> multiple people who tried to use the pages instead.
> 
> This works to some extend, but goes boom as soon as somebody messes
> with 
> the pages reference counts or tries to map it into an address space
> or 
> something like that.
> 
> We got so far that we now intentionally mangle the page addresses in
> the 
> sg_table to prevent people from using it: 
> https://elixir.bootlin.com/linux/latest/source/drivers/dma-buf/dma-buf.c#L763

Isn't that breaking the chains though? I'd expect page_link to be
mangled only if !sg_is_chain(sg).

> > > There is work underway to replace the sg_table with (for example)
> > > just
> > > an array of DMA addresses.
> > Ok, so I believe at some point we will need an equivalent of
> > dmaengine_prep_slave_sg() which takes an array of DMA addresses.
> 
> Well we will probably come up with a new container for this, but
> yeah.

Understood.

You said there was work underway, could you point me to the
corresponding mailing list threads and/or code?

> Regards,
> Christian.

Cheers,
-Paul

> > 
> > > > I basically have two options, and I can't decide which one is
> > > > the
> > > > best (or the less ugly):
> > > > 
> > > > - Either I add a new API function similar to
> > > > dmaengine_prep_slave_sg(),
> > > > which still takes a scatterlist as argument but also takes the
> > > > number
> > > > of bytes as argument;
> > > > 
> > > > - Or I add a function to duplicate the scatterlist and then
> > > > shrink
> > > > it
> > > > manually, which doesn't sound like a good idea either.
> > > > 
> > > > What would be the recommended way?
> > > I strongly recommend to come up with a new function which only
> > > takes
> > > DMA
> > > addresses and separate segment length.
> > Alright, thanks for your input.
> > 
> > So I would add a new dma_device.dma_prep_slave_dma_array() callback
> > with a corresponding API function, and then the drivers can be
> > converted from using .dma_prep_slave_sg() to this new function in
> > due
> > time.
> > 
> > Vinod, that works for you?
> > 
> > Cheers,
> > -Paul
>