Re: vm86 segfaults with NX bit

[Egbert Eich <eich@suse.de>]

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 1614 bytes --]

Takashi Iwai writes:
 > At Thu, 20 Nov 2008 08:58:44 -0800,
 > Arjan van de Ven wrote:
 > > 
 > > On Thu, 20 Nov 2008 16:39:06 +0100
 > > Takashi Iwai <tiwai@suse.de> wrote:
 > > 
 > > > Hi,
 > > > 
 > > > X guys reported that VESA driver segfaults in vm86 mode with a kernel
 > > > with CONFIG_X86_PAE, and it turned to be the NX bit.  See Novell
 > > > bugzilla #443440:
 > > > 	https://bugzilla.novell.com/show_bug.cgi?id=443440
 > > > 
 > > > I can confirm that the latest 2.6.28-rc still causes the same problem,
 > > > and it disappears when booted with noexec=off option.
 > > > 
 > > > Also, when NX bit is reset for the first 1MB (or smaller) in
 > > > do_sys_vm86(), it also works as expected.  But I have little clue
 > > > where to restore the bit again.
 > > > 
 > > > Any good suggestions / solutions?
 > > > 
 > > 
 > > did the code that mapped this memory setup use PROT_EXEC ?
 > 
 > A good question...
 > Luc, Egbert, how does VESA driver handle it?


Yes, we set the PROT_EXEC for the VBIOS. But the segfault doesn't happen
in VBIOS as far as I can tell. It happens on the 'exit instruction', a
hlt (0xf6) which is trapped by vm86 so it returns to the caller.
This is done by putting 0xf6 into address 0x600 and putting this address
on the stack.
The memory that's mapped at 0x600 in the Xserver's address space is
allocated thru shmget() and attached to the right place in the processes
memory space for vm86.
We should call mprotect() on these ranges to set this memory executable.
Hey, this code was orginally written almost 10 years ago!

The patch below fixes the issue.

Cheers,
	Egbert.


[-- Attachment #2: diff.vm86_exec --]
[-- Type: text/plain, Size: 1414 bytes --]

diff --git a/hw/xfree86/os-support/linux/int10/linux.c b/hw/xfree86/os-support/linux/int10/linux.c
index 67eb161..b15f7fd 100644
--- a/hw/xfree86/os-support/linux/int10/linux.c
+++ b/hw/xfree86/os-support/linux/int10/linux.c
@@ -1,6 +1,6 @@
 /*
  * linux specific part of the int10 module
- * Copyright 1999, 2000, 2001, 2002, 2003, 2004 Egbert Eich
+ * Copyright 1999, 2000, 2001, 2002, 2003, 2004, 2008 Egbert Eich
  */
 #ifdef HAVE_XORG_CONFIG_H
 #include <xorg-config.h>
@@ -357,7 +357,10 @@ MapCurrentInt10(xf86Int10InfoPtr pInt)
 		   "shmat(low_mem) error: %s\n",strerror(errno));
 	return FALSE;
     }
-    
+    if (mprotect((void*)0, V_RAM, PROT_READ|PROT_WRITE|PROT_EXEC) != 0)
+        xf86DrvMsg(pInt->scrnIndex, X_ERROR,
+		   "Cannot set EXEC bit on low memory: %s\n", strerror(errno));
+
     if (((linuxInt10Priv*)pInt->private)->highMem >= 0) {
 	addr = shmat(((linuxInt10Priv*)pInt->private)->highMem,
 		     (char*)HIGH_MEM, 0);
@@ -368,6 +371,11 @@ MapCurrentInt10(xf86Int10InfoPtr pInt)
 		       "shmget error: %s\n",strerror(errno));
 	    return FALSE;
 	}
+	if (mprotect((void*)HIGH_MEM, HIGH_MEM_SIZE,
+		     PROT_READ|PROT_WRITE|PROT_EXEC) != 0)
+	    xf86DrvMsg(pInt->scrnIndex, X_ERROR,
+		       "Cannot set EXEC bit on high memory: %s\n",
+		       strerror(errno));
     } else {
 	if ((fd = open(DEV_MEM, O_RDWR, 0)) >= 0) {
 	    if (mmap((void *)(V_BIOS), SYS_BIOS - V_BIOS,