Re: [PATCH v2 stable/linux-6.18.y 2/2] selinux: fix overlayfs mmap() and mprotect() access checks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] SELinux `PROCESS__EXECMEM` permission is incorrectly checked against the mounter's credentials for overlayfs backing files.
--

commit 05257f77d90324327d84ac084d1841f873b3d958
Author: Paul Moore <paul@paul-moore.com>
    
selinux: fix overlayfs mmap() and mprotect() access checks

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.

Cc: stable@vger.kernel.org
Acked-by: Amir Goldstein <amir73il@gmail.com>

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c

[ ... ]

> @@ -3916,42 +3950,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);

[Severity: High]
Does this incorrectly check PROCESS__EXECMEM against the mounter's credentials
instead of the current task?

When this is called for an overlayfs backing file, the mounter's credentials
are passed in:

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

This means if an application requests PROT_EXEC and PROT_WRITE on a private
overlayfs file mapping (like a JIT compiler), the PROCESS__EXECMEM check is
evaluated against the mounter's SID rather than the mapping process's SID.

If the mounter (e.g., the container runtime) lacks the execmem permission,
could this cause the application's memory allocation to fail with -EACCES?

[ ... ]

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