public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] dma-buf: set SB_I_NOEXEC on the pseudo filesystem
@ 2026-05-20 21:43 John Hubbard
  2026-05-21 11:54 ` Christian Brauner
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: John Hubbard @ 2026-05-20 21:43 UTC (permalink / raw)
  To: Sumit Semwal, Christian König
  Cc: Christian Brauner, Jens Axboe, linux-media, dri-devel,
	linaro-mm-sig, LKML, John Hubbard, stable

The dma-buf pseudo filesystem dispenses S_ANON_INODE inodes via
alloc_anon_inode() but never sets SB_I_NOEXEC on its superblock.
Since commit 1e7ab6f67824 ("anon_inode: rework assertions") in 6.17,
path_noexec() warns on exactly that combination, so an mmap() on any
dma-buf fd trips the warning:

  WARNING: CPU: 11 PID: 121813 at fs/exec.c:118 path_noexec+0x47/0x50
   do_mmap+0x2b5/0x680
   vm_mmap_pgoff+0x129/0x210
   ksys_mmap_pgoff+0x177/0x240
   __x64_sys_mmap+0x33/0x70

dma-bufs have no business being executable, which is the invariant
that the new assertion is enforcing. Set SB_I_NOEXEC on the dmabuf
superblock.

Reproducer on a CONFIG_DEBUG_VFS=y kernel:

  make -C tools/testing/selftests/dmabuf-heaps
  sudo ./tools/testing/selftests/dmabuf-heaps/dmabuf-heap -t system

The selftest allocates from /dev/dma_heap/system and mmaps the
returned fd, which trips the warning without this patch.

Fixes: 1e7ab6f67824 ("anon_inode: rework assertions")
Cc: stable@vger.kernel.org
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/dma-buf/dma-buf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 71f37544a5c6..d86a99d7b8dc 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -216,6 +216,7 @@ static int dma_buf_fs_init_context(struct fs_context *fc)
 	if (!ctx)
 		return -ENOMEM;
 	ctx->dops = &dma_buf_dentry_ops;
+	fc->s_iflags |= SB_I_NOEXEC;
 	return 0;
 }
 
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] dma-buf: set SB_I_NOEXEC on the pseudo filesystem
  2026-05-20 21:43 [PATCH] dma-buf: set SB_I_NOEXEC on the pseudo filesystem John Hubbard
@ 2026-05-21 11:54 ` Christian Brauner
  2026-05-21 22:54   ` John Hubbard
  2026-05-25 11:11 ` Claude review: " Claude Code Review Bot
  2026-05-25 11:11 ` Claude Code Review Bot
  2 siblings, 1 reply; 5+ messages in thread
From: Christian Brauner @ 2026-05-21 11:54 UTC (permalink / raw)
  To: John Hubbard
  Cc: Sumit Semwal, Christian König, Jens Axboe, linux-media,
	dri-devel, linaro-mm-sig, LKML, stable

On Wed, May 20, 2026 at 02:43:50PM -0700, John Hubbard wrote:
> The dma-buf pseudo filesystem dispenses S_ANON_INODE inodes via
> alloc_anon_inode() but never sets SB_I_NOEXEC on its superblock.
> Since commit 1e7ab6f67824 ("anon_inode: rework assertions") in 6.17,
> path_noexec() warns on exactly that combination, so an mmap() on any
> dma-buf fd trips the warning:
> 
>   WARNING: CPU: 11 PID: 121813 at fs/exec.c:118 path_noexec+0x47/0x50
>    do_mmap+0x2b5/0x680
>    vm_mmap_pgoff+0x129/0x210
>    ksys_mmap_pgoff+0x177/0x240
>    __x64_sys_mmap+0x33/0x70
> 
> dma-bufs have no business being executable, which is the invariant
> that the new assertion is enforcing. Set SB_I_NOEXEC on the dmabuf
> superblock.
> 
> Reproducer on a CONFIG_DEBUG_VFS=y kernel:
> 
>   make -C tools/testing/selftests/dmabuf-heaps
>   sudo ./tools/testing/selftests/dmabuf-heaps/dmabuf-heap -t system
> 
> The selftest allocates from /dev/dma_heap/system and mmaps the
> returned fd, which trips the warning without this patch.
> 
> Fixes: 1e7ab6f67824 ("anon_inode: rework assertions")
> Cc: stable@vger.kernel.org
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---

Perfect, the asserts are paying off. Thanks!
Reviewed-by: Christian Brauner (Amutable) <brauner@kernel.org>

>  drivers/dma-buf/dma-buf.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 71f37544a5c6..d86a99d7b8dc 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -216,6 +216,7 @@ static int dma_buf_fs_init_context(struct fs_context *fc)
>  	if (!ctx)
>  		return -ENOMEM;
>  	ctx->dops = &dma_buf_dentry_ops;
> +	fc->s_iflags |= SB_I_NOEXEC;

While you're at it, also raise SB_I_NODEV. You're not creating any
device nodes and this is additional hardening.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] dma-buf: set SB_I_NOEXEC on the pseudo filesystem
  2026-05-21 11:54 ` Christian Brauner
@ 2026-05-21 22:54   ` John Hubbard
  0 siblings, 0 replies; 5+ messages in thread
From: John Hubbard @ 2026-05-21 22:54 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Sumit Semwal, Christian König, Jens Axboe, linux-media,
	dri-devel, linaro-mm-sig, LKML, stable

On 5/21/26 4:54 AM, Christian Brauner wrote:
> On Wed, May 20, 2026 at 02:43:50PM -0700, John Hubbard wrote:
>> The dma-buf pseudo filesystem dispenses S_ANON_INODE inodes via
>> alloc_anon_inode() but never sets SB_I_NOEXEC on its superblock.
>> Since commit 1e7ab6f67824 ("anon_inode: rework assertions") in 6.17,
>> path_noexec() warns on exactly that combination, so an mmap() on any
>> dma-buf fd trips the warning:
>>
>>   WARNING: CPU: 11 PID: 121813 at fs/exec.c:118 path_noexec+0x47/0x50
>>    do_mmap+0x2b5/0x680
>>    vm_mmap_pgoff+0x129/0x210
>>    ksys_mmap_pgoff+0x177/0x240
>>    __x64_sys_mmap+0x33/0x70
>>
>> dma-bufs have no business being executable, which is the invariant
>> that the new assertion is enforcing. Set SB_I_NOEXEC on the dmabuf
>> superblock.
>>
>> Reproducer on a CONFIG_DEBUG_VFS=y kernel:
>>
>>   make -C tools/testing/selftests/dmabuf-heaps
>>   sudo ./tools/testing/selftests/dmabuf-heaps/dmabuf-heap -t system
>>
>> The selftest allocates from /dev/dma_heap/system and mmaps the
>> returned fd, which trips the warning without this patch.
>>
>> Fixes: 1e7ab6f67824 ("anon_inode: rework assertions")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
>> ---
> 
> Perfect, the asserts are paying off. Thanks!
> Reviewed-by: Christian Brauner (Amutable) <brauner@kernel.org>

Thanks for the review!

> 
>>  drivers/dma-buf/dma-buf.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index 71f37544a5c6..d86a99d7b8dc 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -216,6 +216,7 @@ static int dma_buf_fs_init_context(struct fs_context *fc)
>>  	if (!ctx)
>>  		return -ENOMEM;
>>  	ctx->dops = &dma_buf_dentry_ops;
>> +	fc->s_iflags |= SB_I_NOEXEC;
> 
> While you're at it, also raise SB_I_NODEV. You're not creating any
> device nodes and this is additional hardening.

OK, I'll add that and send out a v2, after collecting any other
feedback on this one.

thanks,
-- 
John Hubbard


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Claude review: dma-buf: set SB_I_NOEXEC on the pseudo filesystem
  2026-05-20 21:43 [PATCH] dma-buf: set SB_I_NOEXEC on the pseudo filesystem John Hubbard
  2026-05-21 11:54 ` Christian Brauner
@ 2026-05-25 11:11 ` Claude Code Review Bot
  2026-05-25 11:11 ` Claude Code Review Bot
  2 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 11:11 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: dma-buf: set SB_I_NOEXEC on the pseudo filesystem
Author: John Hubbard <jhubbard@nvidia.com>
Patches: 3
Reviewed: 2026-05-25T21:11:46.873174

---

This is a single-patch fix, not a series. The patch is **correct, well-motivated, and straightforward**. It fixes a real regression introduced by commit `1e7ab6f67824` ("anon_inode: rework assertions") in 6.17, where `path_noexec()` now warns (via `VFS_WARN_ON_ONCE`) when an `S_ANON_INODE` inode is encountered on a superblock that lacks `SB_I_NOEXEC`. The dma-buf pseudo filesystem uses `alloc_anon_inode()` (via `init_pseudo`) but never set `SB_I_NOEXEC`, triggering the warning on any dma-buf mmap.

The fix follows the established pattern used by every other pseudo filesystem in the kernel that issues anonymous inodes (`anon_inodes.c`, `aio.c`, `secretmem.c`, `guest_memfd.c`, `pidfs.c`, `nsfs.c`, etc.) — all set `fc->s_iflags |= SB_I_NOEXEC` in their `init_fs_context` callback.

The `Fixes:` tag and `Cc: stable` are appropriate since this is a regression fix for a new assertion.

No concerns with this patch.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Claude review: dma-buf: set SB_I_NOEXEC on the pseudo filesystem
  2026-05-20 21:43 [PATCH] dma-buf: set SB_I_NOEXEC on the pseudo filesystem John Hubbard
  2026-05-21 11:54 ` Christian Brauner
  2026-05-25 11:11 ` Claude review: " Claude Code Review Bot
@ 2026-05-25 11:11 ` Claude Code Review Bot
  2 siblings, 0 replies; 5+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 11:11 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness: Good.** The one-line change is placed at the right location — inside `dma_buf_fs_init_context()`, after `init_pseudo()` succeeds and before returning. The field `fc->s_iflags` is defined in `struct fs_context` (at `include/linux/fs_context.h:106`) and is documented as being OR'd into `sb->s_iflags` during superblock creation, so this will propagate correctly.

```c
 	ctx->dops = &dma_buf_dentry_ops;
+	fc->s_iflags |= SB_I_NOEXEC;
 	return 0;
```

This exactly mirrors the pattern in `anon_inodefs_init_fs_context()` at `fs/anon_inodes.c:89`:
```c
	fc->s_iflags |= SB_I_NOEXEC;
```

**Commit message: Good.** The commit message clearly explains:
- What the bug is (missing `SB_I_NOEXEC` on dmabuf superblock)
- What triggers it (the new `path_noexec()` assertion from 6.17)
- The exact warning stack trace
- A concrete reproducer using the dma_heap selftest
- Why the fix is correct (dma-bufs have no business being executable)

**Fixes tag: Correct.** Points to `1e7ab6f67824` which introduced the assertion that now fires. This is the right commit to cite since dma-buf was working before that assertion was added.

**Minor nit (non-blocking):** The placement of `fc->s_iflags |= SB_I_NOEXEC` after `ctx->dops = &dma_buf_dentry_ops` works but conventionally other filesystems tend to set the iflags before other context setup (as in `anon_inodes.c`). This is purely cosmetic and doesn't affect correctness at all.

**Verdict: Looks good.** Clean, minimal, correct fix. Ready to merge.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-05-25 11:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-20 21:43 [PATCH] dma-buf: set SB_I_NOEXEC on the pseudo filesystem John Hubbard
2026-05-21 11:54 ` Christian Brauner
2026-05-21 22:54   ` John Hubbard
2026-05-25 11:11 ` Claude review: " Claude Code Review Bot
2026-05-25 11:11 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox