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

Pre-existing issues:
- [High] MADV_WIPEONFORK and __MADV_SET_ANON_VMA_NAME incorrectly use 
vma->vm_file to classify anonymous VMAs, falsely rejecting private /dev/zero 
mappings.
--

commit eca1ad64ddd91aa6f5fb87e2a56f40c30ef8259e
Author: Lorenzo Stoakes (ARM) <[email protected]>

mm/madvise: use predicates for madvise(..., MADV_DOFORK)

This commit updates MADV_DOFORK to use vma_can_merge() instead of manually
checking for VM_SPECIAL flags. This clarifies the intent, ensuring that if a
mapping isn't allowed to be merged, its CoW behavior cannot be manipulated on
fork. No functional change is intended.

> diff --git a/mm/madvise.c b/mm/madvise.c
> index 0fc31835fee1a..f805a4876c875 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -1400,7 +1400,7 @@ static int madvise_vma_behavior(struct madvise_behavior 
> *madv_behavior)
>               new_flags |= VM_DONTCOPY;
>               break;
>       case MADV_DOFORK:
> -             if (new_flags & VM_SPECIAL)
> +             if (!vma_can_merge(vma))
>                       return -EINVAL;
>               new_flags &= ~VM_DONTCOPY;
>               break;

[Severity: High]
This isn't a bug introduced by this patch, but just below this in
madvise_vma_behavior(), MADV_WIPEONFORK incorrectly uses vma->vm_file to
classify anonymous VMAs:

        case MADV_WIPEONFORK:
                /* MADV_WIPEONFORK is only supported on anonymous memory. */
                if (vma->vm_file || new_flags & VM_SHARED)
                        return -EINVAL;

Does this condition falsely reject private /dev/zero mappings because they
retain a file reference?

If userspace ignores this failure, wouldn't memory intended to be wiped
(such as cryptographic secrets) be copied to child processes during fork,
resulting in a cross-process information leak?

Should this use vma_is_anonymous(vma) instead?

[Severity: High]
This is also a pre-existing issue, but further down in the same function,
__MADV_SET_ANON_VMA_NAME has a similar check:

        case __MADV_SET_ANON_VMA_NAME:
                /* Only anonymous mappings can be named */
                if (vma->vm_file && !vma_is_anon_shmem(vma))
                        return -EBADF;
                break;

Does this also falsely reject private /dev/zero mappings by relying on
vma->vm_file to enforce the anonymous mapping restriction?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/20260908-b4-mmap-prepare-vma-flag-sanify-v1-0-dacf19cce...@kernel.org?part=32

Reply via email to