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

[Christian Brauner <brauner@kernel.org>]

On Thu, Mar 19, 2026 at 01:46:16PM +0100, Amir Goldstein wrote:
> 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/
> 
>  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.