* [PATCH 0/3] Fix incorrect overlayfs mmap() and mprotect() LSM access controls
@ 2026-03-16 21:35 Paul Moore
2026-03-16 21:35 ` [PATCH 1/3] backing_file: store user_path_file Paul Moore
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Paul Moore @ 2026-03-16 21:35 UTC (permalink / raw)
To: linux-security-module, selinux, linux-fsdevel, linux-unionfs,
linux-erofs
Cc: Amir Goldstein, Gao Xiang
The existing mmap() and mprotect() LSM access control points for the
overlayfs filesystem are incomplete in that they do not cover both the
user and backing files. This patchset corrects this through the addition
of a new backing file specific LSM hook, security_mmap_backing_file(),
a new user path file associated with a backing file that can be used by
LSMs in the security_file_mprotect() code path, and the associated
SELinux code changes.
The security_mmap_backing_file() hook is intended to allow LSMs to apply
access controls on mmap() operations accessing a backing file, similar to
the security_mmap_file() for user files. Due to the details around the
accesses and the desire to distinguish between the two types of accesses,
a new LSM hook was needed. More information on this new hook can be
found in the associated patch.
The new user path file replaces the existing user path stored in the
backing file. This change was necessary to support LSM based access
controls in the mprotect() code path where only one file is accessible
via the vma->vm_file field. Unfortunately, storing a reference to the
user file inside the backing file does not work due to the cyclic
ref counting so a stand-in was necessary, the new user O_PATH file.
This new O_PATH file is intended to be representative of the original
user file and can be used by LSMs to make access control decisions based
on both the backing and user files.
The SELinux changes in this patchset involve making use of the new
security_mmap_backing_file() hook and updating the existing mprotect()
access controls to take into account both the backing and user files.
These changes preserve the existing SELinux approach of allowing access
on overlayfs files if the current task has the necessary rights to the
user file and the mounting process has the necessary rights to the
underlying backing file.
--
Amir Goldstein (1):
backing_file: store user_path_file
Paul Moore (2):
lsm: add the security_mmap_backing_file() hook
selinux: fix overlayfs mmap() and mprotect() access checks
fs/backing-file.c | 28 +++++---
fs/erofs/ishare.c | 12 ++-
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 ---
include/linux/lsm_audit.h | 2
include/linux/lsm_hook_defs.h | 2
include/linux/security.h | 10 +++
security/security.c | 25 +++++++
security/selinux/hooks.c | 108 ++++++++++++++++++++++++++++------
14 files changed, 231 insertions(+), 60 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] backing_file: store user_path_file
2026-03-16 21:35 [PATCH 0/3] Fix incorrect overlayfs mmap() and mprotect() LSM access controls Paul Moore
@ 2026-03-16 21:35 ` Paul Moore
2026-03-18 10:56 ` Christian Brauner
2026-03-16 21:35 ` [PATCH 2/3] lsm: add the security_mmap_backing_file() hook Paul Moore
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Paul Moore @ 2026-03-16 21:35 UTC (permalink / raw)
To: linux-security-module, selinux, linux-fsdevel, linux-unionfs,
linux-erofs
Cc: Amir Goldstein, Gao Xiang
From: Amir Goldstein <amir73il@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@gmail.com>
Tested-by: Paul Moore <paul@paul-moore.com> (SELinux)
Acked-by: Gao Xiang <hsiangkao@linux.alibaba.com> (EROFS)
Signed-off-by: Paul Moore <paul@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)
+{
+ /* 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);
}
-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);
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);
+ if (unlikely(error)) {
+ destroy_file(&ff->file);
+ kmem_cache_free(bfilp_cachep, ff);
+ return ERR_PTR(error);
+ }
+
ff->file.f_mode |= FMODE_BACKING | FMODE_NOACCOUNT;
return &ff->file;
}
diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
index 72de97c03d0e..60018c635934 100644
--- a/fs/fuse/passthrough.c
+++ b/fs/fuse/passthrough.c
@@ -10,6 +10,7 @@
#include <linux/file.h>
#include <linux/backing-file.h>
#include <linux/splice.h>
+#include <linux/uio.h>
static void fuse_file_accessed(struct file *file)
{
@@ -167,7 +168,7 @@ struct fuse_backing *fuse_passthrough_open(struct file *file, int backing_id)
goto out;
/* Allocate backing file per fuse file to store fuse path */
- backing_file = backing_file_open(&file->f_path, file->f_flags,
+ backing_file = backing_file_open(&file->f_path, file->f_cred, file->f_flags,
&fb->file->f_path, fb->cred);
err = PTR_ERR(backing_file);
if (IS_ERR(backing_file)) {
diff --git a/fs/internal.h b/fs/internal.h
index cbc384a1aa09..cc6f1e251f5a 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -106,8 +106,9 @@ extern void chroot_fs_refs(const struct path *, const struct path *);
*/
struct file *alloc_empty_file(int flags, const struct cred *cred);
struct file *alloc_empty_file_noaccount(int flags, const struct cred *cred);
-struct file *alloc_empty_backing_file(int flags, const struct cred *cred);
-void backing_file_set_user_path(struct file *f, const struct path *path);
+struct file *alloc_empty_backing_file(int flags, const struct cred *cred,
+ const struct cred *user_cred);
+void backing_file_open_user_path(struct file *f, const struct path *path);
static inline void file_put_write_access(struct file *file)
{
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index ff3dbd1ca61f..5914b5cf25e3 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -1374,7 +1374,8 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
return PTR_ERR(cred);
ovl_path_upper(dentry->d_parent, &realparentpath);
- realfile = backing_tmpfile_open(&file->f_path, flags, &realparentpath,
+ realfile = backing_tmpfile_open(&file->f_path, file->f_cred,
+ flags, &realparentpath,
mode, current_cred());
err = PTR_ERR_OR_ZERO(realfile);
pr_debug("tmpfile/open(%pd2, 0%o) = %i\n", realparentpath.dentry, mode, err);
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 97bed2286030..767c128407fc 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -49,6 +49,7 @@ static struct file *ovl_open_realfile(const struct file *file,
flags &= ~O_NOATIME;
realfile = backing_file_open(file_user_path(file),
+ file_user_cred(file),
flags, realpath, current_cred());
}
}
diff --git a/include/linux/backing-file.h b/include/linux/backing-file.h
index 1476a6ed1bfd..8afba93f3ce0 100644
--- a/include/linux/backing-file.h
+++ b/include/linux/backing-file.h
@@ -9,19 +9,42 @@
#define _LINUX_BACKING_FILE_H
#include <linux/file.h>
-#include <linux/uio.h>
#include <linux/fs.h>
+/*
+ * When mmapping a file on a stackable filesystem (e.g., overlayfs), the file
+ * stored in ->vm_file is a backing file whose f_inode is on the underlying
+ * filesystem.
+ *
+ * LSM can use file_user_path_file() to store context related to the user path
+ * that was opened and mmaped.
+ */
+const struct file *backing_file_user_path_file(const struct file *f);
+
+static inline const struct file *file_user_path_file(const struct file *f)
+{
+ if (f && unlikely(f->f_mode & FMODE_BACKING))
+ return backing_file_user_path_file(f);
+ return f;
+}
+
+static inline const struct cred *file_user_cred(const struct file *f)
+{
+ return file_user_path_file(f)->f_cred;
+}
+
struct backing_file_ctx {
const struct cred *cred;
void (*accessed)(struct file *file);
void (*end_write)(struct kiocb *iocb, ssize_t);
};
-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 *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);
ssize_t backing_file_read_iter(struct file *file, struct iov_iter *iter,
diff --git a/include/linux/file_ref.h b/include/linux/file_ref.h
index 31551e4cb8f3..fdaacbcbdb5b 100644
--- a/include/linux/file_ref.h
+++ b/include/linux/file_ref.h
@@ -51,16 +51,6 @@ typedef struct {
#endif
} file_ref_t;
-/**
- * file_ref_init - Initialize a file reference count
- * @ref: Pointer to the reference count
- * @cnt: The initial reference count typically '1'
- */
-static inline void file_ref_init(file_ref_t *ref, unsigned long cnt)
-{
- atomic_long_set(&ref->refcnt, cnt - 1);
-}
-
bool __file_ref_put(file_ref_t *ref, unsigned long cnt);
/**
--
2.53.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] lsm: add the security_mmap_backing_file() hook
2026-03-16 21:35 [PATCH 0/3] Fix incorrect overlayfs mmap() and mprotect() LSM access controls Paul Moore
2026-03-16 21:35 ` [PATCH 1/3] backing_file: store user_path_file Paul Moore
@ 2026-03-16 21:35 ` Paul Moore
2026-03-17 22:42 ` Paul Moore
2026-03-16 21:35 ` [PATCH 3/3] selinux: fix overlayfs mmap() and mprotect() access checks Paul Moore
2026-03-16 21:59 ` [PATCH 0/3] Fix incorrect overlayfs mmap() and mprotect() LSM access controls Paul Moore
3 siblings, 1 reply; 11+ messages in thread
From: Paul Moore @ 2026-03-16 21:35 UTC (permalink / raw)
To: linux-security-module, selinux, linux-fsdevel, linux-unionfs,
linux-erofs
Cc: Amir Goldstein, Gao Xiang
Add the security_mmap_backing_file() hook to allow LSMs to properly
enforce access controls on mmap() operations on stacked filesystems
such as overlayfs.
The existing security_mmap_file() hook exists as an access control point
for mmap() but on stacked filesystems it only provides a way to enforce
access controls on the user visible file. In order to enforce access
controls on the underlying backing file, the new
security_mmap_backing_file() hook is needed.
In addition the LSM hook additions, this patch also constifies the file
struct field in the LSM common_audit_data struct to better support LSMs
that will likely need to pass a const file struct pointer from the new
backing_file_user_path_file() API into the common LSM audit code.
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
fs/backing-file.c | 8 +++++++-
fs/erofs/ishare.c | 6 ++++++
include/linux/lsm_audit.h | 2 +-
include/linux/lsm_hook_defs.h | 2 ++
include/linux/security.h | 10 ++++++++++
security/security.c | 25 +++++++++++++++++++++++++
6 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/fs/backing-file.c b/fs/backing-file.c
index acabeea7efff..cfc7f6611313 100644
--- a/fs/backing-file.c
+++ b/fs/backing-file.c
@@ -13,6 +13,7 @@
#include <linux/splice.h>
#include <linux/uio.h>
#include <linux/mm.h>
+#include <linux/security.h>
#include "internal.h"
@@ -338,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 17a4941d4518..d66c3a935d83 100644
--- a/fs/erofs/ishare.c
+++ b/fs/erofs/ishare.c
@@ -150,8 +150,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/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
index 382c56a97bba..584db296e43b 100644
--- a/include/linux/lsm_audit.h
+++ b/include/linux/lsm_audit.h
@@ -94,7 +94,7 @@ struct common_audit_data {
#endif
char *kmod_name;
struct lsm_ioctlop_audit *op;
- struct file *file;
+ const struct file *file;
struct lsm_ibpkey_audit *ibpkey;
struct lsm_ibendport_audit *ibendport;
int reason;
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 8c42b4bde09c..4150c50a0482 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -198,6 +198,8 @@ LSM_HOOK(int, 0, file_ioctl_compat, struct file *file, unsigned int cmd,
LSM_HOOK(int, 0, mmap_addr, unsigned long addr)
LSM_HOOK(int, 0, mmap_file, struct file *file, unsigned long reqprot,
unsigned long prot, unsigned long flags)
+LSM_HOOK(int, 0, mmap_backing_file, struct vm_area_struct *vma,
+ struct file *backing_file, struct file *user_file)
LSM_HOOK(int, 0, file_mprotect, struct vm_area_struct *vma,
unsigned long reqprot, unsigned long prot)
LSM_HOOK(int, 0, file_lock, struct file *file, unsigned int cmd)
diff --git a/include/linux/security.h b/include/linux/security.h
index 83a646d72f6f..4017361d8cba 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -476,6 +476,9 @@ int security_file_ioctl_compat(struct file *file, unsigned int cmd,
unsigned long arg);
int security_mmap_file(struct file *file, unsigned long prot,
unsigned long flags);
+int security_mmap_backing_file(struct vm_area_struct *vma,
+ struct file *backing_file,
+ struct file *user_file);
int security_mmap_addr(unsigned long addr);
int security_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
unsigned long prot);
@@ -1159,6 +1162,13 @@ static inline int security_mmap_file(struct file *file, unsigned long prot,
return 0;
}
+static inline int security_mmap_backing_file(struct vm_area_struct *vma,
+ struct file *backing_file,
+ struct file *user_file)
+{
+ return 0;
+}
+
static inline int security_mmap_addr(unsigned long addr)
{
return cap_mmap_addr(addr);
diff --git a/security/security.c b/security/security.c
index 67af9228c4e9..8d10b184ce25 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2505,6 +2505,31 @@ int security_mmap_file(struct file *file, unsigned long prot,
flags);
}
+/**
+ * security_mmap_backing_file - Check if mmap'ing a backing file is allowed
+ * @vma: the vm_area_struct for the mmap'd region
+ * @backing_file: the backing file being mmap'd
+ * @user_file: the user file being mmap'd
+ *
+ * Check permissions for a mmap operation on a stacked filesystem. This hook
+ * is called after the security_mmap_file() and is responsible for authorizing
+ * the mmap on @backing_file. It is important to note that the mmap operation
+ * on @user_file has already been authorized and the @vma->vm_file has been
+ * set to @backing_file.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_mmap_backing_file(struct vm_area_struct *vma,
+ struct file *backing_file,
+ struct file *user_file)
+{
+ /* recommended by the stackable filesystem devs */
+ if (WARN_ON_ONCE(!(backing_file->f_mode & FMODE_BACKING)))
+ return -EIO;
+
+ return call_int_hook(mmap_backing_file, vma, backing_file, user_file);
+}
+
/**
* security_mmap_addr() - Check if mmap'ing an address is allowed
* @addr: address
--
2.53.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] selinux: fix overlayfs mmap() and mprotect() access checks
2026-03-16 21:35 [PATCH 0/3] Fix incorrect overlayfs mmap() and mprotect() LSM access controls Paul Moore
2026-03-16 21:35 ` [PATCH 1/3] backing_file: store user_path_file Paul Moore
2026-03-16 21:35 ` [PATCH 2/3] lsm: add the security_mmap_backing_file() hook Paul Moore
@ 2026-03-16 21:35 ` Paul Moore
2026-03-16 21:59 ` [PATCH 0/3] Fix incorrect overlayfs mmap() and mprotect() LSM access controls Paul Moore
3 siblings, 0 replies; 11+ messages in thread
From: Paul Moore @ 2026-03-16 21:35 UTC (permalink / raw)
To: linux-security-module, selinux, linux-fsdevel, linux-unionfs,
linux-erofs
Cc: Amir Goldstein, Gao Xiang
The existing SELinux security model for overlayfs is to allow access if
the current task is able to access the top level file (the "user" file)
and the mounter's credentials are sufficient to access the lower
level file (the "backing" file). Unfortunately, the current code does
not properly enforce these access controls for both mmap() and mprotect()
operations on overlayfs filesystems.
This patch makes use of the newly created security_mmap_backing_file()
LSM hook to provide the missing backing file enforcement for mmap()
operations on overlayfs files, and leverages the new
backing_file_user_path_file() VFS API to provide an equivalent to the
missing user file in mprotect().
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
security/selinux/hooks.c | 108 ++++++++++++++++++++++++++++++++-------
1 file changed, 90 insertions(+), 18 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index d8224ea113d1..013e1e35a1ff 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -94,6 +94,7 @@
#include <linux/io_uring/cmd.h>
#include <uapi/linux/lsm.h>
#include <linux/memfd.h>
+#include <linux/backing-file.h>
#include "initcalls.h"
#include "avc.h"
@@ -1754,7 +1755,7 @@ static int bpf_fd_pass(const struct file *file, u32 sid);
access to the file is not checked, e.g. for cases
where only the descriptor is affected like seek. */
static int file_has_perm(const struct cred *cred,
- struct file *file,
+ const struct file *file,
u32 av)
{
struct file_security_struct *fsec = selinux_file(file);
@@ -3942,9 +3943,9 @@ static int selinux_file_ioctl_compat(struct file *file, unsigned int cmd,
static int default_noexec __ro_after_init;
-static int file_map_prot_check(struct file *file, unsigned long prot, int shared)
+static int file_map_prot_check(const struct cred *cred, const struct file *file,
+ unsigned long prot, bool shared)
{
- const struct cred *cred = current_cred();
u32 sid = cred_sid(cred);
int rc = 0;
@@ -3993,36 +3994,86 @@ static int selinux_mmap_addr(unsigned long addr)
return rc;
}
-static int selinux_mmap_file(struct file *file,
- unsigned long reqprot __always_unused,
- unsigned long prot, unsigned long flags)
+static int selinux_mmap_file_common(const struct cred *cred, struct file *file,
+ unsigned long prot, bool shared)
{
- struct common_audit_data ad;
- int rc;
-
if (file) {
+ int rc;
+ struct common_audit_data ad;
+
ad.type = LSM_AUDIT_DATA_FILE;
ad.u.file = file;
- rc = inode_has_perm(current_cred(), file_inode(file),
- FILE__MAP, &ad);
+ rc = inode_has_perm(cred, file_inode(file), FILE__MAP, &ad);
if (rc)
return rc;
}
- return file_map_prot_check(file, prot,
- (flags & MAP_TYPE) == MAP_SHARED);
+ return file_map_prot_check(cred, file, prot, shared);
+}
+
+static int selinux_mmap_file(struct file *file,
+ unsigned long reqprot __always_unused,
+ unsigned long prot, unsigned long flags)
+{
+ return selinux_mmap_file_common(current_cred(), file, prot,
+ (flags & MAP_TYPE) == MAP_SHARED);
+}
+
+/**
+ * selinux_mmap_backing_file - Check mmap permissions on a backing file
+ * @vma: memory region
+ * @backing_file: stacked filesystem backing file
+ * @user_file: user visible file
+ *
+ * This is called after selinux_mmap_file() on stacked filesystems, and it
+ * is this function's responsibility to verify access to @backing_file and
+ * setup the SELinux state for possible later use in the mprotect() code path.
+ *
+ * By the time this function is called, mmap() access to @user_file has already
+ * been authorized and @vma->vm_file has been set to point to @backing_file.
+ *
+ * Return zero on success, negative values otherwise.
+ */
+static int selinux_mmap_backing_file(struct vm_area_struct *vma,
+ struct file *backing_file,
+ struct file *user_file __always_unused)
+{
+ unsigned long prot = 0;
+
+ /* translate vma->vm_flags perms into PROT perms */
+ if (vma->vm_flags & VM_READ)
+ prot |= PROT_READ;
+ if (vma->vm_flags & VM_WRITE)
+ prot |= PROT_WRITE;
+ if (vma->vm_flags & VM_EXEC)
+ prot |= PROT_EXEC;
+
+ return selinux_mmap_file_common(backing_file->f_cred, backing_file,
+ prot, vma->vm_flags & VM_SHARED);
}
static int selinux_file_mprotect(struct vm_area_struct *vma,
unsigned long reqprot __always_unused,
unsigned long prot)
{
+ int rc;
const struct cred *cred = current_cred();
u32 sid = cred_sid(cred);
+ const struct file *file = vma->vm_file;
+ const struct file *backing_file = NULL;
+
+ /* check if adjustments are needed for stacked filesystems */
+ if (file && (file->f_mode & FMODE_BACKING)) {
+ backing_file = vma->vm_file;
+ file = backing_file_user_path_file(backing_file);
+
+ /* sanity check the special O_PATH user file */
+ if (WARN_ON(!(file->f_mode & FMODE_OPENED)))
+ return -EPERM;
+ }
if (default_noexec &&
(prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC)) {
- int rc = 0;
/*
* We don't use the vma_is_initial_heap() helper as it has
* a history of problems and is currently broken on systems
@@ -4036,11 +4087,15 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
vma->vm_end <= vma->vm_mm->brk) {
rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
PROCESS__EXECHEAP, NULL);
- } else if (!vma->vm_file && (vma_is_initial_stack(vma) ||
+ if (rc)
+ return rc;
+ } else if (!file && (vma_is_initial_stack(vma) ||
vma_is_stack_for_current(vma))) {
rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
PROCESS__EXECSTACK, NULL);
- } else if (vma->vm_file && vma->anon_vma) {
+ if (rc)
+ return rc;
+ } else if (file && vma->anon_vma) {
/*
* We are making executable a file mapping that has
* had some COW done. Since pages might have been
@@ -4048,13 +4103,29 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
* modified content. This typically should only
* occur for text relocations.
*/
- rc = file_has_perm(cred, vma->vm_file, FILE__EXECMOD);
+ rc = file_has_perm(cred, file, FILE__EXECMOD);
+ if (rc)
+ return rc;
+ if (backing_file) {
+ rc = file_has_perm(backing_file->f_cred,
+ backing_file, FILE__EXECMOD);
+ if (rc)
+ return rc;
+ }
}
+ }
+
+ rc = file_map_prot_check(cred, file, prot, vma->vm_flags & VM_SHARED);
+ if (rc)
+ return rc;
+ if (backing_file) {
+ rc = file_map_prot_check(backing_file->f_cred, backing_file,
+ prot, vma->vm_flags & VM_SHARED);
if (rc)
return rc;
}
- return file_map_prot_check(vma->vm_file, prot, vma->vm_flags&VM_SHARED);
+ return 0;
}
static int selinux_file_lock(struct file *file, unsigned int cmd)
@@ -7501,6 +7572,7 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
LSM_HOOK_INIT(file_ioctl, selinux_file_ioctl),
LSM_HOOK_INIT(file_ioctl_compat, selinux_file_ioctl_compat),
LSM_HOOK_INIT(mmap_file, selinux_mmap_file),
+ LSM_HOOK_INIT(mmap_backing_file, selinux_mmap_backing_file),
LSM_HOOK_INIT(mmap_addr, selinux_mmap_addr),
LSM_HOOK_INIT(file_mprotect, selinux_file_mprotect),
LSM_HOOK_INIT(file_lock, selinux_file_lock),
--
2.53.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] Fix incorrect overlayfs mmap() and mprotect() LSM access controls
2026-03-16 21:35 [PATCH 0/3] Fix incorrect overlayfs mmap() and mprotect() LSM access controls Paul Moore
` (2 preceding siblings ...)
2026-03-16 21:35 ` [PATCH 3/3] selinux: fix overlayfs mmap() and mprotect() access checks Paul Moore
@ 2026-03-16 21:59 ` Paul Moore
2026-03-17 7:25 ` Amir Goldstein
3 siblings, 1 reply; 11+ messages in thread
From: Paul Moore @ 2026-03-16 21:59 UTC (permalink / raw)
To: linux-security-module, selinux, linux-fsdevel, linux-unionfs,
linux-erofs
Cc: Amir Goldstein, Gao Xiang
On Mon, Mar 16, 2026 at 5:36 PM Paul Moore <paul@paul-moore.com> wrote:
>
> The existing mmap() and mprotect() LSM access control points for the
> overlayfs filesystem are incomplete in that they do not cover both the
> user and backing files. This patchset corrects this through the addition
> of a new backing file specific LSM hook, security_mmap_backing_file(),
> a new user path file associated with a backing file that can be used by
> LSMs in the security_file_mprotect() code path, and the associated
> SELinux code changes.
>
> The security_mmap_backing_file() hook is intended to allow LSMs to apply
> access controls on mmap() operations accessing a backing file, similar to
> the security_mmap_file() for user files. Due to the details around the
> accesses and the desire to distinguish between the two types of accesses,
> a new LSM hook was needed. More information on this new hook can be
> found in the associated patch.
>
> The new user path file replaces the existing user path stored in the
> backing file. This change was necessary to support LSM based access
> controls in the mprotect() code path where only one file is accessible
> via the vma->vm_file field. Unfortunately, storing a reference to the
> user file inside the backing file does not work due to the cyclic
> ref counting so a stand-in was necessary, the new user O_PATH file.
> This new O_PATH file is intended to be representative of the original
> user file and can be used by LSMs to make access control decisions based
> on both the backing and user files.
>
> The SELinux changes in this patchset involve making use of the new
> security_mmap_backing_file() hook and updating the existing mprotect()
> access controls to take into account both the backing and user files.
> These changes preserve the existing SELinux approach of allowing access
> on overlayfs files if the current task has the necessary rights to the
> user file and the mounting process has the necessary rights to the
> underlying backing file.
>
> --
> Amir Goldstein (1):
> backing_file: store user_path_file
>
> Paul Moore (2):
> lsm: add the security_mmap_backing_file() hook
> selinux: fix overlayfs mmap() and mprotect() access checks
>
> fs/backing-file.c | 28 +++++---
> fs/erofs/ishare.c | 12 ++-
> 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 ---
> include/linux/lsm_audit.h | 2
> include/linux/lsm_hook_defs.h | 2
> include/linux/security.h | 10 +++
> security/security.c | 25 +++++++
> security/selinux/hooks.c | 108 ++++++++++++++++++++++++++++------
> 14 files changed, 231 insertions(+), 60 deletions(-)
Due to the nature of the issue, I'm going to merge this into
lsm/stable-7.0 in a few moments so the changes can get some testing in
linux-next with the idea of sending this up to Linus' later in the
week. If anyone has any concerns over this patchset, please let me
know as soon as possible.
--
paul-moore.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] Fix incorrect overlayfs mmap() and mprotect() LSM access controls
2026-03-16 21:59 ` [PATCH 0/3] Fix incorrect overlayfs mmap() and mprotect() LSM access controls Paul Moore
@ 2026-03-17 7:25 ` Amir Goldstein
2026-03-17 18:16 ` Paul Moore
0 siblings, 1 reply; 11+ messages in thread
From: Amir Goldstein @ 2026-03-17 7:25 UTC (permalink / raw)
To: Paul Moore
Cc: linux-security-module, selinux, linux-fsdevel, linux-unionfs,
linux-erofs, Gao Xiang
On Mon, Mar 16, 2026 at 10:59 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Mon, Mar 16, 2026 at 5:36 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > The existing mmap() and mprotect() LSM access control points for the
> > overlayfs filesystem are incomplete in that they do not cover both the
> > user and backing files. This patchset corrects this through the addition
> > of a new backing file specific LSM hook, security_mmap_backing_file(),
> > a new user path file associated with a backing file that can be used by
> > LSMs in the security_file_mprotect() code path, and the associated
> > SELinux code changes.
> >
> > The security_mmap_backing_file() hook is intended to allow LSMs to apply
> > access controls on mmap() operations accessing a backing file, similar to
> > the security_mmap_file() for user files. Due to the details around the
> > accesses and the desire to distinguish between the two types of accesses,
> > a new LSM hook was needed. More information on this new hook can be
> > found in the associated patch.
> >
> > The new user path file replaces the existing user path stored in the
> > backing file. This change was necessary to support LSM based access
> > controls in the mprotect() code path where only one file is accessible
> > via the vma->vm_file field. Unfortunately, storing a reference to the
> > user file inside the backing file does not work due to the cyclic
> > ref counting so a stand-in was necessary, the new user O_PATH file.
> > This new O_PATH file is intended to be representative of the original
> > user file and can be used by LSMs to make access control decisions based
> > on both the backing and user files.
> >
> > The SELinux changes in this patchset involve making use of the new
> > security_mmap_backing_file() hook and updating the existing mprotect()
> > access controls to take into account both the backing and user files.
> > These changes preserve the existing SELinux approach of allowing access
> > on overlayfs files if the current task has the necessary rights to the
> > user file and the mounting process has the necessary rights to the
> > underlying backing file.
> >
> > --
> > Amir Goldstein (1):
> > backing_file: store user_path_file
> >
> > Paul Moore (2):
> > lsm: add the security_mmap_backing_file() hook
> > selinux: fix overlayfs mmap() and mprotect() access checks
> >
> > fs/backing-file.c | 28 +++++---
> > fs/erofs/ishare.c | 12 ++-
> > 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 ---
> > include/linux/lsm_audit.h | 2
> > include/linux/lsm_hook_defs.h | 2
> > include/linux/security.h | 10 +++
> > security/security.c | 25 +++++++
> > security/selinux/hooks.c | 108 ++++++++++++++++++++++++++++------
> > 14 files changed, 231 insertions(+), 60 deletions(-)
>
> Due to the nature of the issue, I'm going to merge this into
> lsm/stable-7.0 in a few moments so the changes can get some testing in
> linux-next with the idea of sending this up to Linus' later in the
> week. If anyone has any concerns over this patchset, please let me
> know as soon as possible.
>
Since previous 4 revisions were not posted to public list,
let me repeat my concern from v4:
On Fri, Mar 6, 2026 at 5:24 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Fri, Mar 6, 2026 at 3:24 AM Amir Goldstein <amir73il@gmail.com> wrote:
...
> > My expectation is that the merge of this patch will be collaborated
> > with Christian ...
>
> Of course, that is one reason he is on the To/CC line. More on this
> in my reply to your 0/4 comments.
>
I am sorry Paul. This must be a misunderstanding.
My expectation for collaborating the merge of my patch with
Christian was that an agreement would be reached regarding
which way it would be routed to Linus.
CC to Christian and sending the patch to Linus without getting any
ACK from Christian was not the way I expected this to go.
> > and that it will NOT be auto selected or rushed into stable.
>
> I haven't marked it with a 'Fixes:' tag or a stable Cc which in my
> experience are the two quickest ways to get pulled into a stable tree.
> I'm not sure what stable policy Al or Christian have for the VFS tree,
> but LSM and SELinux commits are not pulled into the stable trees
> unless explicitly marked with a stable Cc or requested by a dev after
> the fact.
>
> > I don't mind if you want to route the security_mmap_backing_file() hooks to
> > stable to use it for some simpler bandaid for stable, but rushing this
> > one to stable is not a good idea IMO.
>
> Once again, see my (upcoming) reply to your 0/4 comments.
>
TBH, I don't understand the logic of placing patches in lsm/stable-7.0
without the intent of backporting them to stable.
I perceive my patch as a risky patch for overlayfs and the vfs
this is why I wanted Christian do be part of the decision if and when
my patch is sent to Linus.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] Fix incorrect overlayfs mmap() and mprotect() LSM access controls
2026-03-17 7:25 ` Amir Goldstein
@ 2026-03-17 18:16 ` Paul Moore
0 siblings, 0 replies; 11+ messages in thread
From: Paul Moore @ 2026-03-17 18:16 UTC (permalink / raw)
To: Amir Goldstein
Cc: linux-security-module, selinux, linux-fsdevel, linux-unionfs,
linux-erofs, Gao Xiang
On Tue, Mar 17, 2026 at 3:26 AM Amir Goldstein <amir73il@gmail.com> wrote:
> On Mon, Mar 16, 2026 at 10:59 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Mon, Mar 16, 2026 at 5:36 PM Paul Moore <paul@paul-moore.com> wrote:
...
> > Due to the nature of the issue, I'm going to merge this into
> > lsm/stable-7.0 in a few moments so the changes can get some testing in
> > linux-next with the idea of sending this up to Linus' later in the
> > week. If anyone has any concerns over this patchset, please let me
> > know as soon as possible.
>
> Since previous 4 revisions were not posted to public list,
> let me repeat my concern from v4:
>
> On Fri, Mar 6, 2026 at 5:24 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Fri, Mar 6, 2026 at 3:24 AM Amir Goldstein <amir73il@gmail.com> wrote:
> ...
> > > My expectation is that the merge of this patch will be collaborated
> > > with Christian ...
> >
> > Of course, that is one reason he is on the To/CC line. More on this
> > in my reply to your 0/4 comments.
> >
>
> I am sorry Paul. This must be a misunderstanding.
>
> My expectation for collaborating the merge of my patch with
> Christian was that an agreement would be reached regarding
> which way it would be routed to Linus.
>
> CC to Christian and sending the patch to Linus without getting any
> ACK from Christian was not the way I expected this to go.
To be honest I expected Christian to provide an ACK by now, as he
reviewed and commented on your earlier patches with the O_PATH file
approach. He has had weeks, if not months, to comment further and/or
supply an ACK so at this point I'm proceeding ahead as I mentioned to
you (and Christian last week). You were very anxious to bring these
patches on-list, which I did, however, now that everything is on-list
there is a responsibility to act on this quickly (this was the
motivation for discussing the solution privately at first).
Hopefully Christian will comment, and preferably ACK your patch, but
even if he does not we still need to go ahead and fix this soon in
Linus' tree.
> > > and that it will NOT be auto selected or rushed into stable.
> >
> > I haven't marked it with a 'Fixes:' tag or a stable Cc which in my
> > experience are the two quickest ways to get pulled into a stable tree.
> > I'm not sure what stable policy Al or Christian have for the VFS tree,
> > but LSM and SELinux commits are not pulled into the stable trees
> > unless explicitly marked with a stable Cc or requested by a dev after
> > the fact.
> >
> > > I don't mind if you want to route the security_mmap_backing_file() hooks to
> > > stable to use it for some simpler bandaid for stable, but rushing this
> > > one to stable is not a good idea IMO.
> >
> > Once again, see my (upcoming) reply to your 0/4 comments.
>
> TBH, I don't understand the logic of placing patches in lsm/stable-7.0
> without the intent of backporting them to stable.
I'm not going to copy-n-paste my previous off-list reply, as I have a
rather strict policy about forwarding private or off-list emails to a
public list without consent. However, the quick answer is that
inclusion in a lsm/stable-X.Y branch does not mean a patch(set) is
automatically tagged for stable, in fact you'll notice none of the
patches have a stable marking, mostly due to your previous request.
As the LSM and SELinux trees are not pulled into the stables trees
unless explicitly marked, or requested, I do not expect those patches
to be automatically backported. I do not know the stable backport
policy for VFS patches.
> I perceive my patch as a risky patch for overlayfs and the vfs
> this is why I wanted Christian do be part of the decision if and when
> my patch is sent to Linus.
Christian has been a part of the discussion for months now, and has
already provided feedback on the VFS portions of this patchset (which
you have incorporated). I agree, I would appreciate it if Christian
could supply his ACK, or an alternate solution, but as you strongly
encouraged us to bring this issue on-list, we now need to get a fix in
Linus' tree soon.
--
paul-moore.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] lsm: add the security_mmap_backing_file() hook
2026-03-16 21:35 ` [PATCH 2/3] lsm: add the security_mmap_backing_file() hook Paul Moore
@ 2026-03-17 22:42 ` Paul Moore
0 siblings, 0 replies; 11+ messages in thread
From: Paul Moore @ 2026-03-17 22:42 UTC (permalink / raw)
To: linux-security-module, selinux, linux-fsdevel, linux-unionfs,
linux-erofs
Cc: Amir Goldstein, Gao Xiang
On Mon, Mar 16, 2026 at 5:36 PM Paul Moore <paul@paul-moore.com> wrote:
>
> Add the security_mmap_backing_file() hook to allow LSMs to properly
> enforce access controls on mmap() operations on stacked filesystems
> such as overlayfs.
>
> The existing security_mmap_file() hook exists as an access control point
> for mmap() but on stacked filesystems it only provides a way to enforce
> access controls on the user visible file. In order to enforce access
> controls on the underlying backing file, the new
> security_mmap_backing_file() hook is needed.
>
> In addition the LSM hook additions, this patch also constifies the file
> struct field in the LSM common_audit_data struct to better support LSMs
> that will likely need to pass a const file struct pointer from the new
> backing_file_user_path_file() API into the common LSM audit code.
>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
> fs/backing-file.c | 8 +++++++-
> fs/erofs/ishare.c | 6 ++++++
> include/linux/lsm_audit.h | 2 +-
> include/linux/lsm_hook_defs.h | 2 ++
> include/linux/security.h | 10 ++++++++++
> security/security.c | 25 +++++++++++++++++++++++++
> 6 files changed, 51 insertions(+), 2 deletions(-)
...
> diff --git a/fs/erofs/ishare.c b/fs/erofs/ishare.c
> index 17a4941d4518..d66c3a935d83 100644
> --- a/fs/erofs/ishare.c
> +++ b/fs/erofs/ishare.c
> @@ -150,8 +150,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);
> }
The kernel test robot helpfully pointed out that this patch was
missing a security.h include for the newly added LSM hook. The fixup
below has been applied to the patch in lsm/stable-7.0.
diff --git a/fs/erofs/ishare.c b/fs/erofs/ishare.c
index d66c3a935d83..f80925b66599 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"
--
paul-moore.com
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] backing_file: store user_path_file
2026-03-16 21:35 ` [PATCH 1/3] backing_file: store user_path_file Paul Moore
@ 2026-03-18 10:56 ` Christian Brauner
2026-03-18 12:06 ` Amir Goldstein
0 siblings, 1 reply; 11+ messages in thread
From: Christian Brauner @ 2026-03-18 10:56 UTC (permalink / raw)
To: Paul Moore, Amir Goldstein
Cc: linux-security-module, selinux, linux-fsdevel, linux-unionfs,
linux-erofs, Gao Xiang
On Mon, Mar 16, 2026 at 05:35:56PM -0400, Paul Moore wrote:
> From: Amir Goldstein <amir73il@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@gmail.com>
> Tested-by: Paul Moore <paul@paul-moore.com> (SELinux)
> Acked-by: Gao Xiang <hsiangkao@linux.alibaba.com> (EROFS)
> Signed-off-by: Paul Moore <paul@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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] backing_file: store user_path_file
2026-03-18 10:56 ` Christian Brauner
@ 2026-03-18 12:06 ` Amir Goldstein
2026-03-18 20:00 ` Paul Moore
0 siblings, 1 reply; 11+ messages in thread
From: Amir Goldstein @ 2026-03-18 12:06 UTC (permalink / raw)
To: Christian Brauner
Cc: Paul Moore, linux-security-module, selinux, linux-fsdevel,
linux-unionfs, linux-erofs, Gao Xiang
On Wed, Mar 18, 2026 at 11:57 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Mon, Mar 16, 2026 at 05:35:56PM -0400, Paul Moore wrote:
> > From: Amir Goldstein <amir73il@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@gmail.com>
> > Tested-by: Paul Moore <paul@paul-moore.com> (SELinux)
> > Acked-by: Gao Xiang <hsiangkao@linux.alibaba.com> (EROFS)
> > Signed-off-by: Paul Moore <paul@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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] backing_file: store user_path_file
2026-03-18 12:06 ` Amir Goldstein
@ 2026-03-18 20:00 ` Paul Moore
0 siblings, 0 replies; 11+ messages in thread
From: Paul Moore @ 2026-03-18 20:00 UTC (permalink / raw)
To: Amir Goldstein
Cc: Christian Brauner, linux-security-module, selinux, linux-fsdevel,
linux-unionfs, linux-erofs, Gao Xiang
On Wed, Mar 18, 2026 at 8:07 AM Amir Goldstein <amir73il@gmail.com> wrote:
> On Wed, Mar 18, 2026 at 11:57 AM Christian Brauner <brauner@kernel.org> wrote:
> > On Mon, Mar 16, 2026 at 05:35:56PM -0400, Paul Moore wrote:
> > > From: Amir Goldstein <amir73il@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@gmail.com>
> > > Tested-by: Paul Moore <paul@paul-moore.com> (SELinux)
> > > Acked-by: Gao Xiang <hsiangkao@linux.alibaba.com> (EROFS)
> > > Signed-off-by: Paul Moore <paul@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/file_table.c b/fs/file_table.c
> > > index aaa5faaace1e..b7dc94656c44 100644
> > > --- a/fs/file_table.c
> > > +++ b/fs/file_table.c
> > > @@ -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.
That's good, I can remove the FMODE_OPENED sanity check in
selinux_file_mprotect() now.
> > > +{
> > > + /* 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.
The alloc_empty_backing_file() function will call init_file() on the
O_PATH file which in turnwill call security_file_alloc(). We depend
on that behavior as we need to set the creds properly on the file.
The tricky bit is that security_file_release() is currently only used
by IMA/EVM; it was created when we folded IMA/EVM into the LSM
framework, making them proper LSMs and cleaning up a lot of the hook
calls in the VFS. The commit description provides some info on the
hook:
IMA calculates at file close the new digest of the file content
and writes it to security.ima, so that appraisal at next file
access succeeds.
The new hook cannot return an error and cannot cause the operation
to be reverted.
As O_PATH files never go through the
security_file_open()/security_file_post_open() path, O_PATH files
should be ignored by IMA/EVM (which makes sense, as there is no data
or xattrs to measure, update, etc.). For that reason, skipping
security_file_release() should be okay in this particular case.
Other LSMs which allocate file specific state, e.g. AppArmor, in
security_file_alloc() should free that state in security_file_free()
which is handled in Amir's patch.
> 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?
See my comments above. If we wanted to look into removing the
destroy_file() special case for the user_path file hanging off the
backing file I'm happy to work with you guys in the future to sort
that out, but I'd prefer to tackle that at a later date as it could
potentially have a much larger footprint than this patchset.
--
paul-moore.com
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-03-18 20:00 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-16 21:35 [PATCH 0/3] Fix incorrect overlayfs mmap() and mprotect() LSM access controls Paul Moore
2026-03-16 21:35 ` [PATCH 1/3] backing_file: store user_path_file Paul Moore
2026-03-18 10:56 ` Christian Brauner
2026-03-18 12:06 ` Amir Goldstein
2026-03-18 20:00 ` Paul Moore
2026-03-16 21:35 ` [PATCH 2/3] lsm: add the security_mmap_backing_file() hook Paul Moore
2026-03-17 22:42 ` Paul Moore
2026-03-16 21:35 ` [PATCH 3/3] selinux: fix overlayfs mmap() and mprotect() access checks Paul Moore
2026-03-16 21:59 ` [PATCH 0/3] Fix incorrect overlayfs mmap() and mprotect() LSM access controls Paul Moore
2026-03-17 7:25 ` Amir Goldstein
2026-03-17 18:16 ` Paul Moore
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox