[PATCH 1/3] backing_file: store user_path_file
Amir Goldstein
amir73il at gmail.com
Wed Mar 18 23:06:48 AEDT 2026
On Wed, Mar 18, 2026 at 11:57 AM Christian Brauner <brauner at kernel.org> wrote:
>
> On Mon, Mar 16, 2026 at 05:35:56PM -0400, Paul Moore wrote:
> > From: Amir Goldstein <amir73il at gmail.com>
> >
> > Instead of storing the user_path, store an O_PATH file for the
> > user_path with the original user file creds and a security context.
> >
> > The user_path_file is only exported as a const pointer and its refcnt
> > is initialized to FILE_REF_DEAD, because it is not a refcounted object.
> >
> > The only caller of file_ref_init() is now open coded, so the helper
> > is removed.
> >
> > Signed-off-by: Amir Goldstein <amir73il at gmail.com>
> > Tested-by: Paul Moore <paul at paul-moore.com> (SELinux)
> > Acked-by: Gao Xiang <hsiangkao at linux.alibaba.com> (EROFS)
> > Signed-off-by: Paul Moore <paul at paul-moore.com>
> > ---
> > fs/backing-file.c | 20 ++++++++------
> > fs/erofs/ishare.c | 6 ++--
> > fs/file_table.c | 53 ++++++++++++++++++++++++++++--------
> > fs/fuse/passthrough.c | 3 +-
> > fs/internal.h | 5 ++--
> > fs/overlayfs/dir.c | 3 +-
> > fs/overlayfs/file.c | 1 +
> > include/linux/backing-file.h | 29 ++++++++++++++++++--
> > include/linux/file_ref.h | 10 -------
> > 9 files changed, 90 insertions(+), 40 deletions(-)
> >
> > diff --git a/fs/backing-file.c b/fs/backing-file.c
> > index 45da8600d564..acabeea7efff 100644
> > --- a/fs/backing-file.c
> > +++ b/fs/backing-file.c
> > @@ -11,6 +11,7 @@
> > #include <linux/fs.h>
> > #include <linux/backing-file.h>
> > #include <linux/splice.h>
> > +#include <linux/uio.h>
> > #include <linux/mm.h>
> >
> > #include "internal.h"
> > @@ -18,9 +19,10 @@
> > /**
> > * backing_file_open - open a backing file for kernel internal use
> > * @user_path: path that the user reuqested to open
> > + * @user_cred: credentials that the user used for open
> > * @flags: open flags
> > * @real_path: path of the backing file
> > - * @cred: credentials for open
> > + * @cred: credentials for open of the backing file
> > *
> > * Open a backing file for a stackable filesystem (e.g., overlayfs).
> > * @user_path may be on the stackable filesystem and @real_path on the
> > @@ -29,19 +31,19 @@
> > * returned file into a container structure that also stores the stacked
> > * file's path, which can be retrieved using backing_file_user_path().
> > */
> > -struct file *backing_file_open(const struct path *user_path, int flags,
> > +struct file *backing_file_open(const struct path *user_path,
> > + const struct cred *user_cred, int flags,
> > const struct path *real_path,
> > const struct cred *cred)
> > {
> > struct file *f;
> > int error;
> >
> > - f = alloc_empty_backing_file(flags, cred);
> > + f = alloc_empty_backing_file(flags, cred, user_cred);
> > if (IS_ERR(f))
> > return f;
> >
> > - path_get(user_path);
> > - backing_file_set_user_path(f, user_path);
> > + backing_file_open_user_path(f, user_path);
> > error = vfs_open(real_path, f);
> > if (error) {
> > fput(f);
> > @@ -52,7 +54,8 @@ struct file *backing_file_open(const struct path *user_path, int flags,
> > }
> > EXPORT_SYMBOL_GPL(backing_file_open);
> >
> > -struct file *backing_tmpfile_open(const struct path *user_path, int flags,
> > +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)
> > {
> > @@ -60,12 +63,11 @@ struct file *backing_tmpfile_open(const struct path *user_path, int flags,
> > struct file *f;
> > int error;
> >
> > - f = alloc_empty_backing_file(flags, cred);
> > + f = alloc_empty_backing_file(flags, cred, user_cred);
> > if (IS_ERR(f))
> > return f;
> >
> > - path_get(user_path);
> > - backing_file_set_user_path(f, user_path);
> > + backing_file_open_user_path(f, user_path);
> > error = vfs_tmpfile(real_idmap, real_parentpath, f, mode);
> > if (error) {
> > fput(f);
> > diff --git a/fs/erofs/ishare.c b/fs/erofs/ishare.c
> > index 829d50d5c717..17a4941d4518 100644
> > --- a/fs/erofs/ishare.c
> > +++ b/fs/erofs/ishare.c
> > @@ -106,15 +106,15 @@ static int erofs_ishare_file_open(struct inode *inode, struct file *file)
> >
> > if (file->f_flags & O_DIRECT)
> > return -EINVAL;
> > - realfile = alloc_empty_backing_file(O_RDONLY|O_NOATIME, current_cred());
> > + realfile = alloc_empty_backing_file(O_RDONLY|O_NOATIME, current_cred(),
> > + file->f_cred);
> > if (IS_ERR(realfile))
> > return PTR_ERR(realfile);
> > ihold(sharedinode);
> > realfile->f_op = &erofs_file_fops;
> > realfile->f_inode = sharedinode;
> > realfile->f_mapping = sharedinode->i_mapping;
> > - path_get(&file->f_path);
> > - backing_file_set_user_path(realfile, &file->f_path);
> > + backing_file_open_user_path(realfile, &file->f_path);
> >
> > file_ra_state_init(&realfile->f_ra, file->f_mapping);
> > realfile->private_data = EROFS_I(inode);
> > diff --git a/fs/file_table.c b/fs/file_table.c
> > index aaa5faaace1e..b7dc94656c44 100644
> > --- a/fs/file_table.c
> > +++ b/fs/file_table.c
> > @@ -27,6 +27,7 @@
> > #include <linux/task_work.h>
> > #include <linux/swap.h>
> > #include <linux/kmemleak.h>
> > +#include <linux/backing-file.h>
> >
> > #include <linux/atomic.h>
> >
> > @@ -43,11 +44,11 @@ static struct kmem_cache *bfilp_cachep __ro_after_init;
> >
> > static struct percpu_counter nr_files __cacheline_aligned_in_smp;
> >
> > -/* Container for backing file with optional user path */
> > +/* Container for backing file with optional user path file */
> > struct backing_file {
> > struct file file;
> > union {
> > - struct path user_path;
> > + struct file user_path_file;
> > freeptr_t bf_freeptr;
> > };
> > };
> > @@ -56,24 +57,44 @@ struct backing_file {
> >
> > const struct path *backing_file_user_path(const struct file *f)
> > {
> > - return &backing_file(f)->user_path;
> > + return &backing_file(f)->user_path_file.f_path;
> > }
> > EXPORT_SYMBOL_GPL(backing_file_user_path);
> >
> > -void backing_file_set_user_path(struct file *f, const struct path *path)
> > +const struct file *backing_file_user_path_file(const struct file *f)
> > {
> > - backing_file(f)->user_path = *path;
> > + return &backing_file(f)->user_path_file;
> > +}
> > +EXPORT_SYMBOL_GPL(backing_file_user_path_file);
> > +
> > +void backing_file_open_user_path(struct file *f, const struct path *path)
>
> I think this is a bad idea. This should return an error but still
> WARN_ON(). Just make callers handle that error just like we do
> everywhere else.
OK.
>
> > +{
> > + /* open an O_PATH file to reference the user path - cannot fail */
> > + WARN_ON(vfs_open(path, &backing_file(f)->user_path_file));
> > +}
> > +EXPORT_SYMBOL_GPL(backing_file_open_user_path);
> > +
> > +static void destroy_file(struct file *f)
> > +{
> > + security_file_free(f);
> > + put_cred(f->f_cred);
>
> Note that calling destroy_file() in this way bypasses
> security_file_release(). Presumably this doesn't matter because no LSM
> does a security_alloc_file() for this but it adds a nother wrinkly into
> the cleanup path.
>
This is for Paul to comment on.
The way I see it, security_file_open() was not called on the user path file,
so no reason to call security_file_release()?
It is very much a possibility that LSM would want to allocate security
context for the user path file during backing_file_mmap, when both files
are available in context, so that later mprotect() will have this stored
information in the user path file security context.
But in this case, wouldn't security_file_free() be enough?
>
> > }
> > -EXPORT_SYMBOL_GPL(backing_file_set_user_path);
> >
> > static inline void file_free(struct file *f)
> > {
> > - security_file_free(f);
> > + destroy_file(f);
> > if (likely(!(f->f_mode & FMODE_NOACCOUNT)))
> > percpu_counter_dec(&nr_files);
> > - put_cred(f->f_cred);
> > if (unlikely(f->f_mode & FMODE_BACKING)) {
> > - path_put(backing_file_user_path(f));
> > + struct file *user_path_file = &backing_file(f)->user_path_file;
> > +
> > + /*
> > + * no refcount on the user_path_file - they die together,
> > + * so __fput() is not called for user_path_file. path_put()
> > + * is the only relevant cleanup from __fput().
> > + */
> > + destroy_file(user_path_file);
> > + path_put(&user_path_file->__f_path);
> > kmem_cache_free(bfilp_cachep, backing_file(f));
> > } else {
> > kmem_cache_free(filp_cachep, f);
> > @@ -201,7 +222,7 @@ static int init_file(struct file *f, int flags, const struct cred *cred)
> > * fget-rcu pattern users need to be able to handle spurious
> > * refcount bumps we should reinitialize the reused file first.
> > */
> > - file_ref_init(&f->f_ref, 1);
> > + atomic_long_set(&f->f_ref.refcnt, FILE_REF_ONEREF);
>
> No, please don't open-code this. The point is to stop any open-access to
> f_ref. And also below you introduce another atomic_long_set() open-coded
> call as well. Simply adapt file_ref_init() to not do the -1 subtraction
> and use the constants directly.
>
OK.
> > return 0;
> > }
> >
> > @@ -290,7 +311,8 @@ struct file *alloc_empty_file_noaccount(int flags, const struct cred *cred)
> > * This is only for kernel internal use, and the allocate file must not be
> > * installed into file tables or such.
> > */
> > -struct file *alloc_empty_backing_file(int flags, const struct cred *cred)
> > +struct file *alloc_empty_backing_file(int flags, const struct cred *cred,
> > + const struct cred *user_cred)
> > {
> > struct backing_file *ff;
> > int error;
> > @@ -305,6 +327,15 @@ struct file *alloc_empty_backing_file(int flags, const struct cred *cred)
> > return ERR_PTR(error);
> > }
> >
> > + error = init_file(&ff->user_path_file, O_PATH, user_cred);
> > + /* user_path_file is not refcounterd - it dies with the backing file */
> > + atomic_long_set(&ff->user_path_file.f_ref.refcnt, FILE_REF_DEAD);
>
> Please massage this and send that patch. I'll stuff it into a stable vfs
> branch that both Paul and I can merge. Paul can then send his PR.
Sure. I'll try to send it later today.
Thanks,
Amir.
More information about the Linux-erofs
mailing list