Messages in this thread | | | From | Andy Lutomirski <> | Date | Wed, 18 Dec 2019 18:39:09 -0800 | Subject | Re: [PATCH] x86-64/entry: add instruction suffix to SYSRET |
| |
On Mon, Dec 16, 2019 at 7:23 AM Brian Gerst <brgerst@gmail.com> wrote: > > On Mon, Dec 16, 2019 at 5:12 AM Jan Beulich <jbeulich@suse.com> wrote: > > > > On 13.12.2019 18:49, Andy Lutomirski wrote: > > > On Fri, Dec 13, 2019 at 1:55 AM Jan Beulich <jbeulich@suse.com> wrote: > > >> > > >> On 12.12.2019 22:43, Andy Lutomirski wrote: > > >>> On Tue, Dec 10, 2019 at 7:40 AM Jan Beulich <jbeulich@suse.com> wrote: > > >>>> > > >>>> On 10.12.2019 16:29, Andy Lutomirski wrote: > > >>>>>> On Dec 10, 2019, at 2:48 AM, Jan Beulich <JBeulich@suse.com> wrote: > > >>>>>> > > >>>>>> Omitting suffixes from instructions in AT&T mode is bad practice when > > >>>>>> operand size cannot be determined by the assembler from register > > >>>>>> operands, and is likely going to be warned about by upstream gas in the > > >>>>>> future. Add the missing suffix here. > > >>>>>> > > >>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > >>>>>> > > >>>>>> --- a/arch/x86/entry/entry_64.S > > >>>>>> +++ b/arch/x86/entry/entry_64.S > > >>>>>> @@ -1728,7 +1728,7 @@ END(nmi) > > >>>>>> SYM_CODE_START(ignore_sysret) > > >>>>>> UNWIND_HINT_EMPTY > > >>>>>> mov $-ENOSYS, %eax > > >>>>>> - sysret > > >>>>>> + sysretl > > >>>>> > > >>>>> Isn’t the default sysretq? sysretl looks more correct, but that suggests > > >>>>> that your changelog is wrong. > > >>>> > > >>>> No, this is different from ret, and more like iret and lret. > > >>>> > > >>>>> Is this code even reachable? > > >>>> > > >>>> Yes afaict, supported by the comment ahead of the symbol. syscall_init() > > >>>> puts its address into MSR_CSTAR when !IA32_EMULATION. > > >>>> > > >>> > > >>> What I meant was: can a program actually get itself into 32-bit mode > > >>> to execute a 32-bit SYSCALL instruction? > > >> > > >> Why not? It can set up a 32-bit code segment descriptor, far-branch > > >> into it, and then execute SYSCALL. I can't see anything preventing > > >> this in the logic involved in descriptor adjustment system calls. In > > >> fact it looks to be at least partly the opposite - fill_ldt() > > >> disallows creation of 64-bit code segments (oddly enough > > >> fill_user_desc() then still copies the bit back, despite there > > >> apparently being no way for it to get set). > > > > > > Do we allow creation of 32-bit code segments on !IA32_EMULATION > > > kernels? > > > > As per above - I think so. > > > > > I think we shouldn't, but I'm not really sure. > > > > It may be a little exotic, but I can't see any reason to disallow > > a 64-bit process to switch to compatibility mode temporarily. One > > contrived use case could be to be able to invoke INTO or BOUND. > > I think it should be kept intact for future use by WINE. WINE is > currently set up so that 32/16-bit Windows emulation needs a 32-bit > build against 32-bit Linux libraries, using the kernel compat layer. > With many distributions wanting to drop 32-bit support this has been a > big sticking point. If WINE could be modified so that the core is > always built as 64-bit with 32-bit compatibility handled entirely in > userspace, that would remove its dependency on 32-bit Linux libraries > and thus wouldn't require IA32_EMULATION.
I just read an article about WINE on Mac OS doing more or less this. I'm wondering if we should wire up set_thread_area() on x86_64 for uses like this. It even has a syscall number already -- it's just not wired up.
Anyway, this isn't particularly high priority, IMO -- I don't think many people set IA32_EMULATION=n. What we really ought to do is to get rid of the special ignore_sysret path and instead go through the normal syscall path but just do:
if (!IS_ENABLED(IA32_EMULATION)) return -ENOSYS;
or equivalent. The last thing we need is a whole special asm path that essentially no one uses.
| |