Re: [PATCH v4 3/3] selinux: fix overlayfs mmap() and mprotect() access checks

Amir Goldstein
2026-04-03T06:17:15Z

On Fri, Apr 3, 2026 at 5:09 AM Paul Moore <paul@paul-moore.com> wrote:
>
> 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, and leverages the backing file API and new LSM blob to
> provide the necessary information to properly enforce the mprotect()
> access controls.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Can't say much about selinux implementation, but
for the use of backing file API and the concept solution

Acked-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.

> ---
> security/selinux/hooks.c | 256 +++++++++++++++++++++---------
> security/selinux/include/objsec.h | 11 ++
> 2 files changed, 196 insertions(+), 71 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index d8224ea113d1..76e0fb7dcb36 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1745,6 +1745,60 @@ static inline int file_path_has_perm(const struct cred *cred,
> static int bpf_fd_pass(const struct file *file, u32 sid);
> #endif
>
> +static int __file_has_perm(const struct cred *cred, const struct file *file,
> + u32 av, bool bf_user_file)
> +
> +{
> + struct common_audit_data ad;
> + struct inode *inode;
> + u32 ssid = cred_sid(cred);
> + u32 tsid_fd;
> + int rc;
> +
> + if (bf_user_file) {
> + struct backing_file_security_struct *bfsec;
> + const struct path *path;
> +
> + if (WARN_ON(!(file->f_mode & FMODE_BACKING)))
> + return -EIO;
> +
> + bfsec = selinux_backing_file(file);
> + path = backing_file_user_path(file);
> + tsid_fd = bfsec->uf_sid;
> + inode = d_inode(path->dentry);
> +
> + ad.type = LSM_AUDIT_DATA_PATH;
> + ad.u.path = *path;
> + } else {
> + struct file_security_struct *fsec = selinux_file(file);
> +
> + tsid_fd = fsec->sid;
> + inode = file_inode(file);
> +
> + ad.type = LSM_AUDIT_DATA_FILE;
> + ad.u.file = file;
> + }
> +
> + if (ssid != tsid_fd) {
> + rc = avc_has_perm(ssid, tsid_fd, SECCLASS_FD, FD__USE, &ad);
> + if (rc)
> + return rc;
> + }
> +
> +#ifdef CONFIG_BPF_SYSCALL
> + /* regardless of backing vs user file, use the underlying file here */
> + rc = bpf_fd_pass(file, ssid);
> + if (rc)
> + return rc;
> +#endif
> +
> + /* av is zero if only checking access to the descriptor. */
> + if (av)
> + return inode_has_perm(cred, inode, av, &ad);
> +
> + return 0;
> +}
> +
> /* Check whether a task can use an open file descriptor to
> access an inode in a given way. Check access to the
> descriptor itself, and then use dentry_has_perm to
> @@ -1753,41 +1807,10 @@ static int bpf_fd_pass(const struct file *file, u32 sid);
> has the same SID as the process. If av is zero, then
> 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,
> - u32 av)
> +static inline int file_has_perm(const struct cred *cred,
> + const struct file *file, u32 av)
> {
> - struct file_security_struct *fsec = selinux_file(file);
> - struct inode *inode = file_inode(file);
> - struct common_audit_data ad;
> - u32 sid = cred_sid(cred);
> - int rc;
> -
> - ad.type = LSM_AUDIT_DATA_FILE;
> - ad.u.file = file;
> -
> - if (sid != fsec->sid) {
> - rc = avc_has_perm(sid, fsec->sid,
> - SECCLASS_FD,
> - FD__USE,
> - &ad);
> - if (rc)
> - goto out;
> - }
> -
> -#ifdef CONFIG_BPF_SYSCALL
> - rc = bpf_fd_pass(file, cred_sid(cred));
> - if (rc)
> - return rc;
> -#endif
> -
> - /* av is zero if only checking access to the descriptor. */
> - rc = 0;
> - if (av)
> - rc = inode_has_perm(cred, inode, av, &ad);
> -
> -out:
> - return rc;
> + return __file_has_perm(cred, file, av, false);
> }
>
> /*
> @@ -3825,6 +3848,17 @@ static int selinux_file_alloc_security(struct file *file)
> return 0;
> }
>
> +static int selinux_backing_file_alloc(struct file *backing_file,
> + const struct file *user_file)
> +{
> + struct backing_file_security_struct *bfsec;
> +
> + bfsec = selinux_backing_file(backing_file);
> + bfsec->uf_sid = selinux_file(user_file)->sid;
> +
> + return 0;
> +}
> +
> /*
> * Check whether a task has the ioctl permission and cmd
> * operation to an inode.
> @@ -3942,42 +3976,55 @@ 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, bool bf_user_file)
> {
> - const struct cred *cred = current_cred();
> - u32 sid = cred_sid(cred);
> - int rc = 0;
> + struct inode *inode = NULL;
> + bool prot_exec = prot & PROT_EXEC;
> + bool prot_write = prot & PROT_WRITE;
> +
> + if (file) {
> + if (bf_user_file)
> + inode = d_inode(backing_file_user_path(file)->dentry);
> + else
> + inode = file_inode(file);
> + }
> +
> + if (default_noexec && prot_exec &&
> + (!file || IS_PRIVATE(inode) || (!shared && prot_write))) {
> + int rc;
> + u32 sid = cred_sid(cred);
>
> - if (default_noexec &&
> - (prot & PROT_EXEC) && (!file || IS_PRIVATE(file_inode(file)) ||
> - (!shared && (prot & PROT_WRITE)))) {
> /*
> - * We are making executable an anonymous mapping or a
> - * private file mapping that will also be writable.
> - * This has an additional check.
> + * We are making executable an anonymous mapping or a private
> + * file mapping that will also be writable.
> */
> - rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
> - PROCESS__EXECMEM, NULL);
> + rc = avc_has_perm(sid, sid, SECCLASS_PROCESS, PROCESS__EXECMEM,
> + NULL);
> if (rc)
> - goto error;
> + return rc;
> }
>
> if (file) {
> - /* read access is always possible with a mapping */
> + /* "read" always possible, "write" only if shared */
> u32 av = FILE__READ;
> -
> - /* write access only matters if the mapping is shared */
> - if (shared && (prot & PROT_WRITE))
> + if (shared && prot_write)
> av |= FILE__WRITE;
> -
> - if (prot & PROT_EXEC)
> + if (prot_exec)
> av |= FILE__EXECUTE;
>
> - return file_has_perm(cred, file, av);
> + return __file_has_perm(cred, file, av, bf_user_file);
> }
>
> -error:
> - return rc;
> + return 0;
> +}
> +
> +static inline int file_map_prot_check(const struct cred *cred,
> + const struct file *file,
> + unsigned long prot, bool shared)
> +{
> + return __file_map_prot_check(cred, file, prot, shared, false);
> }
>
> static int selinux_mmap_addr(unsigned long addr)
> @@ -3993,36 +4040,80 @@ 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;
> + bool backing_file;
> + bool shared = vma->vm_flags & VM_SHARED;
> +
> + /* check if we need to trigger the "backing files are awful" mode */
> + backing_file = file && (file->f_mode & FMODE_BACKING);
>
> 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 +4127,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 +4143,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,
> + backing_file);
> + if (rc)
> + return rc;
> + if (backing_file) {
> + rc = file_has_perm(file->f_cred, file,
> + FILE__EXECMOD);
> + if (rc)
> + return rc;
> + }
> }
> + }
> +
> + rc = __file_map_prot_check(cred, file, prot, shared, backing_file);
> + if (rc)
> + return rc;
> + if (backing_file) {
> + rc = file_map_prot_check(file->f_cred, file, prot, 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)
> @@ -7393,6 +7504,7 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
> .lbs_cred = sizeof(struct cred_security_struct),
> .lbs_task = sizeof(struct task_security_struct),
> .lbs_file = sizeof(struct file_security_struct),
> + .lbs_backing_file = sizeof(struct backing_file_security_struct),
> .lbs_inode = sizeof(struct inode_security_struct),
> .lbs_ipc = sizeof(struct ipc_security_struct),
> .lbs_key = sizeof(struct key_security_struct),
> @@ -7498,9 +7610,11 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
>
> LSM_HOOK_INIT(file_permission, selinux_file_permission),
> LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security),
> + LSM_HOOK_INIT(backing_file_alloc, selinux_backing_file_alloc),
> 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),
> diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
> index 5bddd28ea5cb..b19e5d978e82 100644
> --- a/security/selinux/include/objsec.h
> +++ b/security/selinux/include/objsec.h
> @@ -88,6 +88,10 @@ struct file_security_struct {
> u32 pseqno; /* Policy seqno at the time of file open */
> };
>
> +struct backing_file_security_struct {
> + u32 uf_sid; /* associated user file fsec->sid */
> +};
> +
> struct superblock_security_struct {
> u32 sid; /* SID of file system superblock */
> u32 def_sid; /* default SID for labeling */
> @@ -195,6 +199,13 @@ static inline struct file_security_struct *selinux_file(const struct file *file)
> return file->f_security + selinux_blob_sizes.lbs_file;
> }
>
> +static inline struct backing_file_security_struct *
> +selinux_backing_file(const struct file *backing_file)
> +{
> + void *blob = backing_file_security(backing_file);
> + return blob + selinux_blob_sizes.lbs_backing_file;
> +}
> +
> static inline struct inode_security_struct *
> selinux_inode(const struct inode *inode)
> {
> --
> 2.53.0
>