linux-kernel - Re: [PATCH v2 0/3] udmabuf: Sync to attached devices

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] [day] [month] [year] [list]

Message-ID: <008f6004-dcf0-42e8-b2df-f97c0ee5ba66@ti.com>
Date: Fri, 15 Aug 2025 14:40:16 -0500
From: Andrew Davis <afd@...com>
To: Christian König <christian.koenig@....com>,
        Gerd Hoffmann
	<kraxel@...hat.com>,
        Sumit Semwal <sumit.semwal@...aro.org>,
        Paul Cercueil
	<paul@...pouillou.net>,
        Vivek Kasireddy <vivek.kasireddy@...el.com>,
        "Daniel
 Vetter" <daniel@...ll.ch>
CC: <dri-devel@...ts.freedesktop.org>, <linux-media@...r.kernel.org>,
        <linaro-mm-sig@...ts.linaro.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 0/3] udmabuf: Sync to attached devices

On 8/15/25 4:41 AM, Christian König wrote:
> On 14.08.25 18:10, Andrew Davis wrote:
>> Hello all,
>>
>> This series makes it so the udmabuf will sync the backing buffer
>> with the set of attached devices as required for DMA-BUFs when
>> doing {begin,end}_cpu_access.
> 
> Yeah the reason why we didn't do that is that this doesn't even work 100% reliable in theory. So this patchset here might make your use case work but is a bit questionable in general.
> 
> udmabuf is about turning a file descriptor created by memfd_create() into a DMA-buf. Mapping that memory can happen through the memfd as well and so it is perfectly valid to skip the DMA-buf begin_access and end_access callbacks.
> 

If someone maps the memory backed by the DMA-buf outside of the DMA-APIs then we cannot really
control that, but in this case if they do map with the DMA-API then it is *not* valid to skip
these begin_access and end_access callbacks. And that is the case I am addressing here.

Right now we are not syncing the mapping for any attached device, we just zap it from
the CPU caches using some hacky loopback and hope that is enough for the devices :/

> Additional to that when CONFIG_DMABUF_DEBUG is enabled the DMA-buf code mangles the page addresses in the sg table to prevent importers from abusing it. That makes dma_sync_sgtable_for_cpu() and dma_sync_sgtable_for_device() on the exporter side crash.
> 

I was not aware of this mangle_sg_table() hack, must have been added while I was not looking :)

Seems very broken TBH, taking a quick look, I see on this line[0] you call it, then
just a couple lines later you use that same mangled page_link to walk the SG table[1]..

If anyone enables DMA_API_DEBUG and tried to attach/map a non-contiguous DMA-BUF with
a chained sg I don't see how that doesn't crash out.

> That's the reason why DMA-buf heaps uses a copy of the sg table for calling dma_sync_sgtable_for_cpu()/dma_sync_sgtable_for_device().
> 

Could you point me to where Heaps uses a copy of the SG table? I see it using the
exact same SG table for dma_sync_sgtable_for_*() that we created when mapping the
device attachments.

Thanks,
Andrew

[0] https://github.com/torvalds/linux/blob/master/drivers/dma-buf/dma-buf.c#L1142
[1] https://github.com/torvalds/linux/blob/master/drivers/dma-buf/dma-buf.c#L1151

> It's basically a hack and should be removed, but for this we need to change all clients which is tons of work.
> 
> Regards,
> Christian.
> 
>>
>> Thanks
>> Andrew
>>
>> Changes for v2:
>>   - fix attachment table use-after-free
>>   - rebased on v6.17-rc1
>>
>> Andrew Davis (3):
>>    udmabuf: Keep track current device mappings
>>    udmabuf: Sync buffer mappings for attached devices
>>    udmabuf: Use module_misc_device() to register this device
>>
>>   drivers/dma-buf/udmabuf.c | 133 +++++++++++++++++++-------------------
>>   1 file changed, 67 insertions(+), 66 deletions(-)
>>
>