Re: [PATCH] selinux: fix incorrect execmem checks on overlayfs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] SELinux uses `vma->vm_file` instead of `vma_is_anonymous()` to classify anonymous VMAs, leading to incorrect permission checks.
--

commit 2446671db8e6a83fbf4c3451f231d685bec26e4e
Author: Ondrej Mosnacek <omosnace@redhat.com>

selinux: fix incorrect execmem checks on overlayfs

This commit fixes overlayfs mmap() and mprotect() access checks by skipping
the execmem check in __file_map_prot_check() during a "mounter check". It
introduces a boolean to indicate this state, ensuring the check is only
performed against the calling task's credentials.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 0f704380a8c81..fca310b7270f3 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
[ ... ]
> @@ -3988,7 +3989,7 @@ static int __file_map_prot_check(const struct cred *cred,
>  			inode = file_inode(file);
>  	}
>  
> -	if (default_noexec && prot_exec &&
> +	if (!mounter_check && default_noexec && prot_exec &&
>  	    (!file || IS_PRIVATE(inode) || (!shared && prot_write))) {

[Severity: High]
This isn't a bug introduced by this patch, but is it correct to rely on
!file to identify anonymous mappings here?

Using a file pointer to check for anonymous VMAs instead of
vma_is_anonymous() can misclassify mappings. A private mapping of /dev/zero
has vma->vm_file != NULL but is functionally an anonymous mapping. This could
allow an anonymous mapping to bypass the PROCESS__EXECMEM check.

This same pre-existing pattern appears in selinux_file_mprotect(), which uses
!file to check for anonymous stack mappings:

security/selinux/hooks.c:selinux_file_mprotect() {
    ...
    } else if (!file && (vma_is_initial_stack(vma) ||
                vma_is_stack_for_current(vma))) {
    ...
}

It also applies the FILE__EXECMOD check to the backing file if vma->anon_vma
is set, which would incorrectly check the backing device for /dev/zero:

security/selinux/hooks.c:selinux_file_mprotect() {
    ...
    } else if (file && vma->anon_vma) {
        /* ... */
        rc = __file_has_perm(cred, file, FILE__EXECMOD,
                             backing_file);
    ...
}

Could this allow SELinux policies to be bypassed for driver mappings that call
vma_set_anonymous() by incorrectly treating them as file-backed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703114733.1243995-1-omosnace@redhat.com?part=1