public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Christian Brauner <brauner@kernel.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	Miklos Szeredi <miklos@szeredi.hu>,
	 Paul Moore <paul@paul-moore.com>, Gao Xiang <xiang@kernel.org>,
	 linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	 linux-erofs@lists.ozlabs.org, linux-fsdevel@vger.kernel.org,
	 linux-unionfs@vger.kernel.org,
	 syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com
Subject: Re: [PATCH v2] fs: allow backing file code to open an O_PATH file with negative dentry
Date: Fri, 20 Mar 2026 13:33:55 +0100	[thread overview]
Message-ID: <CAOQ4uxhsTG0mQCeiAw-8qGqjQo9HjRYfdRYQzPGXSgKb4udOiw@mail.gmail.com> (raw)
In-Reply-To: <20260320122918.1726043-1-amir73il@gmail.com>

On Fri, Mar 20, 2026 at 1:29 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> The fields f_mapping and f_wb_err are irrelevant for the O_PATH file
> in backing_file_user_path_file().
>
> Create a dedicated helper kernel_path_file_open(), which skips all the
> generic code in do_dentry_open() and does only the essentials, so that
> the internal O_PATH file could be opened with a negative dentry.
>
> This is needed for backing_tmpfile_open() to open a backing O_PATH
> tmpfile before instantiating the dentry.
>
> The callers of backing_tmpfile_open() are responsible for calling
> backing_tmpfile_finish() after making the path positive.
>
> 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.

Sorry, forgot to mention that this patch applies on top of [2]
so the vfs CI is likely to fail to apply it.

The previous patch + fix are also available on my github
https://github.com/amir73il/linux/commits/user_path_file/

Thanks,
Amir.

>
> Following your feedback on v1, this version makes an effort to stay
> out of the way of main vfs execution paths and restrict the changes
> to backing_file users.
>
> This still introduced a temporary state of an O_PATH file with negative
> path, but only for a short time and only for backing_file users and
> ones that use backing_tmpfile_open() (i.e. only overlayfs), so the
> risk is minimal.
>
> WDYT?
>
> Thanks,
> Amir.
>
> Changes since v1:
> - Create helper for internal O_PATH open with negative path
> - Create backing_tmpfile_finish() API to fixup the negative path
>
> [1] https://syzkaller.appspot.com/bug?extid=f34aab278bf5d664e2be
> [2] https://lore.kernel.org/linux-fsdevel/20260318131258.1457101-1-amir73il@gmail.com/
>
>  fs/backing-file.c            |  6 ++++++
>  fs/file_table.c              | 14 ++++++++++++--
>  fs/internal.h                |  7 +++++++
>  fs/open.c                    | 34 ++++++++++++++++++++++++++++++----
>  fs/overlayfs/dir.c           |  2 ++
>  include/linux/backing-file.h |  1 +
>  6 files changed, 58 insertions(+), 6 deletions(-)
>
> diff --git a/fs/backing-file.c b/fs/backing-file.c
> index d0a64c2103907..3357d624eac96 100644
> --- a/fs/backing-file.c
> +++ b/fs/backing-file.c
> @@ -80,6 +80,12 @@ struct file *backing_tmpfile_open(const struct path *user_path,
>  }
>  EXPORT_SYMBOL(backing_tmpfile_open);
>
> +void backing_tmpfile_finish(struct file *file)
> +{
> +       backing_file_set_user_path_inode(file);
> +}
> +EXPORT_SYMBOL_GPL(backing_tmpfile_finish);
> +
>  struct backing_aio {
>         struct kiocb iocb;
>         refcount_t ref;
> diff --git a/fs/file_table.c b/fs/file_table.c
> index e8b4eb2bbff85..a4d1064d50896 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -69,11 +69,21 @@ EXPORT_SYMBOL_GPL(backing_file_user_path_file);
>
>  int backing_file_open_user_path(struct file *f, const struct path *path)
>  {
> -       /* open an O_PATH file to reference the user path - should not fail */
> -       return WARN_ON(vfs_open(path, &backing_file(f)->user_path_file));
> +       if (WARN_ON(!(f->f_mode & FMODE_BACKING)))
> +               return -EIO;
> +       kernel_path_file_open(&backing_file(f)->user_path_file, path);
> +       return 0;
>  }
>  EXPORT_SYMBOL_GPL(backing_file_open_user_path);
>
> +void backing_file_set_user_path_inode(struct file *f)
> +{
> +       if (WARN_ON(!(f->f_mode & FMODE_BACKING)))
> +               return;
> +       file_set_d_inode(&backing_file(f)->user_path_file);
> +}
> +EXPORT_SYMBOL_GPL(backing_file_set_user_path_inode);
> +
>  static void destroy_file(struct file *f)
>  {
>         security_file_free(f);
> diff --git a/fs/internal.h b/fs/internal.h
> index 7c44a58627ba3..4a9e5e00678d9 100644
> --- a/fs/internal.h
> +++ b/fs/internal.h
> @@ -109,6 +109,13 @@ struct file *alloc_empty_file_noaccount(int flags, const struct cred *cred);
>  struct file *alloc_empty_backing_file(int flags, const struct cred *cred,
>                                       const struct cred *user_cred);
>  int backing_file_open_user_path(struct file *f, const struct path *path);
> +void backing_file_set_user_path_inode(struct file *f);
> +void kernel_path_file_open(struct file *f, const struct path *path);
> +
> +static inline void file_set_d_inode(struct file *f)
> +{
> +       f->f_inode = d_inode(f->f_path.dentry);
> +}
>
>  static inline void file_put_write_access(struct file *file)
>  {
> diff --git a/fs/open.c b/fs/open.c
> index 91f1139591abe..a7b3b04cd9ae7 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -884,10 +884,38 @@ static inline int file_get_write_access(struct file *f)
>         return error;
>  }
>
> +static const struct file_operations empty_fops = {};
> +
> +static void do_path_file_open(struct file *f)
> +{
> +       f->f_mode = FMODE_PATH | FMODE_OPENED;
> +       file_set_fsnotify_mode(f, FMODE_NONOTIFY);
> +       f->f_op = &empty_fops;
> +}
> +
> +/**
> + * kernel_path_file_open - open an O_PATH file for kernel internal use
> + * @f:         pre-allocated file with f_flags and f_cred initialized
> + * @path:      path to reference (may have a negative dentry)
> + *
> + * Open a minimal O_PATH file that only references a path.
> + * Unlike vfs_open(), this does not require a positive dentry and does not
> + * set up f_mapping and other fields not needed for O_PATH.
> + * If path is negative at the time of this call, the caller is responsible for
> + * callingn backing_file_set_user_path_inode() after making the path positive.
> +
> + */
> +void kernel_path_file_open(struct file *f, const struct path *path)
> +{
> +       f->__f_path = *path;
> +       path_get(&f->f_path);
> +       file_set_d_inode(f);
> +       do_path_file_open(f);
> +}
> +
>  static int do_dentry_open(struct file *f,
>                           int (*open)(struct inode *, struct file *))
>  {
> -       static const struct file_operations empty_fops = {};
>         struct inode *inode = f->f_path.dentry->d_inode;
>         int error;
>
> @@ -898,9 +926,7 @@ static int do_dentry_open(struct file *f,
>         f->f_sb_err = file_sample_sb_err(f);
>
>         if (unlikely(f->f_flags & O_PATH)) {
> -               f->f_mode = FMODE_PATH | FMODE_OPENED;
> -               file_set_fsnotify_mode(f, FMODE_NONOTIFY);
> -               f->f_op = &empty_fops;
> +               do_path_file_open(f);
>                 return 0;
>         }
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 5fd32ccc134d2..4010c87e10196 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -1408,6 +1408,8 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
>                         err = ovl_instantiate(dentry, inode, newdentry, false, file);
>                         if (!err) {
>                                 file->private_data = of;
> +                               /* user_path_file was opened with a negative path */
> +                               backing_tmpfile_finish(realfile);
>                         } else {
>                                 dput(newdentry);
>                                 ovl_file_free(of);
> diff --git a/include/linux/backing-file.h b/include/linux/backing-file.h
> index 8afba93f3ce07..52ac51ada6ff9 100644
> --- a/include/linux/backing-file.h
> +++ b/include/linux/backing-file.h
> @@ -47,6 +47,7 @@ struct file *backing_tmpfile_open(const struct path *user_path,
>                                   const struct cred *user_cred, int flags,
>                                   const struct path *real_parentpath,
>                                   umode_t mode, const struct cred *cred);
> +void backing_tmpfile_finish(struct file *file);
>  ssize_t backing_file_read_iter(struct file *file, struct iov_iter *iter,
>                                struct kiocb *iocb, int flags,
>                                struct backing_file_ctx *ctx);
> --
> 2.53.0
>

      reply	other threads:[~2026-03-20 12:34 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-20 12:29 [PATCH v2] fs: allow backing file code to open an O_PATH file with negative dentry Amir Goldstein
2026-03-20 12:33 ` Amir Goldstein [this message]

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=CAOQ4uxhsTG0mQCeiAw-8qGqjQo9HjRYfdRYQzPGXSgKb4udOiw@mail.gmail.com \
    --to=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=linux-erofs@lists.ozlabs.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=paul@paul-moore.com \
    --cc=selinux@vger.kernel.org \
    --cc=syzbot+f34aab278bf5d664e2be@syzkaller.appspotmail.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=xiang@kernel.org \
    /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