lkml.org 
[lkml]   [2009]   [May]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] i386: fix return to 16-bit stack from NMI handler
[Alexander van Heukelum - Sun, May 24, 2009 at 08:24:09PM +0200]
> The nmi handler changes esp on return from the kernel to a process
> with a 16-bit stack. To reproduce, compile and run the following
> program with the nmi watchdog enabled (nmi_watchdog=2 on the command
> line). Using gdb you can see that the high bits of esp contain garbage,
> while the low bits are still correct. This is what the 'espfix' code is
> supposed to fix, but the nmi handler does not include it.
>
> The nmi code cannot call the irqtrace infrastructure. Otherwise, the tail
> of the normal iret-code is correct for the nmi code path too. To be
> able to share this code-path, I moved the TRACE_IRQS_IRET a bit earlier.
> This code-path now includes the espfix code, which explicitly disables
> interrupts. This short interrupts-off section is now not traced anymore.
> The preempt-return-to-kernel path now includes the preliminary test to
> decide if the espfix code should be called. This is never the case, but
> doing it this way keeps the patch simple and the few extra instructions
> should not affect timing in any significant way.
>
> #define _GNU_SOURCE
> #include <stdio.h>
> #include <sys/types.h>
> #include <sys/mman.h>
> #include <unistd.h>
> #include <sys/syscall.h>
> #include <asm/ldt.h>
>
> int modify_ldt(int func, void *ptr, unsigned long bytecount)
> {
> return syscall(SYS_modify_ldt, func, ptr, bytecount);
> }
>
> /* this is assumed to be usable */
> #define SEGBASEADDR 0x10000
> #define SEGLIMIT 0xffff
>
> /* 16-bit segment */
> struct user_desc desc = {
> .entry_number = 0,
> .base_addr = SEGBASEADDR,
> .limit = SEGLIMIT,
> .seg_32bit = 0,
> .contents = 0, /* ??? */
> .read_exec_only = 0,
> .limit_in_pages = 0,
> .seg_not_present = 0,
> .useable = 1
> };
>
> int main(void)
> {
> setvbuf(stdout, NULL, _IONBF, 0);
>
> /* map a 64 kb segment */
> char *pointer = mmap((void *)SEGBASEADDR, SEGLIMIT+1,
> PROT_EXEC|PROT_READ|PROT_WRITE,
> MAP_SHARED|MAP_ANONYMOUS, -1, 0);
> if (pointer == NULL) {
> printf("could not map space\n");
> return 0;
> }
>
> /* write ldt, new mode */
> int err = modify_ldt(0x11, &desc, sizeof(desc));
> if (err) {
> printf("error modifying ldt: %i\n", err);
> return 0;
> }
>
> for (int i=0; i<1000; i++) {
> asm volatile (
> "pusha\n\t"
> "mov %ss, %eax\n\t" /* preserve ss:esp */
> "mov %esp, %ebp\n\t"
> "push $7\n\t" /* index 0, ldt, user mode */
> "push $61440\n\t" /* esp */
> "lss (%esp), %esp\n\t" /* switch to new stack */
> "push %eax\n\t" /* save old ss:esp on new stack */
> "push %ebp\n\t"
>
> "mov %esp, %edx\n\t"
>
> /* wait a bit */
> "mov $10000000, %ecx\n\t"
> "1: loop 1b\n\t"
>
> "cmp %esp, %edx\n\t"
> "je 1f\n\t"
> "ud2\n\t" /* esp changed inexplicably! */
> "1:\n\t"
> "lss (%esp), %esp\n\t" /* restore old ss:esp */
> "popa\n\t");
>
> printf("\rx%ix", i);
> }
>
> return 0;
> }
>
> The bug is present in 2.6.28, and probably even much earlier.
>
> Signed-off-by: Alexander van Heukelum <heukelum@fastmail.fm>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Cyrill Gorcunov <gorcunov@gmail.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: The stable team <stable@kernel.org>
>
> ---
>
> Hi Ingo, Peter, Cyrill, ...
>
> Mucking with entry_32.S and trying to exercise all code paths, I found
> the problem described above. Please check this patch carefully for side
> effects I may have overlooked!
>
> Greetings,
> Alexander
>

Really good catch, Alexander!

At least at moment I can't imagine more simple/cleaner solution.
I've added Andi and Thomas to CC list as well. Just to be sure...

-- Cyrill


\
 
 \ /
  Last update: 2009-05-26 16:17    [W:0.051 / U:0.268 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site