Re: [PATCH] fs: allow vfs code to open an O_PATH file with negative dentry

On Thu, Mar 19, 2026 at 4:55 PM Paul Moore [off-list ref] wrote:

On Thu, Mar 19, 2026 at 10:50 AM Amir Goldstein [off-list ref] wrote:

quoted

On Thu, Mar 19, 2026 at 2:13 PM Christian Brauner [off-list ref] wrote:

quoted

On Thu, Mar 19, 2026 at 01:46:16PM +0100, Amir Goldstein wrote:

quoted

The fields f_mapping, f_wb_err, f_sb_err are irrelevant for O_PATH file.
Skip setting them for O_PATH file, so that the O_PATH file could be
opened with a negative dentry.

This is not something that a user should be able to do, but vfs code,
such as ovl_tmpfile() can use this to open a backing O_PATH tmpfile
before instantiating the dentry.

Reported-by: syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Christian,

This patch fixes the syzbot report [1] that the
backing_file_user_path_file() patch [2] introduces.

This is not the only possible fix, but it is the cleanest one IMO.
There is a small risk in introducing a state of an O_PATH file with
NULL f_inode, but I (and the bots that I asked) did not find any
obvious risk in this state.

Note that specifically, the user path inode is accessed via d_inode()
and not via file_inode(), which makes this safe for file_user_inode()
callers.

BTW, I missed this regression with the original patch because I
only ran the quick overlayfs sanity test.

Now I ran a full quick fstest cycle and verified that the O_TMPFILE
test case is covered and that the bug is detected.

WDYT?

Thanks,
Amir.

[1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
[2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/ (local)

 fs/open.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/open.c b/fs/open.c
index 91f1139591abe..2004a8c0d9c97 100644
--- a/fs/open.c
+++ b/fs/open.c

@@ -893,9 +893,6 @@ static int do_dentry_open(struct file *f,

      path_get(&f->f_path);
      f->f_inode = inode;
-     f->f_mapping = inode->i_mapping;
-     f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
-     f->f_sb_err = file_sample_sb_err(f);

      if (unlikely(f->f_flags & O_PATH)) {
              f->f_mode = FMODE_PATH | FMODE_OPENED;

@@ -904,6 +901,10 @@ static int do_dentry_open(struct file *f,
              return 0;
      }

+     f->f_mapping = inode->i_mapping;
+     f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
+     f->f_sb_err = file_sample_sb_err(f);
+
      if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
              i_readcount_inc(inode);
      } else if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {

I think this is really ugly and I'm really unhappy that we should adjust
initialization of generic vfs code for this. My preference is to push
the pain into the backing file stuff. And my ultimate preference is for
this backing file stuff to be removed again for a simple struct path.
We're working around some more fundamental cleanup here imho.

Fair enough, we can rip the entire thing from vfs if you don't like it.
The user path file can be opened and stored internally by selinux
without adding all the associated risks in vfs.

Paul,

Please see compile tested code at:
https://github.com/amir73il/linux/commits/user_path_file/

No.  Definitely no.  Ignoring the fact that there is no reason we
should pushing this into the LSM, doing it in this way means it is
very likely that each LSM wanting to provide mmap/mprotect controls on
overlayfs will have to create a new O_PATH file.  No.

... and let me preemptively comment that this doesn't belong in the
LSM framework either.

As Christian already mentioned, this really needs to be addressed in
the backing file code, please do it there.

OK, will give it another try.

Thanks,
Amir.