Messages in this thread |  | | Date | Tue, 4 Sep 2018 00:10:32 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH v3 3/5] x86/mm: add help function to check specific protection flags in range |
| |
On Tue, 21 Aug 2018, Bin Yang wrote: > /* > + * static_protections() "forces" page protections for some address > + * ranges. Return true if any part of the address/len range is forced > + * to change from 'prot'. > + */ > +static inline bool > +needs_static_protections(pgprot_t prot, unsigned long address, > + unsigned long len, unsigned long pfn) > +{ > + int i; > + > + address &= PAGE_MASK; > + len = PFN_ALIGN(len); > + for (i = 0; i < (len >> PAGE_SHIFT); i++, address += PAGE_SIZE, pfn++) { > + pgprot_t chk_prot = static_protections(prot, address, pfn); > + > + if (pgprot_val(chk_prot) != pgprot_val(prot)) > + return true; > + } > + > + /* Does static_protections() demand a change ? */ > + return false; > +}
...
> if (cpa->force_split) > @@ -660,14 +684,8 @@ try_preserve_large_page(pte_t *kpte, unsigned long address, > * static_protection() requires a different pgprot for one of > * the pages in the range we try to preserve: > */ > - pfn = old_pfn; > - for (i = 0; i < (psize >> PAGE_SHIFT); i++, addr += PAGE_SIZE, pfn++) { > - pgprot_t chk_prot = static_protections(req_prot, addr, pfn); > - > - if (pgprot_val(chk_prot) != pgprot_val(new_prot)) > - goto out_unlock; > - } > - > + if (needs_static_protections(new_prot, addr, psize, old_pfn)) > + goto out_unlock;
This is not the same. The existing code does:
new_prot = static_protections(req_prot, address, pfn);
which is the protection updated pgprot for the base of the address range which should be modified. The loop does:
chk_prot = static_protections(req_prot, addr, pfn);
if (chk_prot != new_prot) goto split;
Now mapping your new function back and then the loop becomes:
chk_prot = static_protections(new_prot, addr, pfn);
if (chk_prot != new_prot) goto split;
which is broken in case that after the initial static protections invocation
new_prot = static_protections(req_prot, address, pfn);
the result is:
new_prot != req_prot
and in the loop
new_prot is valid for _ALL_ pages in the large page because the static protection which got applied for the first address can be applied to the complete range, i.e. new_prot it is not further modified by static_protections() for any page.
That again can cause wrong large page preservations.
Thanks,
tglx
|  |