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
>
prev parent 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