Re: [stable/linux-6.6.y 3/3] selinux: fix overlayfs mmap() and mprotect() access checks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `PROCESS__EXECMEM` permission is incorrectly evaluated against the mounter's credentials for backing files, causing spurious access denials.
- [Low] The patch references undefined LSM hooks and structures for backing files, causing a compilation failure.
--

commit 65acd7a339b0b894f76445b113fd808e3582ef18
Author: Paul Moore <paul@paul-moore.com>

selinux: fix overlayfs mmap() and mprotect() access checks

The existing SELinux security model for overlayfs allows access if the
current task can access the top-level file and the mounter can access the
backing file. This commit updates the mmap() and mprotect() operations
to use the newly created LSM hooks to properly enforce access controls
for the backing file.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 60092d0b013ce..3f11c5ae8fbf0 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c

[ ... ]

> @@ -3755,42 +3789,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)
>  {

[ ... ]

> +	if (default_noexec && prot_exec &&
> +	    (!file || IS_PRIVATE(inode) || (!shared && prot_write))) {
> +		int rc;
> +		u32 sid = cred_sid(cred);
>  
>  		/*
> -		 * 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);

[Severity: High]
When this is evaluated for a backing file, is the check performed against
the mounter's credentials instead of the current task?

In selinux_mmap_backing_file(), the mounter's credentials are passed:

selinux_mmap_backing_file()
  selinux_mmap_file_common(backing_file->f_cred, ...)
    file_map_prot_check(cred, ...)
      __file_map_prot_check(cred, ...)

This leads to evaluating the process-level PROCESS__EXECMEM permission against
the mounter's SID rather than the actual process mapping the memory.

Could this cause spurious access denials if the mounter lacks execmem
permissions, breaking containerized applications?

[ ... ]

> @@ -6870,6 +6981,7 @@ static void selinux_bpf_prog_free(struct bpf_prog_aux *aux)
>  struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
>  	.lbs_cred = sizeof(struct task_security_struct),
>  	.lbs_file = sizeof(struct file_security_struct),
> +	.lbs_backing_file = sizeof(struct backing_file_security_struct),

[Severity: Low]
Does this assign to a non-existent field in struct lsm_blob_sizes?

The core LSM infrastructure doesn't appear to define .lbs_backing_file in
this tree. Is this patch missing prerequisite commits from the backport?

[ ... ]

> @@ -7074,9 +7186,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),

[Severity: Low]
Are backing_file_alloc and mmap_backing_file missing their definitions
in the core LSM infrastructure?

Attempting to initialize these hooks causes a compilation failure here.

> diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
> index 8159fd53c3de2..541933dd295c0 100644
> --- a/security/selinux/include/objsec.h
> +++ b/security/selinux/include/objsec.h

[ ... ]

> @@ -158,6 +162,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;
> +}

[Severity: Low]
Is backing_file_security() an undeclared function in this codebase?

Since the prerequisite LSM hooks for backing files appear to be missing,
calling this will result in a build failure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622031556.2664089-4-caixinchen1@huawei.com?part=1