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

Amir Goldstein <[email protected]>
Newsgroups org.kernel.vger.linux-unionfs,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-security-module,org.kernel.vger.selinux,org.ozlabs.lists.linux-erofs
Message-ID <CAOQ4uxhsTG0mQCeiAw-8qGqjQo9HjRYfdRYQzPGXSgKb4udOiw@mail.gmail.com>
On Fri, Mar 20, 2026 at 1:29 PM Amir Goldstein <[email protected]> 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: [email protected]
> Signed-off-by: Amir Goldstein <[email protected]>
> ---
>
> 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/[email protected]/
>
>  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
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.