On 1/21/21 2:16 PM, David Laight wrote:

From: Yu, Yu-cheng 

quoted

On 1/21/2021 10:44 AM, Borislav Petkov wrote:

quoted

On Tue, Dec 29, 2020 at 01:30:35PM -0800, Yu-cheng Yu wrote:

[...]

quoted

quoted

@@ -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;

quoted

quoted

?

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)