Messages in this thread | | | Date | Wed, 20 Feb 2019 19:48:59 +0100 | From | Solar Designer <> | Subject | Re: [PATCH v2] x86/asm: Pin sensitive CR4 bits |
| |
On Wed, Feb 20, 2019 at 10:09:34AM -0800, Kees Cook wrote: > +extern volatile unsigned long cr4_pin; > + > static inline void native_write_cr4(unsigned long val) > { > +again: > + val |= cr4_pin; > asm volatile("mov %0,%%cr4": : "r" (val), "m" (__force_order)); > + /* > + * If the MOV above was used directly as a ROP gadget we can > + * notice the lack of pinned bits in "val" and start the function > + * from the beginning to gain the cr4_pin bits for sure. > + */ > + if (WARN_ONCE((val & cr4_pin) != cr4_pin, "cr4 bypass attempt?!\n")) > + goto again; > }
I think "goto again" is too mild a response given that it occurs after a successful write of a non-pinned value to CR4. I think it'd allow some exploits to eventually win the race: make their desired use of whatever functionality SMEP, etc. would have prevented - which may be just a few instructions they need to run - before the CR4 value is reverted after "goto again". I think it's one of those cases where a kernel panic would be more appropriate.
Also, WARN_ONCE possibly introduces a delay sufficient to realistically win this race on the first try. If we choose to warn, we should do it after having reverted the CR4 value, not before.
Alexander
| |