Re: [PATCH 32/39] mm/madvise: use predicates for madvise(..., MADV_DOFORK)

[sashiko-bot@kernel.org]

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) <ljs@kernel.org>

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-dacf19cce22b@kernel.org?part=32