[PATCH v3 1/2] lsm: add backing_file LSM hooks
Amir Goldstein
amir73il at gmail.com
Sat Mar 28 19:29:03 AEDT 2026
On Fri, Mar 27, 2026 at 11:05 PM Paul Moore <paul at paul-moore.com> wrote:
>
> Stacked filesystems such as overlayfs do not currently provide the
> necessary mechanisms for LSMs to properly enforce access controls on the
> mmap() and mprotect() operations. In order to resolve this gap, a LSM
> security blob is being added to the backing_file struct and the following
> new LSM hooks are being created:
>
> security_backing_file_alloc()
> security_backing_file_free()
> security_mmap_backing_file()
>
> The first two hooks are to manage the lifecycle of the LSM security blob
> in the backing_file struct, while the third provides a new mmap() access
> control point for the underlying backing file. It is also expected that
> LSMs will likely want to update their security_file_mprotect() callback
> to address issues with their mprotect() controls, but that does not
> require a change to the security_file_mprotect() LSM hook.
>
> There are a two other small changes to support these new LSM hooks. We
> pass the user file associated with a backing file down to
> alloc_empty_backing_file() so it can be included in the
> security_backing_file_alloc() hook, and we constify the file struct field
> in the LSM common_audit_data struct to better support LSMs that need to
> pass a const file struct pointer into the common LSM audit code.
>
> Thanks to Arnd Bergmann for identifying the missing EXPORT_SYMBOL_GPL()
> and supplying a fixup.
>
> Cc: stable at vger.kernel.org
> Acked-by: Christian Brauner <brauner at kernel.org>
> Signed-off-by: Paul Moore <paul at paul-moore.com>
> ---
I 100% agree with Christian.
This is much better than my O_PATH file hack
It is also what Miklos had initially suggested.
I have a minor suggestion for API change though
> fs/backing-file.c | 18 ++++--
> fs/erofs/ishare.c | 10 +++-
> fs/file_table.c | 21 ++++++-
> fs/fuse/passthrough.c | 2 +-
> fs/internal.h | 3 +-
> fs/overlayfs/dir.c | 2 +-
> fs/overlayfs/file.c | 2 +-
> include/linux/backing-file.h | 4 +-
> include/linux/fs.h | 1 +
> include/linux/lsm_audit.h | 2 +-
> include/linux/lsm_hook_defs.h | 5 ++
> include/linux/lsm_hooks.h | 1 +
> include/linux/security.h | 22 ++++++++
> security/lsm.h | 1 +
> security/lsm_init.c | 9 +++
> security/security.c | 100 ++++++++++++++++++++++++++++++++++
> 16 files changed, 187 insertions(+), 16 deletions(-)
>
> diff --git a/fs/backing-file.c b/fs/backing-file.c
> index 45da8600d564..1f3bbfc75882 100644
> --- a/fs/backing-file.c
> +++ b/fs/backing-file.c
> @@ -12,6 +12,7 @@
> #include <linux/backing-file.h>
> #include <linux/splice.h>
> #include <linux/mm.h>
> +#include <linux/security.h>
>
> #include "internal.h"
>
> @@ -29,14 +30,15 @@
> * 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 file *user_file, int flags,
> const struct path *real_path,
> const struct cred *cred)
> {
> + const struct path *user_path = &user_file->f_path;
> struct file *f;
> int error;
>
> - f = alloc_empty_backing_file(flags, cred);
> + f = alloc_empty_backing_file(flags, cred, user_file);
> if (IS_ERR(f))
> return f;
>
> @@ -52,15 +54,16 @@ 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 file *user_file, int flags,
> const struct path *real_parentpath,
> umode_t mode, const struct cred *cred)
> {
> struct mnt_idmap *real_idmap = mnt_idmap(real_parentpath->mnt);
> + const struct path *user_path = &user_file->f_path;
> struct file *f;
> int error;
>
> - f = alloc_empty_backing_file(flags, cred);
> + f = alloc_empty_backing_file(flags, cred, user_file);
> if (IS_ERR(f))
> return f;
>
> @@ -336,8 +339,13 @@ int backing_file_mmap(struct file *file, struct vm_area_struct *vma,
>
> vma_set_file(vma, file);
>
> - scoped_with_creds(ctx->cred)
> + scoped_with_creds(ctx->cred) {
> + ret = security_mmap_backing_file(vma, file, user_file);
> + if (ret)
> + return ret;
> +
> ret = vfs_mmap(vma->vm_file, vma);
> + }
>
> if (ctx->accessed)
> ctx->accessed(user_file);
> diff --git a/fs/erofs/ishare.c b/fs/erofs/ishare.c
> index 829d50d5c717..ec3fc5ac1a55 100644
> --- a/fs/erofs/ishare.c
> +++ b/fs/erofs/ishare.c
> @@ -4,6 +4,7 @@
> */
> #include <linux/xxhash.h>
> #include <linux/mount.h>
> +#include <linux/security.h>
> #include "internal.h"
> #include "xattr.h"
>
> @@ -106,7 +107,8 @@ 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);
> if (IS_ERR(realfile))
> return PTR_ERR(realfile);
> ihold(sharedinode);
> @@ -150,8 +152,14 @@ static ssize_t erofs_ishare_file_read_iter(struct kiocb *iocb,
> static int erofs_ishare_mmap(struct file *file, struct vm_area_struct *vma)
> {
> struct file *realfile = file->private_data;
> + int err;
>
> vma_set_file(vma, realfile);
> +
> + err = security_mmap_backing_file(vma, realfile, file);
> + if (err)
> + return err;
> +
> return generic_file_readonly_mmap(file, vma);
> }
>
> diff --git a/fs/file_table.c b/fs/file_table.c
> index aaa5faaace1e..0bdc26cae138 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -50,6 +50,7 @@ struct backing_file {
> struct path user_path;
> freeptr_t bf_freeptr;
> };
Shouldn't we wrap this with
#ifdef CONFIG_SECURITY
> + void *security;
please initialize it in init_file()
> };
>
> #define backing_file(f) container_of(f, struct backing_file, file)
> @@ -66,6 +67,11 @@ void backing_file_set_user_path(struct file *f, const struct path *path)
> }
> EXPORT_SYMBOL_GPL(backing_file_set_user_path);
>
> +void *backing_file_security(const struct file *f)
> +{
> + return backing_file(f)->security;
I think LSM code should be completely responsible for this ptr
assignment/free so you should export
void **backing_file_security_ptr(const struct file *f)
{
return &backing_file(f)->security;
}
> +
> static inline void file_free(struct file *f)
> {
> security_file_free(f);
> @@ -73,8 +79,11 @@ static inline void file_free(struct file *f)
> percpu_counter_dec(&nr_files);
> put_cred(f->f_cred);
> if (unlikely(f->f_mode & FMODE_BACKING)) {
> - path_put(backing_file_user_path(f));
> - kmem_cache_free(bfilp_cachep, backing_file(f));
> + struct backing_file *ff = backing_file(f);
> +
> + security_backing_file_free(&ff->security);
Why do you need to add this in vfs code?
Can't you do the same in security_file_free(f)?
if (unlikely(f->f_mode & FMODE_BACKING))
security_backing_file_free(backing_file_security_ptr(f));
> + path_put(&ff->user_path);
> + kmem_cache_free(bfilp_cachep, ff);
> } else {
> kmem_cache_free(filp_cachep, f);
> }
> @@ -290,7 +299,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 file *user_file)
> {
> struct backing_file *ff;
> int error;
> @@ -306,6 +316,11 @@ struct file *alloc_empty_backing_file(int flags, const struct cred *cred)
> }
>
> ff->file.f_mode |= FMODE_BACKING | FMODE_NOACCOUNT;
> + error = security_backing_file_alloc(&ff->security, user_file);> + if (unlikely(error)) {
> + fput(&ff->file);
> + return ERR_PTR(error);
> + }
> return &ff->file;
> }
> EXPORT_SYMBOL_GPL(alloc_empty_backing_file);
Maybe, and I am not sure,
alloc_empty_backing_file() should call ONLY
error = security_backing_file_alloc(&ff->file, user_file);
Instead of security_file_alloc() AND security_backing_file_alloc()
and security_backing_file_alloc() can make use of
backing_file_security_ptr() accessor internally?
I think this will further abstract LSM implementation details from vfs
and avoid the need to spray #ifdef SECURITY in vfs code.
WDYT?
Thanks for following through with this elegant and clean API!
Amir.
More information about the Linux-erofs
mailing list