lkml.org 
[lkml]   [2018]   [Sep]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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

    \
     
     \ /
      Last update: 2018-09-04 00:10    [W:4.800 / U:0.660 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site