Messages in this thread | | | Subject | Re: [RFC PATCH 1/2] x86: Don't let pgprot_modify() change the page encryption bit | From | Thomas Hellström (VMware) <> | Date | Thu, 5 Sep 2019 18:29:25 +0200 |
| |
On 9/5/19 5:59 PM, Dave Hansen wrote: > On 9/5/19 8:21 AM, Thomas Hellström (VMware) wrote: >>>> #define pgprot_modify pgprot_modify >>>> static inline pgprot_t pgprot_modify(pgprot_t oldprot, pgprot_t >>>> newprot) >>>> { >>>> - pgprotval_t preservebits = pgprot_val(oldprot) & _PAGE_CHG_MASK; >>>> - pgprotval_t addbits = pgprot_val(newprot); >>>> + pgprotval_t preservebits = pgprot_val(oldprot) & >>>> + (_PAGE_CHG_MASK | sme_me_mask); >>>> + pgprotval_t addbits = pgprot_val(newprot) & ~sme_me_mask; >>>> return __pgprot(preservebits | addbits); >>>> } >>> _PAGE_CHG_MASK is claiming similar functionality about preserving bits >>> when changing PTEs: > ... >>>> #define _PAGE_CHG_MASK (PTE_PFN_MASK | _PAGE_PCD | _PAGE_PWT >>>> | \ >>>> _PAGE_SPECIAL | _PAGE_ACCESSED | >>>> _PAGE_DIRTY | \ >>>> _PAGE_SOFT_DIRTY | _PAGE_DEVMAP) >>> This makes me wonder if we should be including sme_me_mask in >>> _PAGE_CHG_MASK (logically). >> I was thinking the same. But what confuses me is that addbits isn't >> masked with ~_PAGE_CHG_MASK, which is needed for sme_me_mask, since the >> problem otherwise is typically that the encryption bit is incorrectly >> set in addbits. I wonder whether it's an optimization or intentional. > I think there's a built-in assumption that 'newprot' won't have any of > the _PAGE_CHG_MASK bits set. That makes sense because there are no > protection bits in the mask. But, the code certainly doesn't enforce that. > > Are you seeing 'sme_me_mask' bits set in 'newprot'?
Yes. AFAIK it's only one bit, and typically always set.
/Thomas
| |