From: Yu, Yu-cheng > > On 1/21/2021 10:44 AM, Borislav Petkov wrote: > > On Tue, Dec 29, 2020 at 01:30:35PM -0800, Yu-cheng Yu wrote: > [...] > >> @@ -343,6 +349,16 @@ static inline pte_t pte_mkold(pte_t pte) > >> > >> static inline pte_t pte_wrprotect(pte_t pte) > >> { > >> + /* > >> + * Blindly clearing _PAGE_RW might accidentally create > >> + * a shadow stack PTE (RW=0, Dirty=1). Move the hardware > >> + * dirty value to the software bit. > >> + */ > >> + if (cpu_feature_enabled(X86_FEATURE_SHSTK)) { > >> + pte.pte |= (pte.pte & _PAGE_DIRTY) >> _PAGE_BIT_DIRTY << _PAGE_BIT_COW; > > > > Why the unreadable shifting when you can simply do: > > > > if (pte.pte & _PAGE_DIRTY) > > pte.pte |= _PAGE_COW; > > > > ? > > It clears _PAGE_DIRTY and sets _PAGE_COW. That is, > > if (pte.pte & _PAGE_DIRTY) { > pte.pte &= ~_PAGE_DIRTY; > pte.pte |= _PAGE_COW; > } > > So, shifting makes resulting code more efficient. Does the compiler manage to do one shift? How can it clear anything? There is only an |= against the target. Something horrid with ^= might set and clear. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)