Wrong signed data type on pageflags_* functions - limit to 2GB memory allocation

Posted by Luca Bonissi 9 months, 2 weeks ago

On 32bit qemu-user targets, memory allocation failed after about 2GB due 
to incorrect signed (instead of the correct unsigned) "last" parameter 
in pageflags_find and pageflags_next functions (file accel/tcg/user-exec.c).

The parameter, on 32bit targets, will be signed-extent to the 64bit 
final uint64_t parameters, leading to incorrect comparison on the RBTree 
(only the first call to mmap on the upper 2GB memory will be successful).

Following the patch to fix the bug:

--- qemu-20230327.orig/accel/tcg/user-exec.c	2023-03-27 
15:41:42.000000000 +0200
+++ qemu-20230327/accel/tcg/user-exec.c	2023-07-15 14:09:07.160453759 +0200
@@ -144,7 +144,7 @@ typedef struct PageFlagsNode {

  static IntervalTreeRoot pageflags_root;

-static PageFlagsNode *pageflags_find(target_ulong start, target_long last)
+static PageFlagsNode *pageflags_find(target_ulong start, target_ulong last)
  {
      IntervalTreeNode *n;

@@ -153,7 +153,7 @@ static PageFlagsNode *pageflags_find(tar
  }

  static PageFlagsNode *pageflags_next(PageFlagsNode *p, target_ulong start,
-                                     target_long last)
+                                     target_ulong last)
  {
      IntervalTreeNode *n;

Re: [PATCH] Wrong signed data type on pageflags_* functions - limit to 2GB memory allocation

Posted by Luca Bonissi 9 months, 2 weeks ago

On 32bit qemu-user targets, memory allocation failed after about 2GB due 
to incorrect signed (instead of the correct unsigned) "last" parameter 
in pageflags_find and pageflags_next functions (file accel/tcg/user-exec.c).

The parameter, on 32bit targets, will be signed-extent to the 64bit 
final uint64_t parameters, leading to incorrect comparison on the RBTree 
(only the first call to mmap on the upper 2GB memory will be successful).

Following the patch to fix the bug (re-submit to add "signed-off-by"):

Signed-off-by: Luca Bonissi <qemu@bonslack.org>
---

diff -up a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
--- a/accel/tcg/user-exec.c    2023-03-27 15:41:42.000000000 +0200
+++ b/accel/tcg/user-exec.c    2023-07-15 14:09:07.160453759 +0200
@@ -144,7 +144,7 @@ typedef struct PageFlagsNode {

  static IntervalTreeRoot pageflags_root;

-static PageFlagsNode *pageflags_find(target_ulong start, target_long last)
+static PageFlagsNode *pageflags_find(target_ulong start, target_ulong last)
  {
      IntervalTreeNode *n;

@@ -153,7 +153,7 @@ static PageFlagsNode *pageflags_find(tar
  }

  static PageFlagsNode *pageflags_next(PageFlagsNode *p, target_ulong start,
-                                     target_long last)
+                                     target_ulong last)
  {
      IntervalTreeNode *n;

Re: [PATCH] Wrong signed data type on pageflags_* functions - limit to 2GB memory allocation

Posted by Richard Henderson 9 months, 1 week ago

On 7/18/23 15:50, Luca Bonissi wrote:
> On 32bit qemu-user targets, memory allocation failed after about 2GB due to incorrect 
> signed (instead of the correct unsigned) "last" parameter in pageflags_find and 
> pageflags_next functions (file accel/tcg/user-exec.c).
> 
> The parameter, on 32bit targets, will be signed-extent to the 64bit final uint64_t 
> parameters, leading to incorrect comparison on the RBTree (only the first call to mmap on 
> the upper 2GB memory will be successful).
> 
> Following the patch to fix the bug (re-submit to add "signed-off-by"):
> 
> Signed-off-by: Luca Bonissi <qemu@bonslack.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Don't reply to previous patches with a new patch -- tooling doesn't handle it.
I've applied this by hand.


r~