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

On Fri, Jul 3, 2026 at 2:05 PM <[email protected]> wrote:
>
> 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 <[email protected]>
>
> 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.

I don't think this is a problem. There are only a few places where
vma_set_anonymous() is called:
1. mmap_zero_private_success()/mmap_zero_prepare() - this is only
called for MAP_PRIVATE mappings of /dev/zero, where execmem will be
checked if the mapping is writable. If it's not writable, it's the
same situation as creating an RX MAP_PRIVATE mapping of a file that
contains zeros - i.e. the ability to do this will be evaluated by
checking read+execute perms on the /dev/zero inode, which seems better
than treating it as an anonymous mapping.
2. do_mmap_private() - here it is only called when vma->vm_file is NULL.
3. __mmap_new_vma() - here it is only called when vma->vm_file is set to NULL.
4. do_brk_flags() - here it is called for the heap VMA, which is never
file-backed.
5. create_init_stack_vma() - here it is called for the initial stack
VMA, which is never file-backed.

> 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))) {
>     ...
> }

Initial stack shouldn't be file-backed, so that's a non-issue.

> 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);
>     ...
> }

The LLM seems to be a bit confused here... The quoted code would check
against the user file (not backing file), and IMHO correctly so. If
the user privately mmaps /dev/zero RW, then modifies the mapping, and
then makes the region executable, it *is* essentially "executing" a
modified version of (possibly overlayed) /dev/zero. And the subsequent
check would then check mounter creds against the backing /dev/zero,
which is also correct.

I don't know, maybe mapping /dev/zero is so close to just doing an
anonymous mapping that it's a bit pointless to take its inodes into
account, but it doesn't seem outright incorrect to me, either.

--
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.