[PATCH 1/3] backing_file: store user_path_file
Christian Brauner
brauner at kernel.org
Wed Mar 18 21:56:57 AEDT 2026
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.
> +{
> + /* 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.
> }
> -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.
> 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.
More information about the Linux-erofs
mailing list