diff mbox

[PATCHv2] Avoid page_set_flags() assert in qemu-user host page protection code

Message ID 20100331210036.GA44152@triton8.kn-bremen.de
State New
Headers show

Commit Message

Juergen Lock March 31, 2010, 9 p.m. UTC
V2 that uses endaddr = end-of-guest-address-space if !h2g_valid(endaddr)
after I found out that indeed works; and also disables the FreeBSD 6.x
/compat/linux/proc/self/maps fallback because it can return partial lines
if (at least I think that's the reason) the mappings change between
subsequent read() calls.

Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>

Comments

Richard Henderson March 31, 2010, 9:15 p.m. UTC | #1
On 03/31/2010 02:00 PM, Juergen Lock wrote:
> V2 that uses endaddr = end-of-guest-address-space if !h2g_valid(endaddr)
> after I found out that indeed works; and also disables the FreeBSD 6.x
> /compat/linux/proc/self/maps fallback because it can return partial lines
> if (at least I think that's the reason) the mappings change between
> subsequent read() calls.
> 
> Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>

Acked-by: Richard Henderson <rth@twiddle.net>

> 
> --- a/exec.c
> +++ b/exec.c
> @@ -306,13 +306,14 @@ static void page_init(void)
>  
>                      if (h2g_valid(endaddr)) {
>                          endaddr = h2g(endaddr);
> -                        page_set_flags(startaddr, endaddr, PAGE_RESERVED);
>                      } else {
>  #if TARGET_ABI_BITS <= L1_MAP_ADDR_SPACE_BITS
>                          endaddr = ~0ul;
> -                        page_set_flags(startaddr, endaddr, PAGE_RESERVED);
> +#else
> +                        endaddr = ((abi_ulong)1 << L1_MAP_ADDR_SPACE_BITS) - 1;
>  #endif
>                      }
> +                    page_set_flags(startaddr, endaddr, PAGE_RESERVED);
>                  }
>              }
>              free(freep);
> @@ -323,11 +324,7 @@ static void page_init(void)
>  
>          last_brk = (unsigned long)sbrk(0);
>  
> -#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__)
> -        f = fopen("/compat/linux/proc/self/maps", "r");
> -#else
>          f = fopen("/proc/self/maps", "r");
> -#endif
>          if (f) {
>              mmap_lock();
>  
> @@ -343,7 +340,11 @@ static void page_init(void)
>                      if (h2g_valid(endaddr)) {
>                          endaddr = h2g(endaddr);
>                      } else {
> +#if TARGET_ABI_BITS <= L1_MAP_ADDR_SPACE_BITS
>                          endaddr = ~0ul;
> +#else
> +                        endaddr = ((abi_ulong)1 << L1_MAP_ADDR_SPACE_BITS) - 1;
> +#endif
>                      }
>                      page_set_flags(startaddr, endaddr, PAGE_RESERVED);
>                  }
> 
>
Aurelien Jarno April 9, 2010, 8:02 p.m. UTC | #2
On Wed, Mar 31, 2010 at 11:00:36PM +0200, Juergen Lock wrote:
> V2 that uses endaddr = end-of-guest-address-space if !h2g_valid(endaddr)
> after I found out that indeed works; and also disables the FreeBSD 6.x
> /compat/linux/proc/self/maps fallback because it can return partial lines
> if (at least I think that's the reason) the mappings change between
> subsequent read() calls.
> 
> Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>

Thanks, applied.

> --- a/exec.c
> +++ b/exec.c
> @@ -306,13 +306,14 @@ static void page_init(void)
>  
>                      if (h2g_valid(endaddr)) {
>                          endaddr = h2g(endaddr);
> -                        page_set_flags(startaddr, endaddr, PAGE_RESERVED);
>                      } else {
>  #if TARGET_ABI_BITS <= L1_MAP_ADDR_SPACE_BITS
>                          endaddr = ~0ul;
> -                        page_set_flags(startaddr, endaddr, PAGE_RESERVED);
> +#else
> +                        endaddr = ((abi_ulong)1 << L1_MAP_ADDR_SPACE_BITS) - 1;
>  #endif
>                      }
> +                    page_set_flags(startaddr, endaddr, PAGE_RESERVED);
>                  }
>              }
>              free(freep);
> @@ -323,11 +324,7 @@ static void page_init(void)
>  
>          last_brk = (unsigned long)sbrk(0);
>  
> -#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__)
> -        f = fopen("/compat/linux/proc/self/maps", "r");
> -#else
>          f = fopen("/proc/self/maps", "r");
> -#endif
>          if (f) {
>              mmap_lock();
>  
> @@ -343,7 +340,11 @@ static void page_init(void)
>                      if (h2g_valid(endaddr)) {
>                          endaddr = h2g(endaddr);
>                      } else {
> +#if TARGET_ABI_BITS <= L1_MAP_ADDR_SPACE_BITS
>                          endaddr = ~0ul;
> +#else
> +                        endaddr = ((abi_ulong)1 << L1_MAP_ADDR_SPACE_BITS) - 1;
> +#endif
>                      }
>                      page_set_flags(startaddr, endaddr, PAGE_RESERVED);
>                  }
> 
> 
>
Aurelien Jarno April 10, 2010, 3:22 p.m. UTC | #3
On Fri, Apr 09, 2010 at 10:02:41PM +0200, Aurelien Jarno wrote:
> On Wed, Mar 31, 2010 at 11:00:36PM +0200, Juergen Lock wrote:
> > V2 that uses endaddr = end-of-guest-address-space if !h2g_valid(endaddr)
> > after I found out that indeed works; and also disables the FreeBSD 6.x
> > /compat/linux/proc/self/maps fallback because it can return partial lines
> > if (at least I think that's the reason) the mappings change between
> > subsequent read() calls.
> > 
> > Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>
> 
> Thanks, applied.
> 

This patch actually breaks the build on 32-bit hosts. I have reverted
it.
Juergen Lock April 10, 2010, 5:45 p.m. UTC | #4
On Sat, Apr 10, 2010 at 05:22:08PM +0200, Aurelien Jarno wrote:
> On Fri, Apr 09, 2010 at 10:02:41PM +0200, Aurelien Jarno wrote:
> > On Wed, Mar 31, 2010 at 11:00:36PM +0200, Juergen Lock wrote:
> > > V2 that uses endaddr = end-of-guest-address-space if !h2g_valid(endaddr)
> > > after I found out that indeed works; and also disables the FreeBSD 6.x
> > > /compat/linux/proc/self/maps fallback because it can return partial lines
> > > if (at least I think that's the reason) the mappings change between
> > > subsequent read() calls.
> > > 
> > > Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>
> > 
> > Thanks, applied.
> > 
> 
> This patch actually breaks the build on 32-bit hosts. I have reverted
> it.

Oh dear, sorry about that. :(  Should I make a new version thats
conditional on the host being 64-bit?  Or do you think 32-bit hosts
also need something fixed there?

 Sorry again...
	Juergen
diff mbox

Patch

--- a/exec.c
+++ b/exec.c
@@ -306,13 +306,14 @@  static void page_init(void)
 
                     if (h2g_valid(endaddr)) {
                         endaddr = h2g(endaddr);
-                        page_set_flags(startaddr, endaddr, PAGE_RESERVED);
                     } else {
 #if TARGET_ABI_BITS <= L1_MAP_ADDR_SPACE_BITS
                         endaddr = ~0ul;
-                        page_set_flags(startaddr, endaddr, PAGE_RESERVED);
+#else
+                        endaddr = ((abi_ulong)1 << L1_MAP_ADDR_SPACE_BITS) - 1;
 #endif
                     }
+                    page_set_flags(startaddr, endaddr, PAGE_RESERVED);
                 }
             }
             free(freep);
@@ -323,11 +324,7 @@  static void page_init(void)
 
         last_brk = (unsigned long)sbrk(0);
 
-#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__)
-        f = fopen("/compat/linux/proc/self/maps", "r");
-#else
         f = fopen("/proc/self/maps", "r");
-#endif
         if (f) {
             mmap_lock();
 
@@ -343,7 +340,11 @@  static void page_init(void)
                     if (h2g_valid(endaddr)) {
                         endaddr = h2g(endaddr);
                     } else {
+#if TARGET_ABI_BITS <= L1_MAP_ADDR_SPACE_BITS
                         endaddr = ~0ul;
+#else
+                        endaddr = ((abi_ulong)1 << L1_MAP_ADDR_SPACE_BITS) - 1;
+#endif
                     }
                     page_set_flags(startaddr, endaddr, PAGE_RESERVED);
                 }