[PATCH] linux-user: Avoid mmap of the last byte of the reserved_va

[PATCH] linux-user: Avoid mmap of the last byte of the reserved_va

[Richard Henderson]

There is an overflow problem in mmap_find_vma_reserved:
when reserved_va == UINT32_MAX, end may overflow to 0.
Rather than a larger rewrite at this time, simply avoid
the final byte of the VA, which avoids searching the
final page, which avoids the overflow.

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1741
Fixes: 95059f9c ("include/exec: Change reserved_va semantics to last byte")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/mmap.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 0aa8ae7356..2692936773 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -281,9 +281,15 @@ static abi_ulong mmap_find_vma_reserved(abi_ulong start, abi_ulong size,
     /* Note that start and size have already been aligned by mmap_find_vma. */
 
     end_addr = start + size;
+    /*
+     * Start at the top of the address space, ignoring the last page.
+     * If reserved_va == UINT32_MAX, then end_addr wraps to 0,
+     * throwing the rest of the calculations off.
+     * TODO: rewrite using last_addr instead.
+     * TODO: use the interval tree instead of probing every page.
+     */
     if (start > reserved_va - size) {
-        /* Start at the top of the address space.  */
-        end_addr = ((reserved_va + 1 - size) & -align) + size;
+        end_addr = ((reserved_va - size) & -align) + size;
         looped = true;
     }
 
@@ -296,8 +302,8 @@ static abi_ulong mmap_find_vma_reserved(abi_ulong start, abi_ulong size,
                 /* Failure.  The entire address space has been searched.  */
                 return (abi_ulong)-1;
             }
-            /* Re-start at the top of the address space.  */
-            addr = end_addr = ((reserved_va + 1 - size) & -align) + size;
+            /* Re-start at the top of the address space (see above). */
+            addr = end_addr = ((reserved_va - size) & -align) + size;
             looped = true;
         } else {
             prot = page_get_flags(addr);
-- 
2.34.1

Re: [PATCH] linux-user: Avoid mmap of the last byte of the reserved_va

[Michael Tokarev]

29.06.2023 11:08, Richard Henderson wrote:
> There is an overflow problem in mmap_find_vma_reserved:
> when reserved_va == UINT32_MAX, end may overflow to 0.
> Rather than a larger rewrite at this time, simply avoid
> the final byte of the VA, which avoids searching the
> final page, which avoids the overflow.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1741
> Fixes: 95059f9c ("include/exec: Change reserved_va semantics to last byte")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

So, I pushed this to debian (where we've seen multiple failures),
let's see how it goes..

/mjt

Re: [PATCH] linux-user: Avoid mmap of the last byte of the reserved_va

[Michael Tokarev]

29.06.2023 11:08, Richard Henderson wrote:
> There is an overflow problem in mmap_find_vma_reserved:
> when reserved_va == UINT32_MAX, end may overflow to 0.
> Rather than a larger rewrite at this time, simply avoid
> the final byte of the VA, which avoids searching the
> final page, which avoids the overflow.

This hack appears to fix known issues and apparently does not
introduce regressions.

Can it be applied to master and picked up from there, since
master is also broken?  You can revert it in the subsequent
patchset like the one you posted today.

You can add my:

Tested-by: Michael Tokarev <mjt@tls.msk.ru>
Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>

Thanks!

/mjt

Re: [PATCH] linux-user: Avoid mmap of the last byte of the reserved_va

[Richard Henderson]

On 6/30/23 18:06, Michael Tokarev wrote:
> 29.06.2023 11:08, Richard Henderson wrote:
>> There is an overflow problem in mmap_find_vma_reserved:
>> when reserved_va == UINT32_MAX, end may overflow to 0.
>> Rather than a larger rewrite at this time, simply avoid
>> the final byte of the VA, which avoids searching the
>> final page, which avoids the overflow.
> 
> This hack appears to fix known issues and apparently does not
> introduce regressions.
> 
> Can it be applied to master and picked up from there, since
> master is also broken?  You can revert it in the subsequent
> patchset like the one you posted today.
> 
> You can add my:
> 
> Tested-by: Michael Tokarev <mjt@tls.msk.ru>
> Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>

Yes, that's a good idea.  Queued to tcg-next.


r~