[Qemu-devel] [PATCH] fix interaction with noexecstack

[Qemu-devel] [PATCH] fix interaction with noexecstack

[Lennert Buytenhek]

Hi,

qemu-arm doesn't work on Fedora Core 2 by default because it expects to
be able to execute stuff off the heap (code_gen_buffer), but that part of
the heap is not marked PROT_EXEC, and Fedora Core 2 has a patch that sets
the CS segment limit to the highest PROT_EXEC-mapped address in the address
space.

It's a bit of a hacky fix, but it Works For Me(tm) on the default Fedora
2.6.5 kernel.  I haven't yet looked into why qemu-arm was crashing on the
Fedora 2.6.[78] update kernels.


--L


diff -urN qemu-20040804.orig/exec.c qemu-20040804.test/exec.c
--- qemu-20040804.orig/exec.c	2004-07-06 02:00:18.000000000 +0300
+++ qemu-20040804.test/exec.c	2004-09-04 02:49:25.414945456 +0300
@@ -127,6 +127,9 @@
 
 static void page_init(void)
 {
+    unsigned long address;
+    unsigned long length;
+
     /* NOTE: we can always suppose that qemu_host_page_size >=
        TARGET_PAGE_SIZE */
 #ifdef _WIN32
@@ -134,6 +137,17 @@
 #else
     qemu_real_host_page_size = getpagesize();
 #endif
+
+    address = (unsigned long)code_gen_buffer;
+    address &= ~(qemu_real_host_page_size - 1);
+
+    length = (unsigned long)code_gen_buffer + sizeof(code_gen_buffer);
+    length -= address;
+    length += qemu_real_host_page_size - 1;
+    length &= ~(qemu_real_host_page_size - 1);
+
+    mprotect(address, length, PROT_READ | PROT_WRITE | PROT_EXEC);
+
     if (qemu_host_page_size == 0)
         qemu_host_page_size = qemu_real_host_page_size;
     if (qemu_host_page_size < TARGET_PAGE_SIZE)

Re: [Qemu-devel] [PATCH] fix interaction with noexecstack

[Lennert Buytenhek]

On Sat, Sep 04, 2004 at 02:00:17AM +0200, Lennert Buytenhek wrote:

> It's a bit of a hacky fix, but it Works For Me(tm) on the default Fedora
> 2.6.5 kernel.  I haven't yet looked into why qemu-arm was crashing on the
> Fedora 2.6.[78] update kernels.

OK, this patch also appears to fix qemu for the 2.6.[78] Fedora
kernels.  Which means I can use qemu-arm again, whee.  Please apply
this patch or suggest an alternative approach.  (Should we just be
mmap()ping the entire code_gen_buffer instead of putting it on the
heap and then mprotect()ing it?)


--L

Re: [Qemu-devel] [PATCH] fix interaction with noexecstack

[Fabrice Bellard]

The patch seems OK for me, at least until the instruction cache is 
dynamically allocated.

Fabrice.

Lennert Buytenhek wrote:
> Hi,
> 
> qemu-arm doesn't work on Fedora Core 2 by default because it expects to
> be able to execute stuff off the heap (code_gen_buffer), but that part of
> the heap is not marked PROT_EXEC, and Fedora Core 2 has a patch that sets
> the CS segment limit to the highest PROT_EXEC-mapped address in the address
> space.
> 
> It's a bit of a hacky fix, but it Works For Me(tm) on the default Fedora
> 2.6.5 kernel.  I haven't yet looked into why qemu-arm was crashing on the
> Fedora 2.6.[78] update kernels.
> 
> 
> --L
> 
> 
> diff -urN qemu-20040804.orig/exec.c qemu-20040804.test/exec.c
> --- qemu-20040804.orig/exec.c	2004-07-06 02:00:18.000000000 +0300
> +++ qemu-20040804.test/exec.c	2004-09-04 02:49:25.414945456 +0300
> @@ -127,6 +127,9 @@
>  
>  static void page_init(void)
>  {
> +    unsigned long address;
> +    unsigned long length;
> +
>      /* NOTE: we can always suppose that qemu_host_page_size >=
>         TARGET_PAGE_SIZE */
>  #ifdef _WIN32
> @@ -134,6 +137,17 @@
>  #else
>      qemu_real_host_page_size = getpagesize();
>  #endif
> +
> +    address = (unsigned long)code_gen_buffer;
> +    address &= ~(qemu_real_host_page_size - 1);
> +
> +    length = (unsigned long)code_gen_buffer + sizeof(code_gen_buffer);
> +    length -= address;
> +    length += qemu_real_host_page_size - 1;
> +    length &= ~(qemu_real_host_page_size - 1);
> +
> +    mprotect(address, length, PROT_READ | PROT_WRITE | PROT_EXEC);
> +
>      if (qemu_host_page_size == 0)
>          qemu_host_page_size = qemu_real_host_page_size;
>      if (qemu_host_page_size < TARGET_PAGE_SIZE)
> 
> 
> 
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel
> 
> 

Re: [Qemu-devel] [PATCH] fix interaction with noexecstack

[Filip Navara]

Fabrice Bellard wrote:

> The patch seems OK for me, at least until the instruction cache is 
> dynamically allocated.

I haven't tried the patch, but there isn't any "mprotect" function on 
Windows so I guess it would break MinGW builds. See the code below. It 
should work, but *I HAVEN'T TESTED IT*:

Regards,
Filip

> Lennert Buytenhek wrote:

[snip]

>> --- qemu-20040804.orig/exec.c    2004-07-06 02:00:18.000000000 +0300
>> +++ qemu-20040804.test/exec.c    2004-09-04 02:49:25.414945456 +0300
>> @@ -127,6 +127,9 @@
>>  
>>  static void page_init(void)
>>  {
>> +    unsigned long address;
>> +    unsigned long length;
>
#ifdef _WIN32
   DWORD old_protect;
#endif

>> +
>>      /* NOTE: we can always suppose that qemu_host_page_size >=
>>         TARGET_PAGE_SIZE */
>>  #ifdef _WIN32
>> @@ -134,6 +137,17 @@
>>  #else
>>      qemu_real_host_page_size = getpagesize();
>>  #endif
>> +
>> +    address = (unsigned long)code_gen_buffer;
>> +    address &= ~(qemu_real_host_page_size - 1);
>> +
>> +    length = (unsigned long)code_gen_buffer + sizeof(code_gen_buffer);
>> +    length -= address;
>> +    length += qemu_real_host_page_size - 1;
>> +    length &= ~(qemu_real_host_page_size - 1);
>> +
>
#ifdef _WIN32
*   *VirtualProtect(address, length, PAGE_EXECUTE_READWRITE, &old_protect);
#else

>> +    mprotect(address, length, PROT_READ | PROT_WRITE | PROT_EXEC);
>
#endif

>> +
>>      if (qemu_host_page_size == 0)
>>          qemu_host_page_size = qemu_real_host_page_size;
>>      if (qemu_host_page_size < TARGET_PAGE_SIZE)
>

[Qemu-devel] Re: [PATCH] fix interaction with noexecstack

[Ronald]

Le Sat, 04 Sep 2004 14:19:15 +0200, Filip Navara a écrit :

> Fabrice Bellard wrote:
> 
>> The patch seems OK for me, at least until the instruction cache is
>> dynamically allocated.
> 
> I haven't tried the patch, but there isn't any "mprotect" function on
> Windows so I guess it would break MinGW builds. See the code below. It
> should work, but *I HAVEN'T TESTED IT*:
> 

Tested, need to include <winbase.h> (and <windows.h>).
gcc juste produce a warning with VirtualProtect:
/home/ronald/Prog/Win32/combo/qemu/exec.c: Dans la fonction « page_init »:
/home/ronald/Prog/Win32/combo/qemu/exec.c:155: AVERTISSEMENT: passage de
arg 1 de « VirtualProtect » transforme en pointeur un entier sans
transtypage

aproximative translation: in function page_init passing arg1 of
VirtualProtect is making pointer from integer without a cast.

Note: with **VirtualProtect or *VirtualProtect make is aborting, I have
used VirtualProtect.

Re: [Qemu-devel] Re: [PATCH] fix interaction with noexecstack

[Filip Navara]

[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]

Ronald wrote:

>Tested, need to include <winbase.h> (and <windows.h>).
>gcc juste produce a warning with VirtualProtect:
>/home/ronald/Prog/Win32/combo/qemu/exec.c: Dans la fonction « page_init »:
>/home/ronald/Prog/Win32/combo/qemu/exec.c:155: AVERTISSEMENT: passage de
>arg 1 de « VirtualProtect » transforme en pointeur un entier sans
>transtypage
>
>aproximative translation: in function page_init passing arg1 of
>VirtualProtect is making pointer from integer without a cast.
>  
>
Thanks much for testing it, now I got a minute to test it on my box. The 
attach patch adds the #include, fixes the warning and also dynamicly 
detects the page size. These changes are actually needed when running 
QEMU on WinXP SP2 on AMD64. I'm quite not happy with the (original) 
patch yet, because it uses "unsigned long" for storing pointer and this 
will *break any 64-bit build* on non-Windows platforms. It would be nice 
if someone can solve it.

(My only hope is that the MinGW build of QEMU will not be broken...)

Regards,
Filip

>Note: with **VirtualProtect or *VirtualProtect make is aborting, I have
>used VirtualProtect.
>  
>
Sorry, my mail client messed that up.

[-- Attachment #2: exec.diff --]
[-- Type: text/x-patch, Size: 1373 bytes --]

--- qemu/exec.c	Sat Sep  4 15:51:23 2004
+++ qemu/exec.c	Sat Sep  4 16:12:05 2004
@@ -18,6 +18,9 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #include "config.h"
+#ifdef _WIN32
+#include <windows.h>
+#endif
 #include <stdlib.h>
 #include <stdio.h>
 #include <stdarg.h>
@@ -127,13 +130,35 @@
 
 static void page_init(void)
 {
+#ifdef _WIN32
+    SYSTEM_INFO system_info;
+    DWORD old_protect;
+#else
+    unsigned long address;
+    unsigned long length;
+#endif
+
     /* NOTE: we can always suppose that qemu_host_page_size >=
        TARGET_PAGE_SIZE */
 #ifdef _WIN32
-    qemu_real_host_page_size = 4096;
+    GetSystemInfo(&system_info);
+    qemu_real_host_page_size = system_info.dwPageSize;
+
+    VirtualProtect(code_gen_buffer, sizeof(code_gen_buffer),
+                   PAGE_EXECUTE_READWRITE, &old_protect);
 #else
     qemu_real_host_page_size = getpagesize();
+
+    address = (unsigned long)code_gen_buffer;
+    address &= ~(qemu_real_host_page_size - 1);
+
+    length = sizeof(code_gen_buffer);
+    length += qemu_real_host_page_size - 1;
+    length &= ~(qemu_real_host_page_size - 1);
+
+    mprotect(address, length, PROT_READ | PROT_WRITE | PROT_EXEC);
 #endif
+
     if (qemu_host_page_size == 0)
         qemu_host_page_size = qemu_real_host_page_size;
     if (qemu_host_page_size < TARGET_PAGE_SIZE)

[Qemu-devel] Re: Re: [PATCH] fix interaction with noexecstack

[Ronald]

Le Sat, 04 Sep 2004 18:15:47 +0200, Filip Navara a écrit :


>>  
> Thanks much for testing it, now I got a minute to test it on my box. The
> attach patch adds the #include, fixes the warning and also dynamicly
> detects the page size. These changes are actually needed when running QEMU
> on WinXP SP2 on AMD64. I'm quite not happy with the (original) patch yet,
> because it uses "unsigned long" for storing pointer and this will *break
> any 64-bit build* on non-Windows platforms. It would be nice if someone
> can solve it.
> 
> (My only hope is that the MinGW build of QEMU will not be broken...)
> 
> Regards,
> Filip
> 

works for me.
Build with mingw on linux, run on win98 host

RE: [Qemu-devel] Re: Re: [PATCH] fix interaction with noexecstack

[Hartmut Birr]

> -----Original Message-----
> From: qemu-devel-bounces+hartmut.birr=gmx.de@nongnu.org 
> [mailto:qemu-devel-bounces+hartmut.birr=gmx.de@nongnu.org] On 
> Behalf Of Ronald
> Sent: Saturday, September 04, 2004 7:07 PM
> To: qemu-devel@nongnu.org
> Subject: [Qemu-devel] Re: Re: [PATCH] fix interaction with noexecstack
> 
> 
> Le Sat, 04 Sep 2004 18:15:47 +0200, Filip Navara a écrit :
> 
> 
> >>  
> > Thanks much for testing it, now I got a minute to test it 
> on my box. The
> > attach patch adds the #include, fixes the warning and also dynamicly
> > detects the page size. These changes are actually needed 
> when running QEMU
> > on WinXP SP2 on AMD64. I'm quite not happy with the 
> (original) patch yet,
> > because it uses "unsigned long" for storing pointer and 
> this will *break
> > any 64-bit build* on non-Windows platforms. It would be 
> nice if someone
> > can solve it.
> > 
> > (My only hope is that the MinGW build of QEMU will not be broken...)
> > 
> > Regards,
> > Filip
> > 
> 
> works for me.
> Build with mingw on linux, run on win98 host
> 

Works for me too. Build with msys on WinXP, run on AMD64 WinXP host with DEP
full enabled.

- Hartmut

Re: [Qemu-devel] Re: [PATCH] fix interaction with noexecstack

[Lennert Buytenhek]

On Sat, Sep 04, 2004 at 06:15:47PM +0200, Filip Navara wrote:

> patch yet, because it uses "unsigned long" for storing pointer and this 
> will *break any 64-bit build* on non-Windows platforms. It would be nice 
> if someone can solve it.

There are no architectures that I am aware of that run linux and
where a void * does not fit inside an unsigned long.

In fact, the linux kernel heavily depends on being able to cast
void *'s to unsigned longs and back.


--L

Re: [Qemu-devel] Re: [PATCH] fix interaction with noexecstack

[Fabrice Bellard]

Applied with some modifications.

Fabrice.

Filip Navara wrote:
> Ronald wrote:
> 
>> Tested, need to include <winbase.h> (and <windows.h>).
>> gcc juste produce a warning with VirtualProtect:
>> /home/ronald/Prog/Win32/combo/qemu/exec.c: Dans la fonction « 
>> page_init »:
>> /home/ronald/Prog/Win32/combo/qemu/exec.c:155: AVERTISSEMENT: passage de
>> arg 1 de « VirtualProtect » transforme en pointeur un entier sans
>> transtypage
>>
>> aproximative translation: in function page_init passing arg1 of
>> VirtualProtect is making pointer from integer without a cast.
>>  
>>
> Thanks much for testing it, now I got a minute to test it on my box. The 
> attach patch adds the #include, fixes the warning and also dynamicly 
> detects the page size. These changes are actually needed when running 
> QEMU on WinXP SP2 on AMD64. I'm quite not happy with the (original) 
> patch yet, because it uses "unsigned long" for storing pointer and this 
> will *break any 64-bit build* on non-Windows platforms. It would be nice 
> if someone can solve it.
> 
> (My only hope is that the MinGW build of QEMU will not be broken...)
> 
> Regards,
> Filip
> 
>> Note: with **VirtualProtect or *VirtualProtect make is aborting, I have
>> used VirtualProtect.
>>  
>>
> Sorry, my mail client messed that up