Re: [PATCH v2 1/6] mm: export vmf_insert_pfn_prot_mkwrite(), change variants to inline

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

Pre-existing issues:
- [Critical] The global shared zero page can be mapped as writable by inser=
t_pfn, leading to memory corruption.
--

--- Patch [1]: [PATCH v2 1/6] mm: export vmf_insert_pfn_prot_mkwrite(), cha=
nge variants to inline ---
commit 010e09346cc93d215af7040a0cfe7fd891a1bd7d
Author: Paolo Bonzini <[email protected]>

mm: export vmf_insert_pfn_prot_mkwrite(), change variants to inline

This commit introduces vmf_insert_pfn_prot_mkwrite() and
vmf_insert_pfn_mkwrite() to allow drivers to install writable PTEs during
faults when a mapping is under write notification. It refactors the existing
vmf_insert_pfn() family of functions into inline wrappers around this new,
more flexible variant.

> diff --git a/mm/memory.c b/mm/memory.c
> index 6637c5b13c9ba..786b114d9921a 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
[ ... ]
> -vm_fault_t vmf_insert_pfn_prot(struct vm_area_struct *vma, unsigned long=
 addr,
> -			unsigned long pfn, pgprot_t pgprot)
> +vm_fault_t vmf_insert_pfn_prot_mkwrite(struct vm_area_struct *vma,
> +			unsigned long addr, unsigned long pfn, pgprot_t pgprot,
> +			bool mkwrite)
>  {
[ ... ]
>  	pfnmap_setup_cachemode_pfn(pfn, &pgprot);
> =20
> -	return insert_pfn(vma, addr, pfn, pgprot, false);
> -}
> -EXPORT_SYMBOL(vmf_insert_pfn_prot);
> -
> -/**
> - * vmf_insert_pfn - insert single pfn into user vma
[ ... ]
> +	return insert_pfn(vma, addr, pfn, pgprot, mkwrite);
>  }
> -EXPORT_SYMBOL(vmf_insert_pfn);
> +EXPORT_SYMBOL(vmf_insert_pfn_prot_mkwrite);

[Severity: Critical]
This is a pre-existing issue, but by allowing mkwrite to be passed directly=
=20
through to insert_pfn(), can this result in the global shared zero page bei=
ng
mapped writable?

If a driver maps the zero page via vmf_insert_pfn_mkwrite(..., true) into a
shared writable VM_PFNMAP mapping, insert_pfn() in mm/memory.c handles the
new entry like this:

	/* Ok, finally just insert the thing.. */
	entry =3D pte_mkspecial(pfn_pte(pfn, prot));

	if (mkwrite) {
		entry =3D pte_mkyoung(entry);
		entry =3D maybe_mkwrite(pte_mkdirty(entry), vma);
	}

Because there is no check verifying if the passed PFN is the zero page,
maybe_mkwrite() will explicitly make the PTE writable, bypassing the zero
page protections ordinarily afforded by vm_mixed_ok().=20

If the global zero page becomes writable, unprivileged userspace could write
to it, which would cause memory corruption across the system for any process
reading uninitialized memory. Should we add a defense in insert_pfn() to
explicitly reject or write-protect the zero page when mkwrite is true?

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260804120529.1730=
[email protected]?part=3D1