public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
From: Christian König <christian.koenig@amd.com>
To: Albert Esteve <aesteve@redhat.com>
Cc: Sumit Semwal <sumit.semwal@linaro.org>,
	Benjamin Gaignard <benjamin.gaignard@collabora.com>,
	Brian Starkey <Brian.Starkey@arm.com>,
	John Stultz <jstultz@google.com>,
	"T.J. Mercier" <tjmercier@google.com>,
	Shuah Khan <shuah@kernel.org>,
	linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org, mripard@kernel.org
Subject: Re: [PATCH 0/2] dma-buf: add DMA_BUF_IOCTL_DERIVE for reduced-permission aliases
Date: Thu, 21 May 2026 15:14:51 +0200	[thread overview]
Message-ID: <e4fe1485-3b66-4f99-ae8d-18db140d3fa6@amd.com> (raw)
In-Reply-To: <CADSE00J1+V4=qFZPOL8Cr8hYw6d=hf_XcDPezjYGkcxAwLYGyw@mail.gmail.com>

On 5/21/26 15:01, Albert Esteve wrote:
> Hi Christian,
> 
> On Thu, May 21, 2026 at 2:28 PM Christian König
> <christian.koenig@amd.com> wrote:
>>
>> On 5/21/26 11:10, Albert Esteve wrote:
>>> When sharing a dma-buf between components of different trust levels, the
>>> allocator may need to hand a consumer a read-only view of a buffer it
>>> holds with read-write access. An example is a camera pipeline where the
>>> capture component writes frames into a buffer and needs to pass a
>>> read-only handle to a downstream processing component that should not be
>>> able to modify the data.
>>>
>>> However, no such mechanism exists today. The access mode of a dma-buf
>>> file descriptor is fixed at export time, and the standard POSIX
>>> interfaces for duplicating or changing file descriptors (i.e., dup(2),
>>> dup3(2), and fcntl(F_SETFL)) cannot alter the read/write access mode of
>>> the copy.
>>>
>>> One natural candidate would be reopening via /proc/self/fd/<N> with
>>> O_RDONLY, which works for regular files. For dma-buf this would fail
>>> (that is, if we were to add a new handler for open f_op) with ENXIO
>>> because the dmabuf pseudo-filesystem carries SB_NOUSER, which prevents
>>> the VFS from opening its files through path-based resolution from
>>> userspace.
>>
>> OH MY GOD! This is the like the sixth time I had to clarify that in the last few weeks, I'm really wondering where that is suddenly coming from.
> 
> Sorry! I do not know where others came from. But my interest comes
> from automotive, safety, and mixed criticality scenarios. I kind of
> hinted at that in the opening when referring to "different trust
> levels".

AH! Yeah, automotive is most likely the common topic in all those requests.

>>
>> Creating the DMA-buf with O_RDONLY does *NOT* make the DMA-buf itself read only!
>>
>> That's a really common misconception. The flag only controls if mmap() can be done read/write or read-only to handle cache coherency issues.
>>
>> It is still perfectly possible for a device to write into a DMA-buf created with O_RDONLY with DMA!
>>
>> So long story short there is not such feature as a read only DMA-buf, and putting read-only pages into a DMA-buf and then expecting that nobody can write to them is an absolutely clear No-Go.
>>
>> If we would want to implement a read-only DMA-buf feature we would need to go over all the different DMA-buf importers in the kernel and add security checks.
> 
> This clarifies a lot. Too bad, but it makes sense. I will abandon the
> series then.
> 
> Thanks for the review and the explanation!

No problem, I was just really surprised that this came up once more.

Just for completeness: What some exporters do is to reject read/write mmap calls with O_RDONLY and map_dma_buf() callbacks with DMA_BIDIRECTIONAL or DMA_TO_DEVICE.

But as I said this is just to catch cache coherency issues and not access control.

If I'm not completely mistaken some HW actually can't even guarantee read only mappings, in other words even if you say don't write to that buffer in the kernel you could submit shader or DMA commands from userspace which does exactly that and it works.

So I don't really see a chance for that feature to fly as general DMA-buf thing. Maybe between two specific exporters/importers could work, but yeah...

Regards,
Christian.


> 
> BR,
> Albert
> 
>>
>> Regards,
>> Christian.
>>
>>
>>>
>>> Alternatively, exporting the buffer twice would produce two independent
>>> dma_buf instances, which breaks fence synchronization.
>>>
>>> Therefore we add a new DMA_BUF_IOCTL_DERIVE ioctl, which produces a new
>>> file descriptor for an existing dma-buf with a caller-specified subset
>>> of the original permissions:
>>>
>>> ```
>>>   struct dma_buf_derive { __u32 flags; __s32 fd; };
>>>
>>>   struct dma_buf_derive req = { .flags = O_RDONLY | O_CLOEXEC };
>>>   ioctl(rw_fd, DMA_BUF_IOCTL_DERIVE, &req);
>>>   /* req.fd is now a read-only alias of the same buffer */
>>> ```
>>>
>>> Permission escalation is rejected with -EACCES. The new fd aliases the
>>> same struct dma_buf as the original, same dma_resv, same exporter ops,
>>> same underlying memory; so importers attaching to either fd see the same
>>> fence timeline and operate on the same object. Access control for which
>>> components may receive or pass on restricted descriptors can be layered on
>>> top via SELinux file:read and file:write permissions.
>>>
>>> A shared writable mapping (PROT_WRITE | MAP_SHARED) on the read-only fd is
>>> rejected with -EACCES in dma_buf_mmap_internal().
>>>
>>> Two small internal adjustments accompany the ioctl:
>>> - __dma_buf_list_del() is moved to dma_buf_release() so it fires exactly
>>>   once on dentry destruction rather than on every file close.
>>> - dma_buf_file_release() is updated to call dma_buf_put() only for
>>>   files that are not the primary dma-buf file.
>>>
>>> This may not be the best approach, but after considering different
>>> options and alternatives (as described above), we decided to raise the
>>> discussion upstream. Thus, we welcome any alternative proposal or ideas.
>>>
>>> The series is structured as:
>>> - Patch 1 adds the new ioctl implementation.
>>> - Patch 2 adds selftests covering the new ioctl.
>>>
>>> Signed-off-by: Albert Esteve <aesteve@redhat.com>
>>> ---
>>> Albert Esteve (2):
>>>       dma-buf: add DMA_BUF_IOCTL_DERIVE for reduced-permission aliases
>>>       selftests: dma-buf: add DERIVE ioctl tests
>>>
>>>  drivers/dma-buf/dma-buf.c                          |  58 ++++++++++-
>>>  include/uapi/linux/dma-buf.h                       |  28 +++++
>>>  tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c | 114 ++++++++++++++++++++-
>>>  3 files changed, 198 insertions(+), 2 deletions(-)
>>> ---
>>> base-commit: ab5fce87a778cb780a05984a2ca448f2b41aafbf
>>> change-id: 20260520-dmabuf-limit-access-73261353841a
>>>
>>> Best regards,
>>
> 


  reply	other threads:[~2026-05-21 13:15 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-21  9:10 [PATCH 0/2] dma-buf: add DMA_BUF_IOCTL_DERIVE for reduced-permission aliases Albert Esteve
2026-05-21  9:10 ` [PATCH 1/2] " Albert Esteve
2026-05-21 12:30   ` Christian König
2026-05-25 10:27   ` Claude review: " Claude Code Review Bot
2026-05-21  9:10 ` [PATCH 2/2] selftests: dma-buf: add DERIVE ioctl tests Albert Esteve
2026-05-25 10:27   ` Claude review: " Claude Code Review Bot
2026-05-21 12:28 ` [PATCH 0/2] dma-buf: add DMA_BUF_IOCTL_DERIVE for reduced-permission aliases Christian König
2026-05-21 13:01   ` Albert Esteve
2026-05-21 13:14     ` Christian König [this message]
2026-05-25 10:27 ` Claude review: " Claude Code Review Bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e4fe1485-3b66-4f99-ae8d-18db140d3fa6@amd.com \
    --to=christian.koenig@amd.com \
    --cc=Brian.Starkey@arm.com \
    --cc=aesteve@redhat.com \
    --cc=benjamin.gaignard@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jstultz@google.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mripard@kernel.org \
    --cc=shuah@kernel.org \
    --cc=sumit.semwal@linaro.org \
    --cc=tjmercier@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox