linux-kernel - Re: [PATCH] x86/asm/64: do not clear high 32 bits of syscall number when CONFIG_X86

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] [day] [month] [year] [list]

Date:   Sun, 17 Sep 2017 19:45:07 +0300
From:   "Dmitry V. Levin" <ldv@...linux.org>
To:     hpa@...or.com
Cc:     Ingo Molnar <mingo@...nel.org>, Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, X86 ML <x86@...nel.org>,
        Oleg Nesterov <oleg@...hat.com>,
        Eugene Syromyatnikov <evgsyr@...il.com>,
        linux-kernel@...r.kernel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "Dmitry V. Levin" <ldv@...linux.org>
Subject: Re: [PATCH] x86/asm/64: do not clear high 32 bits of syscall number
 when CONFIG_X86_X32=y

On Thu, Sep 14, 2017 at 10:46:37PM -0700, hpa@...or.com wrote:
> On September 14, 2017 10:31:55 PM PDT, Ingo Molnar <mingo@...nel.org> wrote:
> >
> >* Andy Lutomirski <luto@...nel.org> wrote:
> >
> >> >> > diff --git a/arch/x86/entry/entry_64.S
> >b/arch/x86/entry/entry_64.S
> >> >> > index 4916725..3bab6af 100644
> >> >> > --- a/arch/x86/entry/entry_64.S
> >> >> > +++ b/arch/x86/entry/entry_64.S
> >> >> > @@ -185,12 +185,10 @@ entry_SYSCALL_64_fastpath:
> >> >> >          */
> >> >> >         TRACE_IRQS_ON
> >> >> >         ENABLE_INTERRUPTS(CLBR_NONE)
> >> >> > -#if __SYSCALL_MASK == ~0
> >> >> > -       cmpq    $__NR_syscall_max, %rax
> >> >> > -#else
> >> >> > -       andl    $__SYSCALL_MASK, %eax
> >> >> > -       cmpl    $__NR_syscall_max, %eax
> >> >> > +#if __SYSCALL_MASK != ~0
> >> >> > +       andq    $__SYSCALL_MASK, %rax
> >> >> >  #endif
> >> >> > +       cmpq    $__NR_syscall_max, %rax
> >> >>
> >> >> I don't know much about x32 userspace, but there's an argument
> >that
> >> >> the high bits *should* be masked off if the x32 bit is set.
> >> >
> >> > Why?
> >> 
> >> Because it always worked that way.
> >> 
> >> That being said, I'd be okay with applying your patch and seeing
> >> whether anything breaks.  Ingo?
> >
> >So I believe this was introduced with x32 as a 'fresh, modern syscall
> >ABI' 
> >behavioral aspect, because we wanted to protect the overly complex
> >syscall entry 
> >code from 'weird' input values. IIRC there was an old bug where we'd
> >overflow the 
> >syscall table in certain circumstances ...
> >
> >But our new, redesigned entry code is a lot less complex, a lot more
> >readable and 
> >a lot more maintainable (not to mention a lot more robust), so if
> >invalid RAX 
> >values with high bits set get reliably turned into -ENOSYS or such then
> >I'd not 
> >mind the patch per se either, as a general consistency improvement.
> >
> >Of course if something in x32 user-land breaks then this turns into an
> >ABI and we 
> >have to reintroduce this aspect, as a quirk :-/
> >
> >It should also improve x32 syscall performance a tiny bit, right? So
> >might be 
> >worth a try on various grounds.
> >
> >( Another future advantage would be that _maybe_ we could use the high
> >RAX 
> >component as an extra (64-bit only) special argument of sorts. Not that
> >I can 
> >  think of any such use right now. )
> >
> >Thanks,
> >
> >	Ingo
> 
> x32 should be sharing the native 64-but entry code, by design.
> We have had the latter mask the upper 32 bits,

There must be some misunderstanding on your side: the clearing
of the upper 32 bits of 64-bit syscall numbers currently happens
if and only if fastpath and CONFIG_X86_X32_ABI is enabled.


-- 
ldv

Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)