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 >