Re: [PATCH 32/39] mm/madvise: use predicates for madvise(..., MADV_DOFORK)
From: sashiko-bot@kernel.org
Date: 2026-09-08 20:48:07
Also in:
bpf, dri-devel, kvm, kvmarm, linux-perf-users, linux-s390, linux-scsi, selinux
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) [off-list ref] 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.
quoted hunk ↗ jump to hunk
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