x86,vdso: fix the x86 vdso2c tool includes

The build-time tool arch/x86/vdso/vdso2c.c includes <linux/elf.h>, but cannot find it, unless the build host happens to provide it. It should be reading the uapi linux/elf.h

This build regression came along with the vdso2c between 3.15 and 3.16.

Signed-off-by Tommi Kyntola <[email protected]>

---
 arch/x86/vdso/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/x86/vdso/Makefile +1 -2

index 5a4affe..745e9fe 100644
@@ -50,7 +50,7 @@ VDSO_LDFLAGS_vdso.lds = -m64 -Wl,-soname=linux-vdso.so.1 \
 $(obj)/vdso64.so.dbg: $(src)/vdso.lds $(vobjs) FORCE
        $(call if_changed,vdso)
 
-HOST_EXTRACFLAGS += -I$(srctree)/tools/include
+HOST_EXTRACFLAGS += -I$(srctree)/tools/include -I$(srctree)/include/uapi
 hostprogs-y                    += vdso2c
 
 quiet_cmd_vdso2c = VDSO2C  $@
--

2.1.0

It happened in our custom build system. The kernel there doesn't have a dependency to the kernel headers and that's pretty much it.
It's a tiny thing and I already patched our stuff, but I thought I'd let you know. Breaching the contaiment makes the build prone to some really funky results.

cheers,
Tommi

Quoted 45 lines · Andy Lutomirski

Quoted 7 lines · Tommi Kyntola

The build-time tool arch/x86/vdso/vdso2c.c includes <linux/elf.h>, but cannot find it, unless the build host happens to provide it. It should be reading the uapi linux/elf.h

This build regression came along with the vdso2c between 3.15 and 3.16.

I'm curious: what distro or other system are you seeing this problem in?

--Andy

Quoted 22 lines

Signed-off-by Tommi Kyntola <[email protected]>

---
 arch/x86/vdso/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/x86/vdso/Makefile +1 -4

index 5a4affe..745e9fe 100644
@@ -50,7 +50,7 @@ VDSO_LDFLAGS_vdso.lds = -m64
-Wl,-soname=linux-vdso.so.1 \
 $(obj)/vdso64.so.dbg: $(src)/vdso.lds $(vobjs) FORCE
        $(call if_changed,vdso)
 
-HOST_EXTRACFLAGS += -I$(srctree)/tools/include
+HOST_EXTRACFLAGS += -I$(srctree)/tools/include
-I$(srctree)/include/uapi
 hostprogs-y                    += vdso2c
 
 quiet_cmd_vdso2c = VDSO2C  $@
--

Quoted 2 lines

2.1.0

--
Andy Lutomirski
AMA Capital Management, LLC

Commit-ID:  e7d6eefaaa443130079d73cd05039d90b3db7a4a
Gitweb:     http://git.kernel.org/tip/e7d6eefaaa443130079d73cd05039d90b3db7a4a
Author:     Denys Vlasenko <[email protected]>
AuthorDate: Fri, 27 Mar 2015 11:48:17 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 31 Mar 2015 10:45:15 +0200

x86/vdso32/syscall.S: Do not load __USER32_DS to %ss

This vDSO code only gets used by 64-bit kernels, not 32-bit ones.

On 64-bit kernels, the data segment is the same for 32-bit and 64-bit userspace, and the SYSRET instruction loads %ss with its selector.

So there's no need to repeat it by hand. Segment loads are somewhat
expensive: tens of cycles.

Signed-off-by Denys Vlasenko <[email protected]>

[ Removed unnecessary comment. ]

Signed-off-by Andy Lutomirski <[email protected]>

Cc Alexei Starovoitov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Kees Cook <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Oleg Nesterov <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Will Drewry <[email protected]>

Link http://lkml.kernel.org/r/63da6d778f69fd0f1345d9287f6764d58be519fa.1427482099.git.luto@kernel.org open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/vdso/vdso32/syscall.S | 2 --
 1 file changed, 2 deletions(-)

arch/x86/vdso/vdso32/syscall.S +0 -0

index 5415b56..6b286bb 100644

Quoted 42 lines · tip-bot for Denys Vlasenko

Commit-ID:  e7d6eefaaa443130079d73cd05039d90b3db7a4a
Gitweb:     http://git.kernel.org/tip/e7d6eefaaa443130079d73cd05039d90b3db7a4a
Author:     Denys Vlasenko <[email protected]>
AuthorDate: Fri, 27 Mar 2015 11:48:17 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 31 Mar 2015 10:45:15 +0200

x86/vdso32/syscall.S: Do not load __USER32_DS to %ss

This vDSO code only gets used by 64-bit kernels, not 32-bit ones.

On 64-bit kernels, the data segment is the same for 32-bit and 64-bit userspace, and the SYSRET instruction loads %ss with its selector.

So there's no need to repeat it by hand. Segment loads are somewhat
expensive: tens of cycles.

Signed-off-by Denys Vlasenko <[email protected]>

[ Removed unnecessary comment. ]

Signed-off-by Andy Lutomirski <[email protected]>

Cc Alexei Starovoitov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Kees Cook <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Oleg Nesterov <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Will Drewry <[email protected]>

Link http://lkml.kernel.org/r/63da6d778f69fd0f1345d9287f6764d58be519fa.1427482099.git.luto@kernel.org open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/vdso/vdso32/syscall.S | 2 --
 1 file changed, 2 deletions(-)

arch/x86/vdso/vdso32/syscall.S +0 -0

index 5415b56..6b286bb 100644

arch/x86/vdso/vdso32/syscall.S __kernel_vsyscall: · line 24

Quoted 8 lines

arch/x86/vdso/vdso32/syscall.S +0 -2

@@ -19,8 +19,6 @@ __kernel_vsyscall:
 .Lpush_ebp:
        movl    %ecx, %ebp
        syscall
-       movl    $__USER32_DS, %ecx
-       movl    %ecx, %ss
        movl    %ebp, %ecx
        popl    %ebp
 .Lpop_ebp:

This patch unfortunately is causing Wine to break on some applications:

Unhandled exception: stack overflow in 32-bit code (0xf779bc07). Register dump:
CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b
EIP:f779bc07 ESP:00aed60c EBP:00aed750 EFLAGS:00010216( R- -- I -A-P- )
EAX:00000040 EBX:00000010 ECX:00aed750 EDX:00000040 ESI:00000040 EDI:7ffd4000
Stack dump:

0x00aed60c:  00aed648 f7575e5b 7bcc8000 00000000
0x00aed61c:  7bc7bc09 00000010 00aed750 00000040
0x00aed62c:  00aed750 00aed650 7bcc8000 7bc7bbdd
0x00aed63c:  7bcc8000 00aed6a0 00aed750 00aed738
0x00aed64c:  7bc7cfa9 00000011 00aed750 00000040
0x00aed65c:  00000020 00000000 00000000 7bc4f141
Backtrace:
=>0 0xf779bc07 __kernel_vsyscall+0x7() in [vdso].so (0x00aed750)
  1 0xf7575e5b __libc_read+0x4a() in libpthread.so.0 (0x00aed648)
  2 0x7bc7bc09 read_reply_data+0x38(buffer=0xaed750, size=0x40)

[/home/bgerst/src/wine/wine32/dlls/ntdll/../../../dlls/ntdll/server.c:239] in ntdll (0x00aed648)
3 0x7bc7cfa9 wine_server_call+0x178() in ntdll (0x00aed738)
4 0x7bc840ec NtSetEvent+0x4b(handle=0x80,
NumberOfThreadsReleased=0x0(nil)) [/home/bgerst/src/wine/wine32/dlls/ntdll/../../../dlls/ntdll/sync.c:361] in ntdll (0x00aed7c8)
5 0x7b874afa SetEvent+0x24(handle=<couldn't compute location>)
[/home/bgerst/src/wine/wine32/dlls/kernel32/../../../dlls/kernel32/sync.c:572] in kernel32 (0x00aed7e8)
6 0x0044e31a in battle.net launcher (+0x4e319) (0x00aed818)
...

__kernel_vsyscall+0x7 points to "pop %ebp".

This is on an AMD Phenom(tm) II X6 1055T Processor.

It appears that there are some subtle differences in how sysretl works on AMD vs. Intel. According to the Intel docs, the SS selector and descriptor cache is completely reset by sysret to fixed values. The AMD docs however are concerning:

AMD's syscall:
SS.sel = MSR_STAR.SYSCALL_CS + 8
SS.attr = 64-bit stack,dpl0

SS.base = 0x00000000
SS.limit = 0xFFFFFFFF

AMD's sysret:
SS.sel = MSR_STAR.SYSRET_CS + 8 // SS selector is changed,

                                // SS base, limit, attributes unchanged.

Not changing base or limit is no big deal, but not changing attributes could be the problem. It might be leaving the "64-bit stack" attribute set, for whatever that means.

Reloading SS from the GDT would obviously reset any bad state left by sysretl. Unfortunately we may have to put it back in, and then NOP it out on Intel.

I'm curious whether we can somehow end up in the kernel without a sensible SS. What happens if we have SS = 0?

Try this on for size:

1. Wine process does syscall
2. Context switch to any other task
3. Interrupt (software or hardware), which loads SS with ss0, which is 0 on x86_64.
4. Context switch back to Wine.
5. sysretl

Would fixing this be as simple as changing this code in arch/x86/kernel/process.c:

__visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss) = {
        .x86_tss = {
                .sp0 = TOP_OF_INIT_STACK,
#ifdef CONFIG_X86_32
                .ss0 = __KERNEL_DS,

by moving the ifdef down a line? Even if that fixed it, it would be extremely fragile, but IMO it would be a good change to make regardless (i.e. the kernel's SS would be less unpredictable).

Would fixing this be as simple as changing this code in arch/x86/kernel/process.c:

__visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss) = {
        .x86_tss = {
                .sp0 = TOP_OF_INIT_STACK,
#ifdef CONFIG_X86_32
                .ss0 = __KERNEL_DS,

by moving the ifdef down a line?

I'm pretty sure that this is at least a little bit wrong. It makes no sense for me for syscall to set SS.DPL=0 and for sysret to leave SS.DPL=0. It had better at least change DPL to 3. (Except... don't
they mean RPL? Why is the DPL cached at all? But RPL is clearly
changed, since it's part of the selector.)

Quoted 4 lines

Not changing base or limit is no big deal, but not changing attributes could be the problem. It might be leaving the "64-bit stack" attribute set, for whatever that means.

Hmm. I don't know if I believe that explanation. For one thing, the APM says "Executing SYSRET in non-64-bit mode or with a 16- or 32-bit operand size returns to 32-bit mode with a 32-bit stack pointer."

We can revert this patch or fix it, but I'd like to at least try to understand what's wrong first. Borislav, any ideas?

Quoted 85 lines · Brian Gerst

Quoted 42 lines · tip-bot for Denys Vlasenko

Commit-ID:  e7d6eefaaa443130079d73cd05039d90b3db7a4a
Gitweb:     http://git.kernel.org/tip/e7d6eefaaa443130079d73cd05039d90b3db7a4a
Author:     Denys Vlasenko <[email protected]>
AuthorDate: Fri, 27 Mar 2015 11:48:17 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 31 Mar 2015 10:45:15 +0200

x86/vdso32/syscall.S: Do not load __USER32_DS to %ss

This vDSO code only gets used by 64-bit kernels, not 32-bit ones.

On 64-bit kernels, the data segment is the same for 32-bit and 64-bit userspace, and the SYSRET instruction loads %ss with its selector.

So there's no need to repeat it by hand. Segment loads are somewhat
expensive: tens of cycles.

Signed-off-by Denys Vlasenko <[email protected]>

[ Removed unnecessary comment. ]

Signed-off-by Andy Lutomirski <[email protected]>

Cc Alexei Starovoitov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Kees Cook <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Oleg Nesterov <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Will Drewry <[email protected]>

Link http://lkml.kernel.org/r/63da6d778f69fd0f1345d9287f6764d58be519fa.1427482099.git.luto@kernel.org open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/vdso/vdso32/syscall.S | 2 --
 1 file changed, 2 deletions(-)

arch/x86/vdso/vdso32/syscall.S +0 -0

index 5415b56..6b286bb 100644

arch/x86/vdso/vdso32/syscall.S __kernel_vsyscall: · line 24

Quoted 8 lines

arch/x86/vdso/vdso32/syscall.S +0 -2

@@ -19,8 +19,6 @@ __kernel_vsyscall:
 .Lpush_ebp:
        movl    %ecx, %ebp
        syscall
-       movl    $__USER32_DS, %ecx
-       movl    %ecx, %ss
        movl    %ebp, %ecx
        popl    %ebp
 .Lpop_ebp:

This patch unfortunately is causing Wine to break on some applications:

Unhandled exception: stack overflow in 32-bit code (0xf779bc07). Register dump:
CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b
EIP:f779bc07 ESP:00aed60c EBP:00aed750 EFLAGS:00010216( R- -- I -A-P- )
EAX:00000040 EBX:00000010 ECX:00aed750 EDX:00000040 ESI:00000040 EDI:7ffd4000
Stack dump:

0x00aed60c:  00aed648 f7575e5b 7bcc8000 00000000
0x00aed61c:  7bc7bc09 00000010 00aed750 00000040
0x00aed62c:  00aed750 00aed650 7bcc8000 7bc7bbdd
0x00aed63c:  7bcc8000 00aed6a0 00aed750 00aed738
0x00aed64c:  7bc7cfa9 00000011 00aed750 00000040
0x00aed65c:  00000020 00000000 00000000 7bc4f141
Backtrace:
=>0 0xf779bc07 __kernel_vsyscall+0x7() in [vdso].so (0x00aed750)
  1 0xf7575e5b __libc_read+0x4a() in libpthread.so.0 (0x00aed648)
  2 0x7bc7bc09 read_reply_data+0x38(buffer=0xaed750, size=0x40)

[/home/bgerst/src/wine/wine32/dlls/ntdll/../../../dlls/ntdll/server.c:239] in ntdll (0x00aed648)
3 0x7bc7cfa9 wine_server_call+0x178() in ntdll (0x00aed738)
4 0x7bc840ec NtSetEvent+0x4b(handle=0x80,
NumberOfThreadsReleased=0x0(nil)) [/home/bgerst/src/wine/wine32/dlls/ntdll/../../../dlls/ntdll/sync.c:361] in ntdll (0x00aed7c8)
5 0x7b874afa SetEvent+0x24(handle=<couldn't compute location>)
[/home/bgerst/src/wine/wine32/dlls/kernel32/../../../dlls/kernel32/sync.c:572] in kernel32 (0x00aed7e8)
6 0x0044e31a in battle.net launcher (+0x4e319) (0x00aed818)
...

Quoted 8 lines

__kernel_vsyscall+0x7 points to "pop %ebp".

This is on an AMD Phenom(tm) II X6 1055T Processor.

It appears that there are some subtle differences in how sysretl works on AMD vs. Intel. According to the Intel docs, the SS selector and descriptor cache is completely reset by sysret to fixed values. The AMD docs however are concerning:

My understanding is that, in long mode, the segment attributes are ignored, and that there is no such thing as a "64-bit stack". So...

Quoted 4 lines

AMD's syscall:
SS.sel = MSR_STAR.SYSCALL_CS + 8
SS.attr = 64-bit stack,dpl0

I don't really believe that.

Quoted 7 lines

SS.base = 0x00000000
SS.limit = 0xFFFFFFFF

AMD's sysret:
SS.sel = MSR_STAR.SYSRET_CS + 8 // SS selector is changed,

                                // SS base, limit, attributes unchanged.

I'm pretty sure that this is at least a little bit wrong. It makes no sense for me for syscall to set SS.DPL=0 and for sysret to leave SS.DPL=0. It had better at least change DPL to 3. (Except... don't
they mean RPL? Why is the DPL cached at all? But RPL is clearly
changed, since it's part of the selector.)

Quoted 3 lines

Not changing base or limit is no big deal, but not changing attributes could be the problem. It might be leaving the "64-bit stack" attribute set, for whatever that means.

Hmm. I don't know if I believe that explanation. For one thing, the APM says "Executing SYSRET in non-64-bit mode or with a 16- or 32-bit operand size returns to 32-bit mode with a 32-bit stack pointer."

We can revert this patch or fix it, but I'd like to at least try to understand what's wrong first. Borislav, any ideas?

I'm curious whether we can somehow end up in the kernel without a sensible SS. What happens if we have SS = 0?

Try this on for size:

1. Wine process does syscall
2. Context switch to any other task
3. Interrupt (software or hardware), which loads SS with ss0, which is 0 on x86_64.

4. Context switch back to Wine.
5. sysretl

Quoted 3 lines · Denys Vlasenko

The fix can look like this (untested):

arch/x86/ia32/ia32entry.S sysexit_from_sys_call: · line 202

Quoted 10 lines

arch/x86/ia32/ia32entry.S +2 -0

index 0c302d0..9f4c232 100644
@@ -198,6 +198,18 @@ sysexit_from_sys_call:
 	 * with 'sysenter' and it uses the SYSENTER calling convention.
 	 */
 	andl    $~TS_COMPAT,ASM_THREAD_INFO(TI_status, %rsp, SIZEOF_PTREGS)
+	/*
+	 * On AMD, SYSRET32 does not modify %ss cached descriptor;

Ok, but doc says that in both long and compat mode, SYSRET does load SS.sel with the value in MSR_STAR...

Yes. It loads *selector*. AMD docs say that selector is loaded as you say, but *cached descriptor* of SS (which is a different entity) is not modified.

If *cached descriptor* is invalid, in 32-bit mode stack ops will fail. (In 64-bit mode, CPU doesn't do those checks).

Quoted 5 lines · Denys Vlasenko

Yes. It loads *selector*. AMD docs say that selector is loaded as you say, but *cached descriptor* of SS (which is a different entity) is not modified.

If *cached descriptor* is invalid, in 32-bit mode stack ops will fail. (In 64-bit mode, CPU doesn't do those checks).

So how can that happen with wine? Something's changing the cached descriptor ... ?

Quoted 5 lines · Denys Vlasenko

Yes. It loads *selector*. AMD docs say that selector is loaded as you say, but *cached descriptor* of SS (which is a different entity) is not modified.

If *cached descriptor* is invalid, in 32-bit mode stack ops will fail. (In 64-bit mode, CPU doesn't do those checks).

So how can that happen with wine? Something's changing the cached descriptor and only the write to %ss reloads it with the correct value?

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

An alternative fix would be, if we decided to schedule in an interrupt, check %ss for zero and reload it with __KERNEL_DS before schedule.

Quoted 3 lines · Denys Vlasenko

An alternative fix would be, if we decided to schedule in an interrupt, check %ss for zero and reload it with __KERNEL_DS before schedule.

For anyone who has the right hardware (not me!), a possible reproducer is here:

https://git.kernel.org/cgit/linux/kernel/git/luto/misc-tests.git/

make && taskset -c 0 ./sysret_ss_attrs_32

Quoted 8 lines · Andy Lutomirski

Quoted 3 lines · Denys Vlasenko

An alternative fix would be, if we decided to schedule in an interrupt, check %ss for zero and reload it with __KERNEL_DS before schedule.

For anyone who has the right hardware (not me!), a possible reproducer is here:

Quoted 3 lines

https://git.kernel.org/cgit/linux/kernel/git/luto/misc-tests.git/

make && taskset -c 0 ./sysret_ss_attrs_32

[  195.438441] traps: sysret_ss_attrs[1745] trap stack segment ip:f77dab87 sp:ffdf0b70 error:0
[  196.831952] traps: sysret_ss_attrs[1748] trap stack segment ip:f7786b87 sp:fffc0810 error:0

Ran it twice.

1. Do we consider this to be enough of a security issue that we want to fix it for 64-bit userspace as well?

2. Do we fix it at sysret time (at the cost of an ss read even in the best case on AMD chips) or at context switch time (with the risk of more ss writes than necessary)?

That nails it. We really do leak segment limits to other tasks on AMD chips. I see at least two questions we should answer before fixing this:

1. Do we consider this to be enough of a security issue that we want to fix it for 64-bit userspace as well?

2. Do we fix it at sysret time (at the cost of an ss read even in the best case on AMD chips) or at context switch time (with the risk of more ss writes than necessary)?

I slightly favor fixing it at sysret time for both the 32-bit and 64-bit paths., but I'm not really convinced.

Quoted 3 lines · Andy Lutomirski

That nails it. We really do leak segment limits to other tasks on AMD chips. I see at least two questions we should answer before fixing this:

Ok, WTF is going on?! Even this trivial test case causes a Bus Error:

---
static unsigned short GDT3(int idx)

{
        return (idx << 3) | 3;
}

static void *threadproc(void *ctx)

{
        printf("Hello world\n");
        return NULL;
}

int main()

{
        pthread_t thread;
        if (pthread_create(&thread, 0, threadproc, 0) != 0)
                err(1, "pthread_create");

        while (1) {
                usleep(1);
        }

        return 0;
}

---

$ make sysret_ss_attrs_32
gcc -m32 -o sysret_ss_attrs_32 -O2 -g -std=gnu99 -pthread -Wall  sysret_ss_attrs.c -lrt -ldl
sysret_ss_attrs.c:23:23: warning: ‘GDT3’ defined but not used [-Wunused-function]
 static unsigned short GDT3(int idx)
                       ^
$ taskset -c 0 ./sysret_ss_attrs_32
Hello world
Bus error

in dmesg:

[  583.389368] traps: sysret_ss_attrs[2135] trap stack segment ip:f7784b87 sp:ffb640c0 error:0

This is insane.

Quoted 7 lines

1. Do we consider this to be enough of a security issue that we want to fix it for 64-bit userspace as well?

2. Do we fix it at sysret time (at the cost of an ss read even in the best case on AMD chips) or at context switch time (with the risk of more ss writes than necessary)?

Quoted 2 lines

I slightly favor fixing it at sysret time for both the 32-bit and 64-bit paths., but I'm not really convinced.

Yeah, a "call amd_fixup_ss" which gets NOPped out on Intel with alternatives sounds nice and clean to me.

Pending we have an explanation WTH is going on...

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

Quoted 3 lines · Andy Lutomirski

That nails it. We really do leak segment limits to other tasks on AMD chips. I see at least two questions we should answer before fixing this:

Ok, WTF is going on?! Even this trivial test case causes a Bus Error:

---
static unsigned short GDT3(int idx)

{
        return (idx << 3) | 3;
}

static void *threadproc(void *ctx)

{
        printf("Hello world\n");
        return NULL;
}

int main()

{
        pthread_t thread;
        if (pthread_create(&thread, 0, threadproc, 0) != 0)
                err(1, "pthread_create");

        while (1) {
                usleep(1);
        }

        return 0;
}

---

$ make sysret_ss_attrs_32
gcc -m32 -o sysret_ss_attrs_32 -O2 -g -std=gnu99 -pthread -Wall  sysret_ss_attrs.c -lrt -ldl
sysret_ss_attrs.c:23:23: warning: ‘GDT3’ defined but not used [-Wunused-function]
 static unsigned short GDT3(int idx)
                       ^
$ taskset -c 0 ./sysret_ss_attrs_32
Hello world
Bus error

in dmesg:

[  583.389368] traps: sysret_ss_attrs[2135] trap stack segment ip:f7784b87 sp:ffb640c0 error:0

Quoted 42 lines · tip-bot for Denys Vlasenko

Commit-ID:  e7d6eefaaa443130079d73cd05039d90b3db7a4a
Gitweb:     http://git.kernel.org/tip/e7d6eefaaa443130079d73cd05039d90b3db7a4a
Author:     Denys Vlasenko <[email protected]>
AuthorDate: Fri, 27 Mar 2015 11:48:17 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 31 Mar 2015 10:45:15 +0200

x86/vdso32/syscall.S: Do not load __USER32_DS to %ss

This vDSO code only gets used by 64-bit kernels, not 32-bit ones.

On 64-bit kernels, the data segment is the same for 32-bit and 64-bit userspace, and the SYSRET instruction loads %ss with its selector.

So there's no need to repeat it by hand. Segment loads are somewhat
expensive: tens of cycles.

Signed-off-by Denys Vlasenko <[email protected]>

[ Removed unnecessary comment. ]

Signed-off-by Andy Lutomirski <[email protected]>

Cc Alexei Starovoitov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Kees Cook <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Oleg Nesterov <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Will Drewry <[email protected]>

Link http://lkml.kernel.org/r/63da6d778f69fd0f1345d9287f6764d58be519fa.1427482099.git.luto@kernel.org open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/vdso/vdso32/syscall.S | 2 --
 1 file changed, 2 deletions(-)

arch/x86/vdso/vdso32/syscall.S +0 -0

index 5415b56..6b286bb 100644

arch/x86/vdso/vdso32/syscall.S __kernel_vsyscall: · line 24

Quoted 8 lines

arch/x86/vdso/vdso32/syscall.S +0 -2

@@ -19,8 +19,6 @@ __kernel_vsyscall:
 .Lpush_ebp:
        movl    %ecx, %ebp
        syscall
-       movl    $__USER32_DS, %ecx
-       movl    %ecx, %ss
        movl    %ebp, %ecx
        popl    %ebp
 .Lpop_ebp:

This patch unfortunately is causing Wine to break on some applications:

Unhandled exception: stack overflow in 32-bit code (0xf779bc07). Register dump:
CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b
EIP:f779bc07 ESP:00aed60c EBP:00aed750 EFLAGS:00010216( R- -- I -A-P- )
EAX:00000040 EBX:00000010 ECX:00aed750 EDX:00000040 ESI:00000040 EDI:7ffd4000
Stack dump:

0x00aed60c:  00aed648 f7575e5b 7bcc8000 00000000
0x00aed61c:  7bc7bc09 00000010 00aed750 00000040
0x00aed62c:  00aed750 00aed650 7bcc8000 7bc7bbdd
0x00aed63c:  7bcc8000 00aed6a0 00aed750 00aed738
0x00aed64c:  7bc7cfa9 00000011 00aed750 00000040
0x00aed65c:  00000020 00000000 00000000 7bc4f141
Backtrace:
=>0 0xf779bc07 __kernel_vsyscall+0x7() in [vdso].so (0x00aed750)
  1 0xf7575e5b __libc_read+0x4a() in libpthread.so.0 (0x00aed648)
  2 0x7bc7bc09 read_reply_data+0x38(buffer=0xaed750, size=0x40)

[/home/bgerst/src/wine/wine32/dlls/ntdll/../../../dlls/ntdll/server.c:239] in ntdll (0x00aed648)
3 0x7bc7cfa9 wine_server_call+0x178() in ntdll (0x00aed738)
4 0x7bc840ec NtSetEvent+0x4b(handle=0x80,
NumberOfThreadsReleased=0x0(nil)) [/home/bgerst/src/wine/wine32/dlls/ntdll/../../../dlls/ntdll/sync.c:361] in ntdll (0x00aed7c8)
5 0x7b874afa SetEvent+0x24(handle=<couldn't compute location>)
[/home/bgerst/src/wine/wine32/dlls/kernel32/../../../dlls/kernel32/sync.c:572] in kernel32 (0x00aed7e8)
6 0x0044e31a in battle.net launcher (+0x4e319) (0x00aed818)
...

__kernel_vsyscall+0x7 points to "pop %ebp".

This is on an AMD Phenom(tm) II X6 1055T Processor.

It appears that there are some subtle differences in how sysretl works on AMD vs. Intel. According to the Intel docs, the SS selector and descriptor cache is completely reset by sysret to fixed values. The AMD docs however are concerning:

AMD's syscall:
SS.sel = MSR_STAR.SYSCALL_CS + 8
SS.attr = 64-bit stack,dpl0
SS.base = 0x00000000
SS.limit = 0xFFFFFFFF

AMD's sysret:
SS.sel = MSR_STAR.SYSRET_CS + 8 // SS selector is changed,

                                // SS base, limit, attributes unchanged.

Not changing base or limit is no big deal, but not changing attributes could be the problem. It might be leaving the "64-bit stack" attribute set, for whatever that means.

Unhandled exception: stack overflow in 32-bit code (0xf779bc07). Register dump:
CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b
EIP:f779bc07 ESP:00aed60c EBP:00aed750 EFLAGS:00010216( R- -- I -A-P- )
EAX:00000040 EBX:00000010 ECX:00aed750 EDX:00000040 ESI:00000040 EDI:7ffd4000

Reloading SS from the GDT would obviously reset any bad state left by sysretl. Unfortunately we may have to put it back in, and then NOP it out on Intel.

* what if %ss before syscall was NOT the usual value of 0x2b, but some other segment, not the typical 0-base, 0xffffffff limit 32-bit expand-up one? Not restoring proper %ss would not go well.
[but then, Intel CPUs work, and old code worked....]

Quoted 4 lines · Denys Vlasenko

* what if %ss before syscall was NOT the usual value of 0x2b, but some other segment, not the typical 0-base, 0xffffffff limit 32-bit expand-up one? Not restoring proper %ss would not go well.
[but then, Intel CPUs work, and old code worked....]

Have we run the exact same reproducer on Intel already?

Brian, can you run the same thing on an Intel box, if you haven't done so already?

Thanks.

Quoted 102 lines · Brian Gerst

Quoted 42 lines · tip-bot for Denys Vlasenko

Commit-ID:  e7d6eefaaa443130079d73cd05039d90b3db7a4a
Gitweb:     http://git.kernel.org/tip/e7d6eefaaa443130079d73cd05039d90b3db7a4a
Author:     Denys Vlasenko <[email protected]>
AuthorDate: Fri, 27 Mar 2015 11:48:17 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 31 Mar 2015 10:45:15 +0200

x86/vdso32/syscall.S: Do not load __USER32_DS to %ss

This vDSO code only gets used by 64-bit kernels, not 32-bit ones.

On 64-bit kernels, the data segment is the same for 32-bit and 64-bit userspace, and the SYSRET instruction loads %ss with its selector.

So there's no need to repeat it by hand. Segment loads are somewhat
expensive: tens of cycles.

Signed-off-by Denys Vlasenko <[email protected]>

[ Removed unnecessary comment. ]

Signed-off-by Andy Lutomirski <[email protected]>

Cc Alexei Starovoitov <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Kees Cook <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Oleg Nesterov <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Will Drewry <[email protected]>

Link http://lkml.kernel.org/r/63da6d778f69fd0f1345d9287f6764d58be519fa.1427482099.git.luto@kernel.org open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/vdso/vdso32/syscall.S | 2 --
 1 file changed, 2 deletions(-)

arch/x86/vdso/vdso32/syscall.S +0 -0

index 5415b56..6b286bb 100644

arch/x86/vdso/vdso32/syscall.S __kernel_vsyscall: · line 24

Quoted 8 lines

arch/x86/vdso/vdso32/syscall.S +0 -2

@@ -19,8 +19,6 @@ __kernel_vsyscall:
 .Lpush_ebp:
        movl    %ecx, %ebp
        syscall
-       movl    $__USER32_DS, %ecx
-       movl    %ecx, %ss
        movl    %ebp, %ecx
        popl    %ebp
 .Lpop_ebp:

This patch unfortunately is causing Wine to break on some applications:

Unhandled exception: stack overflow in 32-bit code (0xf779bc07). Register dump:
CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b
EIP:f779bc07 ESP:00aed60c EBP:00aed750 EFLAGS:00010216( R- -- I -A-P- )
EAX:00000040 EBX:00000010 ECX:00aed750 EDX:00000040 ESI:00000040 EDI:7ffd4000
Stack dump:

0x00aed60c:  00aed648 f7575e5b 7bcc8000 00000000
0x00aed61c:  7bc7bc09 00000010 00aed750 00000040
0x00aed62c:  00aed750 00aed650 7bcc8000 7bc7bbdd
0x00aed63c:  7bcc8000 00aed6a0 00aed750 00aed738
0x00aed64c:  7bc7cfa9 00000011 00aed750 00000040
0x00aed65c:  00000020 00000000 00000000 7bc4f141
Backtrace:
=>0 0xf779bc07 __kernel_vsyscall+0x7() in [vdso].so (0x00aed750)
  1 0xf7575e5b __libc_read+0x4a() in libpthread.so.0 (0x00aed648)
  2 0x7bc7bc09 read_reply_data+0x38(buffer=0xaed750, size=0x40)

[/home/bgerst/src/wine/wine32/dlls/ntdll/../../../dlls/ntdll/server.c:239] in ntdll (0x00aed648)
3 0x7bc7cfa9 wine_server_call+0x178() in ntdll (0x00aed738)
4 0x7bc840ec NtSetEvent+0x4b(handle=0x80,
NumberOfThreadsReleased=0x0(nil)) [/home/bgerst/src/wine/wine32/dlls/ntdll/../../../dlls/ntdll/sync.c:361] in ntdll (0x00aed7c8)
5 0x7b874afa SetEvent+0x24(handle=<couldn't compute location>)
[/home/bgerst/src/wine/wine32/dlls/kernel32/../../../dlls/kernel32/sync.c:572] in kernel32 (0x00aed7e8)
6 0x0044e31a in battle.net launcher (+0x4e319) (0x00aed818)
...

__kernel_vsyscall+0x7 points to "pop %ebp".

This is on an AMD Phenom(tm) II X6 1055T Processor.

It appears that there are some subtle differences in how sysretl works on AMD vs. Intel. According to the Intel docs, the SS selector and descriptor cache is completely reset by sysret to fixed values. The AMD docs however are concerning:

AMD's syscall:
SS.sel = MSR_STAR.SYSCALL_CS + 8
SS.attr = 64-bit stack,dpl0
SS.base = 0x00000000
SS.limit = 0xFFFFFFFF

AMD's sysret:
SS.sel = MSR_STAR.SYSRET_CS + 8 // SS selector is changed,

Quoted 7 lines

                                // SS base, limit, attributes unchanged.

Not changing base or limit is no big deal, but not changing attributes could be the problem. It might be leaving the "64-bit stack" attribute set, for whatever that means.

I am not aware of any officially existing "64-bit" stack or data segment attribute. x86 data segment descriptors don't have any such bits, the 64-bitness of stack operations in long mode is hardwired. (Unlike code segment descriptors, which _do_ have a bit which controls 64-bitness).

This is not to say that CPU internally is prohibited from having something along those lines.

However, if AMD CPUs would have a bug where after sysretl %ss descriptor cache is left in a bad state causing stack ops to be done in 64-bit fashion, *any* 32-bit userspace would immediately explode. This is not the case.

What Wine could do differently from a typical Linux executable? It may use nonzero %ss base, it may use a non-4Gb limit,
it may use 16-bit stack segment, it may use an expand-down stack segment. (I know very little about Windows/Wine internals, so I just listed all possibilities which came to mind).

Looking at the error message:

Quoted 6 lines

Unhandled exception: stack overflow in 32-bit code (0xf779bc07). Register dump:
CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b
EIP:f779bc07 ESP:00aed60c EBP:00aed750 EFLAGS:00010216( R- -- I -A-P- )
EAX:00000040 EBX:00000010 ECX:00aed750 EDX:00000040 ESI:00000040 EDI:7ffd4000

it is not coming from Wine itself, looks like it's from Windows code, and I'd guess it just tells us that they got exception 12, without further information on the cause.

Quoted 9 lines

Looking at the error message:

Quoted 6 lines

Unhandled exception: stack overflow in 32-bit code (0xf779bc07). Register dump:
CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b
EIP:f779bc07 ESP:00aed60c EBP:00aed750 EFLAGS:00010216( R- -- I -A-P- )
EAX:00000040 EBX:00000010 ECX:00aed750 EDX:00000040 ESI:00000040 EDI:7ffd4000

Quoted 3 lines

it is not coming from Wine itself, looks like it's from Windows code, and I'd guess it just tells us that they got exception 12, without further information on the cause.

The backtrace shows the fault is in the VDSO, the first pop instruction after returning from the kernel.

Quoted 9 lines · Brian Gerst

Quoted 9 lines

Looking at the error message:

Quoted 6 lines

Unhandled exception: stack overflow in 32-bit code (0xf779bc07). Register dump:
CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b
EIP:f779bc07 ESP:00aed60c EBP:00aed750 EFLAGS:00010216( R- -- I -A-P- )
EAX:00000040 EBX:00000010 ECX:00aed750 EDX:00000040 ESI:00000040 EDI:7ffd4000

Quoted 6 lines

Quoted 3 lines

it is not coming from Wine itself, looks like it's from Windows code, and I'd guess it just tells us that they got exception 12, without further information on the cause.

The backtrace shows the fault is in the VDSO, the first pop instruction after returning from the kernel.

Yes, I understand at which insn exception happens.

I meant that *the message* is not generated by Wine or kernel. grep for "Unhandled exception:" comes up empty
on their source trees.

After much grepping, I see that I'm wrong.
It does come from wine:

void info_win32_exception(void)

{
    const EXCEPTION_RECORD*     rec;
    ADDRESS64                   addr;
    char                        hexbuf[MAX_OFFSET_TO_STR_LEN];

    if (!dbg_curr_thread->in_exception)
    {
        dbg_printf("Thread isn't in an exception\n");
        return;
    }
    rec = &dbg_curr_thread->excpt_record;
    memory_get_current_pc(&addr);

    /* print some infos */
    dbg_printf("%s: ",
               dbg_curr_thread->first_chance ? "First chance exception" : "Unhandled exception");
    switch (rec->ExceptionCode)
    {
    case EXCEPTION_BREAKPOINT:
        dbg_printf("breakpoint");
        break;
    case EXCEPTION_SINGLE_STEP:
        dbg_printf("single step");
        break;
    case EXCEPTION_INT_DIVIDE_BY_ZERO:
        dbg_printf("divide by zero");
        break;
    case EXCEPTION_INT_OVERFLOW:
        dbg_printf("overflow");
        break;
    case EXCEPTION_ARRAY_BOUNDS_EXCEEDED:
        dbg_printf("array bounds");
        break;
    case EXCEPTION_ILLEGAL_INSTRUCTION:
        dbg_printf("illegal instruction");
        break;
    case EXCEPTION_STACK_OVERFLOW:
        dbg_printf("stack overflow");
        break;
    ...

I hoped we can easily make Wine show exception's error code. Not that easy :/

The patch does appear to fix the crash.

This patch unfortunately is causing Wine to break on some applications:

Unhandled exception: stack overflow in 32-bit code (0xf779bc07). Register dump:
CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b
EIP:f779bc07 ESP:00aed60c EBP:00aed750 EFLAGS:00010216( R- -- I -A-P- )
EAX:00000040 EBX:00000010 ECX:00aed750 EDX:00000040 ESI:00000040 EDI:7ffd4000
Stack dump:

0x00aed60c:  00aed648 f7575e5b 7bcc8000 00000000
0x00aed61c:  7bc7bc09 00000010 00aed750 00000040
0x00aed62c:  00aed750 00aed650 7bcc8000 7bc7bbdd
0x00aed63c:  7bcc8000 00aed6a0 00aed750 00aed738
0x00aed64c:  7bc7cfa9 00000011 00aed750 00000040
0x00aed65c:  00000020 00000000 00000000 7bc4f141
Backtrace:
=>0 0xf779bc07 __kernel_vsyscall+0x7() in [vdso].so (0x00aed750)
  1 0xf7575e5b __libc_read+0x4a() in libpthread.so.0 (0x00aed648)
  2 0x7bc7bc09 read_reply_data+0x38(buffer=0xaed750, size=0x40)

[/home/bgerst/src/wine/wine32/dlls/ntdll/../../../dlls/ntdll/server.c:239] in ntdll (0x00aed648)
3 0x7bc7cfa9 wine_server_call+0x178() in ntdll (0x00aed738)
4 0x7bc840ec NtSetEvent+0x4b(handle=0x80,
NumberOfThreadsReleased=0x0(nil)) [/home/bgerst/src/wine/wine32/dlls/ntdll/../../../dlls/ntdll/sync.c:361] in ntdll (0x00aed7c8)
5 0x7b874afa SetEvent+0x24(handle=<couldn't compute location>)
[/home/bgerst/src/wine/wine32/dlls/kernel32/../../../dlls/kernel32/sync.c:572] in kernel32 (0x00aed7e8)
6 0x0044e31a in battle.net launcher (+0x4e319) (0x00aed818)
...

__kernel_vsyscall+0x7 points to "pop %ebp".

This is on an AMD Phenom(tm) II X6 1055T Processor.

It appears that there are some subtle differences in how sysretl works on AMD vs. Intel. According to the Intel docs, the SS selector and descriptor cache is completely reset by sysret to fixed values. The AMD docs however are concerning:

AMD's syscall:
SS.sel = MSR_STAR.SYSCALL_CS + 8
SS.attr = 64-bit stack,dpl0
SS.base = 0x00000000
SS.limit = 0xFFFFFFFF

AMD's sysret:
SS.sel = MSR_STAR.SYSRET_CS + 8 // SS selector is changed,

                                // SS base, limit, attributes unchanged.

Not changing base or limit is no big deal, but not changing attributes could be the problem. It might be leaving the "64-bit stack" attribute set, for whatever that means.

Reloading SS from the GDT would obviously reset any bad state left by sysretl. Unfortunately we may have to put it back in, and then NOP it out on Intel.

From 4b9e9bdf08b092724934e62892116af6dd712128 Mon Sep 17 00:00:00 2001