Re: [PATCH v3 2/4] x86/syscalls: Specific usage of verify_pre_usermode_state

On 03/14/17 08:39, Andy Lutomirski wrote:

quoted

Ingo: Which approach do you favor? I want to keep the fast path as
fast as possible obviously.

Even though my name isn't Ingo, Linus keeps trying to get me to be the
actual maintainer of this file.  :)  How about (sorry about whitespace
damage):

#ifdef CONFIG_BUG_ON_DATA_CORRUPTION
       movq    PER_CPU_VAR(current_task), %rax
       bt $63, TASK_addr_limit(%rax)
       jc     syscall_return_slowpath
#endif

Now the kernel is totally unchanged if the config option is off and
it's fast and simple if the option is on.

The idea as far as I understand was that the option was about whether or
not to clobber the broken value or BUG on it, not to remove the check.
My point, though, was that we can bail out to the slow path if there is
a discrepancy and worry about BUG or not there; performance doesn't
matter one iota if this triggers regardless of the remediation.

It isn't clear that using bt would be faster, though; although it saves
an instruction that instruction can be hoisted arbitrarily and so is
extremely likely to be hidden in the pipeline.  cmp (which is really a
variant of sub) is one of the basic ALU instructions that are
super-optimized on every CPU, whereas bt is substantially slower on some
implementations.

This version is also "slightly less secure" since it would make it
possible to overwrite the guard page at the end of TASK_SIZE_MAX if one
could figure out a way to put an arbitrary value into this variable, but
I doubt that matters in any way.

	-hpa