linux-kernel - Re: [PATCH v2 24/36] x86/entry/compat: Implement opportunistic SYSRETL for compat syscalls

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 14 Oct 2015 09:25:24 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Borislav Petkov <bp@...en8.de>
Cc:	Andy Lutomirski <luto@...nel.org>, X86 ML <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Brian Gerst <brgerst@...il.com>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH v2 24/36] x86/entry/compat: Implement opportunistic
 SYSRETL for compat syscalls

On Mon, Oct 12, 2015 at 9:15 AM, Borislav Petkov <bp@...en8.de> wrote:
> On Mon, Oct 05, 2015 at 05:48:12PM -0700, Andy Lutomirski wrote:
>> If CS, SS and IP are as expected and FLAGS is compatible with SYSRETL,
>> then return from fast compat syscalls (both SYSCALL and SYSENTER) using
>> SYSRETL.
>>
>> Unlike native 64-bit opportunistic SYSRET, this is not invisible to
>> user code: RCX and R8-R15 end up in a different state than shown
>> saved in pt_regs.  To compensate, we only do this when returning to
>> the vDSO fast syscall return path.  This won't interfere with
>> syscall restart, as we won't use SYSRETL when returning to the INT80
>> restart instruction.
>>
>> Signed-off-by: Andy Lutomirski <luto@...nel.org>
>> ---
>>  arch/x86/entry/common.c          | 23 +++++++++++++++++++---
>>  arch/x86/entry/entry_64_compat.S | 42 ++++++++++++++++++++++++++++++++++++++--
>>  2 files changed, 60 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
>> index 1b2606edc621..88dc5ba14d47 100644
>> --- a/arch/x86/entry/common.c
>> +++ b/arch/x86/entry/common.c
>> @@ -363,7 +363,8 @@ __visible void do_int80_syscall_32(struct pt_regs *regs)
>>       syscall_return_slowpath(regs);
>>  }
>>
>> -__visible void do_fast_syscall_32(struct pt_regs *regs)
>> +/* Returns 0 to return using IRET or 1 to return using SYSRETL. */
>> +__visible long do_fast_syscall_32(struct pt_regs *regs)
>>  {
>>       /*
>>        * Called using the internal vDSO SYSENTER/SYSCALL32 calling
>> @@ -395,12 +396,28 @@ __visible void do_fast_syscall_32(struct pt_regs *regs)
>>               enter_from_user_mode();
>>  #endif
>>               prepare_exit_to_usermode(regs);
>> -             return;
>> +             return 0;       /* Keep it simple: use IRET. */
>>       }
>>       local_irq_disable();
>>
>>       /* Now this is just like a normal syscall. */
>>       do_int80_syscall_32(regs);
>> -     return;
>> +
>> +#ifdef CONFIG_X86_64
>> +     /*
>> +      * Opportunistic SYSRETL: if possible, try to return using SYSRETL.
>> +      * SYSRETL is available on all 64-bit CPUs, so we don't need to
>> +      * bother with SYSEXIT.
>> +      *
>> +      * Unlike 64-bit opportunistic SYSRET, we can't check that CX == IP,
>> +      * because the ECX fixup above will ensure that this is essentially
>> +      * never the case.
>> +      */
>> +     return regs->cs == __USER32_CS && regs->ss == __USER_DS &&
>> +             regs->ip == landing_pad &&
>> +             (regs->flags & (X86_EFLAGS_RF | X86_EFLAGS_TF)) == 0;
>
> This could've used some readability massaging:
>
>         return    regs->cs == __USER32_CS &&
>                   regs->ss == __USER_DS &&
>                   regs->ip == landing_pad &&
>                 !(regs->flags & (X86_EFLAGS_RF | X86_EFLAGS_TF));
>
> Although I'm not crazy about that one either. This expression is simply not
> parseable at a glance.
>
> Why not write it like the 64-bit opportunistic SYSRET:
>
>         if (regs->cs != __USER32_CS)
>                 return 0;
>
>         if (regs->ss != __USER_DS)
>                 return 0;
>
> and so on...

We could do that.  It might also be nice to factor it into its own function:

static bool may_fast_exit_32(struct pt_regs *regs);

I'll try that eventually if you don't beat me.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/