linux-kernel - Re: [PATCH 2/4] x86/asm/entry: Untangle 'ia32_sysenter_target' into two entry points: entry_SYSENTER_32 and entry_SYSENTER

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]

Message-ID: <20150609093314.GA9675@gmail.com>
Date:	Tue, 9 Jun 2015 11:33:14 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Andy Lutomirski <luto@...capital.net>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	Brian Gerst <brgerst@...il.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Borislav Petkov <bp@...en8.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 2/4] x86/asm/entry: Untangle 'ia32_sysenter_target' into
 two entry points: entry_SYSENTER_32 and entry_SYSENTER_compat


* Andy Lutomirski <luto@...capital.net> wrote:

> On Mon, Jun 8, 2015 at 1:34 AM, Ingo Molnar <mingo@...nel.org> wrote:
> > So the SYSENTER instruction is pretty quirky and it has different behavior
> > depending on bitness and CPU maker.
> >
> > Yet we create a false sense of coherency by naming it 'ia32_sysenter_target'
> > in both of the cases.
> >
> > Split the name into its two uses:
> >
> >         ia32_sysenter_target (32)    -> entry_SYSENTER_32
> >         ia32_sysenter_target (64)    -> entry_SYSENTER_compat
> >
> 
> Now that I'm rebasing my pile on top of this, I have a minor gripe
> about this one.  There are (in my mind, anyway), two SYSENTER
> instructions: the 32-bit one and the 64-bit one.  (That is, there's
> SYSENTER32, which happens when you do SYSENTER in 32-bit or compat
> mode, and SYSENTER64, which happens when you do SYSENTER in long
> mode.)  SYSENTER32, from user code's perspective, does the same thing
> in either case [1].  That means that it really does make sense that
> we'd have two implementations of the same entry point, one written in
> 32-bit asm and one written in 64-bit asm.
> 
> The patch I'm rebasing merges the two wrmsrs to MSR_IA32_SYSENTER, and
> this change makes it uglier.
> 
> [1] Sort of.  We probably have differently nonsensical calling
> conventions, but that's our fault and has nothing to do with the
> hardware.

Did you intend to merge these two wrmsr()s:

 #ifdef CONFIG_X86_64
 void syscall_init(void)
 {
 ...
         wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (u64)entry_SYSENTER_compat);
 ...
 }
 #endif

 #ifdef CONFIG_X86_32
 void enable_sep_cpu(void)
 {
 ...
         wrmsr(MSR_IA32_SYSENTER_EIP, (unsigned long)entry_SYSENTER_32, 0);
 ...
 }

... and the new bifurcated names preserve the #ifdef, right?

So I mostly agree with you, but still I'm a bit torn about this, for the following 
reason:

 - SYSENTER on a 32-bit kernel behaves a bit differently from SYSENTER on a 64-bit
   kernel: for example on 32-bit kernels we'll return with SYSEXIT, while on
   64-bit kernels we return with SYSRET. The difference is small but user-space
   observable: for example EDX is 0 on SYSRET while it points to ->sysenter_return
   in the SYSEXIT case.

This kind of user-observable assymmetry does not exist for other unified syscall 
ABIs, such as the INT80 method.

So I think that despite having to preserve a small non-unified #ifdef for this 
initialization, we are still better off naming the two entry points differently, 
along the pattern we use, because the behavior is slightly different depending on 
the bitness of the kernel.

Thanks,

	Ingo
--
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/