On 1/21/21 2:16 PM, David Laight wrote: > 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? It could shift it off either end since there are both << and >>. > There is only an |= against the target. > > Something horrid with ^= might set and clear. -- ~Randy "He closes his eyes and drops the goggles. You can't get hurt by looking at a bitmap. Or can you?" (Neal Stephenson: Snow Crash)