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 insert_pfn, leading to memory corruption. -- --- Patch [1]: [PATCH v2 1/6] mm: export vmf_insert_pfn_prot_mkwrite(), change 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); > > - 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 through to insert_pfn(), can this result in the global shared zero page being 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 = pte_mkspecial(pfn_pte(pfn, prot)); if (mkwrite) { entry = pte_mkyoung(entry); entry = 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(). 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? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
