From e85d3d374594f6c31e1bf72f8912334194be8654 Mon Sep 17 00:00:00 2001
From: Bill Paul <wpaul@windriver.com>
Date: Wed, 4 Mar 2015 09:19:03 -0800
Subject: [PATCH] This checkin corrects a bug in the implementation of the
sysret instruction. Per the Intel architecture manual, the
stack selector (SS) is to be loaded from the IA32_STAR
register, incremented by 8, _AND_ ORed with 3 (this forces
the privilege level to 3). The latter step (ORing in the 3)
is missing. This breaks the machine behavior and can lead
to correct code malfunctioning in some circumstances.
Signed-off-by: Bill Paul <wpaul@windriver.com>
---
target-i386/seg_helper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
@@ -1043,7 +1043,7 @@ void helper_sysret(CPUX86State *env, int dflag)
DESC_CS_MASK | DESC_R_MASK | DESC_A_MASK);
env->eip = (uint32_t)env->regs[R_ECX];
}
- cpu_x86_load_seg_cache(env, R_SS, selector + 8,
+ cpu_x86_load_seg_cache(env, R_SS, (selector + 8) | 3,
0, 0xffffffff,
DESC_G_MASK | DESC_B_MASK | DESC_P_MASK |
DESC_S_MASK | (3 << DESC_DPL_SHIFT) |
@@ -1056,7 +1056,7 @@ void helper_sysret(CPUX86State *env, int dflag)
DESC_S_MASK | (3 << DESC_DPL_SHIFT) |
DESC_CS_MASK | DESC_R_MASK | DESC_A_MASK);
env->eip = (uint32_t)env->regs[R_ECX];
- cpu_x86_load_seg_cache(env, R_SS, selector + 8,
+ cpu_x86_load_seg_cache(env, R_SS, (selector + 8) | 3,
0, 0xffffffff,
DESC_G_MASK | DESC_B_MASK | DESC_P_MASK |
DESC_S_MASK | (3 << DESC_DPL_SHIFT) |
--
1.8.0
(I tried sending this once earlier but it seems to have disappeared into the void. Apologies if it ends up showing up twice.) Hi guys. I seem to have found a bug in the helper_systet() function in target-i386/seg_helper.c. I downloaded the Intel architecture manual from here: http://www.intel.com/content/www/us/en/processors/architectures-software- developer-manuals.html And it describes the behavior of SYSRET with regards to the stack selector (SS) as follows: SS.Selector <-- (IA32_STAR[63:48]+8) OR 3; (* RPL forced to 3 *) In other words, the value of the SS register is supposed to be loaded from bits 63:48 of the IA32_STAR model-specific register (MSR), incremented by 8, _AND_ ORed with 3. ORing in the 3 sets the privilege level to 3 (user). This is done since SYSRET returns to user mode after a system call. The code in helper_sysret() performs the "increment by 8" step, but _NOT_ the "OR with 3" step. Here's another description of the SYSRET instruction which says basically the same thing, though in slightly different format: http://tptp.cc/mirrors/siyobik.info/instruction/SYSRET.html [...] SS(SEL) = IA32_STAR[63:48] + 8; SS(PL) = 0x3; [...] The effect of this bug is that when x86-64 code uses the SYSCALL instruction to enter kernel mode, the SYSRET instruction will return the CPU to user mode with the SS register incorrectly set (the privilege level bits will be 0). In my case, the original SS value for user mode was 0x2B, but after the return from SYSRET, it was changed to 0x28. It took me quite some time to realize this was due to a bug in QEMU rather than my own code. This caused a problem later when the user-mode code was preempted by an interrupt. At the end of the interrupt handling, an IRET instruction was used to exit the interrupt context, and the IRET instruction would generate a general protection fault because it would attempt to validate the stack selector value and notice that it was set for PL 0 (supervisor) instead of PL 3 (user). From my reading, the code is quite clearly in error with respect to the Intel documentation, and fixing the bug in my local sources results in my test code (which runs correctly on real hardware) working correctly in QEMU. It seems this bug has been there for a long time -- I happened to have the sources for QEMU 0.10.5 and the bug is there too (in target-i386/op_helper.c). I'm puzzled as to how it's gone unnoticed for so long, which has me thinking that maybe I'm missing something. I'm pretty sure this is wrong though. I'm attaching a patch for this below. It's a pretty simple fix. Comments/questions welcome. -Bill