Messages in this thread | | | Subject | Re: [RFC PATCH 1/2] x86: Don't let pgprot_modify() change the page encryption bit | From | Dave Hansen <> | Date | Thu, 5 Sep 2019 08:59:32 -0700 |
| |
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'?
| |