[PATCH 4/4] tests: add tcg coverage for fixed mremap bugs

Matthew Lugg posted 4 patches 1 month ago
Maintainers: Laurent Vivier <laurent@vivier.eu>, "Alex Bennée" <alex.bennee@linaro.org>
[PATCH 4/4] tests: add tcg coverage for fixed mremap bugs
Posted by Matthew Lugg 1 month ago
These tests cover the first two fixes in this patch series. The final
patch is not covered because the bug it fixes is not easily observable
by the guest.

Signed-off-by: Matthew Lugg <mlugg@mlugg.co.uk>
---
 tests/tcg/multiarch/test-mmap.c | 47 +++++++++++++++++++++++++++++----
 1 file changed, 42 insertions(+), 5 deletions(-)

diff --git a/tests/tcg/multiarch/test-mmap.c b/tests/tcg/multiarch/test-mmap.c
index 96257f8ebe..64df694d1a 100644
--- a/tests/tcg/multiarch/test-mmap.c
+++ b/tests/tcg/multiarch/test-mmap.c
@@ -22,6 +22,7 @@
  * along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#define _GNU_SOURCE
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
@@ -36,12 +37,12 @@
 do                                                             \
 {                                                              \
   if (!(x)) {                                                  \
-    fprintf(stderr, "FAILED at %s:%d\n", __FILE__, __LINE__); \
+    fprintf(stderr, " FAILED at %s:%d\n", __FILE__, __LINE__); \
     exit (EXIT_FAILURE);                                       \
   }                                                            \
 } while (0)
 
-unsigned char *dummybuf;
+unsigned char *dummybuf; /* length is 2*pagesize */
 static unsigned int pagesize;
 static unsigned int pagemask;
 int test_fd;
@@ -439,21 +440,56 @@ void check_invalid_mmaps(void)
 {
     unsigned char *addr;
 
+    fprintf(stdout, "%s", __func__);
+
     /* Attempt to map a zero length page.  */
     addr = mmap(NULL, 0, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-    fprintf(stdout, "%s addr=%p", __func__, (void *)addr);
     fail_unless(addr == MAP_FAILED);
     fail_unless(errno == EINVAL);
 
     /* Attempt to map a over length page.  */
     addr = mmap(NULL, -4, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-    fprintf(stdout, "%s addr=%p", __func__, (void *)addr);
     fail_unless(addr == MAP_FAILED);
     fail_unless(errno == ENOMEM);
 
+    /* Attempt to remap a region which exceeds the bounds of memory. */
+    addr = mremap((void *)((uintptr_t)pagesize * 10), SIZE_MAX & ~(size_t)pagemask, pagesize, 0);
+    fail_unless(addr == MAP_FAILED);
+    fail_unless(errno == EFAULT);
+
     fprintf(stdout, " passed\n");
 }
 
+void check_shrink_mmaps(void)
+{
+    unsigned char *a, *b, *c;
+    a = mmap(NULL, pagesize * 2, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+    b = mmap(NULL, pagesize * 2, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+    c = mmap(NULL, pagesize * 2, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+
+    fail_unless(a != MAP_FAILED);
+    fail_unless(b != MAP_FAILED);
+    fail_unless(c != MAP_FAILED);
+
+    /* Ensure we can read the full mappings */
+    memcpy(dummybuf, a, 2 * pagesize);
+    memcpy(dummybuf, b, 2 * pagesize);
+    memcpy(dummybuf, c, 2 * pagesize);
+
+    /* Shrink the middle mapping in-place; the others should be unaffected */
+    b = mremap(b, pagesize * 2, pagesize, 0);
+    fail_unless(b != MAP_FAILED);
+
+    /* Ensure we can still access all valid mappings */
+    memcpy(dummybuf, a, 2 * pagesize);
+    memcpy(dummybuf, b, pagesize);
+    memcpy(dummybuf, c, 2 * pagesize);
+
+    munmap(a, 2 * pagesize);
+    munmap(b, pagesize);
+    munmap(c, 2 * pagesize);
+}
+
 int main(int argc, char **argv)
 {
 	char tempname[] = "/tmp/.cmmapXXXXXX";
@@ -468,7 +504,7 @@ int main(int argc, char **argv)
 
 	/* Assume pagesize is a power of two.  */
 	pagemask = pagesize - 1;
-	dummybuf = malloc (pagesize);
+	dummybuf = malloc (pagesize * 2);
 	printf ("pagesize=%u pagemask=%x\n", pagesize, pagemask);
 
 	test_fd = mkstemp(tempname);
@@ -496,6 +532,7 @@ int main(int argc, char **argv)
 	check_file_fixed_eof_mmaps();
 	check_file_unfixed_eof_mmaps();
 	check_invalid_mmaps();
+    check_shrink_mmaps();
 
 	/* Fails at the moment.  */
 	/* check_aligned_anonymous_fixed_mmaps_collide_with_host(); */
-- 
2.51.0
Re: [PATCH 4/4] tests: add tcg coverage for fixed mremap bugs
Posted by Peter Maydell 3 weeks, 4 days ago
On Sat, 11 Oct 2025 at 21:20, Matthew Lugg <mlugg@mlugg.co.uk> wrote:
>
> These tests cover the first two fixes in this patch series. The final
> patch is not covered because the bug it fixes is not easily observable
> by the guest.
>
> Signed-off-by: Matthew Lugg <mlugg@mlugg.co.uk>
> ---
>  tests/tcg/multiarch/test-mmap.c | 47 +++++++++++++++++++++++++++++----
>  1 file changed, 42 insertions(+), 5 deletions(-)

The test case itself looks good, so my review comments
below are just minor nits.

> diff --git a/tests/tcg/multiarch/test-mmap.c b/tests/tcg/multiarch/test-mmap.c
> index 96257f8ebe..64df694d1a 100644
> --- a/tests/tcg/multiarch/test-mmap.c
> +++ b/tests/tcg/multiarch/test-mmap.c
> @@ -22,6 +22,7 @@
>   * along with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>
> +#define _GNU_SOURCE

Why do we need to define this now ?

>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <stdint.h>
> @@ -36,12 +37,12 @@
>  do                                                             \
>  {                                                              \
>    if (!(x)) {                                                  \
> -    fprintf(stderr, "FAILED at %s:%d\n", __FILE__, __LINE__); \
> +    fprintf(stderr, " FAILED at %s:%d\n", __FILE__, __LINE__); \

I think that this is trying to fix a cosmetic bug in
the printing of error messages: the tests each print
some line without a newline, like:
  fprintf(stdout, "%s addr=%p", __func__, (void *)addr);
and then for the passing case we add a space and complete the line:
  fprintf(stdout, " passed\n");

but this fail_unless() macro is not adding the space, so
presumably we print something awkward like
check_invalid_mmaps addr=0x12435468FAILED at ...

But we should separate out this trivial cleanup from
the patch adding a new test case.

>      exit (EXIT_FAILURE);                                       \
>    }                                                            \
>  } while (0)
>
> -unsigned char *dummybuf;
> +unsigned char *dummybuf; /* length is 2*pagesize */
>  static unsigned int pagesize;
>  static unsigned int pagemask;
>  int test_fd;
> @@ -439,21 +440,56 @@ void check_invalid_mmaps(void)
>  {
>      unsigned char *addr;
>
> +    fprintf(stdout, "%s", __func__);
> +
>      /* Attempt to map a zero length page.  */
>      addr = mmap(NULL, 0, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> -    fprintf(stdout, "%s addr=%p", __func__, (void *)addr);
>      fail_unless(addr == MAP_FAILED);
>      fail_unless(errno == EINVAL);
>
>      /* Attempt to map a over length page.  */
>      addr = mmap(NULL, -4, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> -    fprintf(stdout, "%s addr=%p", __func__, (void *)addr);
>      fail_unless(addr == MAP_FAILED);
>      fail_unless(errno == ENOMEM);

Can we leave the printfs for the existing test cases alone?
You can add a new one for your new subcase:
       fprintf(stdout, "%s mremap addr=%p", __func__, addr);

> +    /* Attempt to remap a region which exceeds the bounds of memory. */
> +    addr = mremap((void *)((uintptr_t)pagesize * 10), SIZE_MAX & ~(size_t)pagemask, pagesize, 0);
> +    fail_unless(addr == MAP_FAILED);
> +    fail_unless(errno == EFAULT);
> +
>      fprintf(stdout, " passed\n");
>  }
>
> +void check_shrink_mmaps(void)
> +{
> +    unsigned char *a, *b, *c;
> +    a = mmap(NULL, pagesize * 2, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +    b = mmap(NULL, pagesize * 2, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +    c = mmap(NULL, pagesize * 2, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +
> +    fail_unless(a != MAP_FAILED);
> +    fail_unless(b != MAP_FAILED);
> +    fail_unless(c != MAP_FAILED);
> +
> +    /* Ensure we can read the full mappings */
> +    memcpy(dummybuf, a, 2 * pagesize);
> +    memcpy(dummybuf, b, 2 * pagesize);
> +    memcpy(dummybuf, c, 2 * pagesize);
> +
> +    /* Shrink the middle mapping in-place; the others should be unaffected */
> +    b = mremap(b, pagesize * 2, pagesize, 0);
> +    fail_unless(b != MAP_FAILED);
> +
> +    /* Ensure we can still access all valid mappings */
> +    memcpy(dummybuf, a, 2 * pagesize);
> +    memcpy(dummybuf, b, pagesize);
> +    memcpy(dummybuf, c, 2 * pagesize);
> +
> +    munmap(a, 2 * pagesize);
> +    munmap(b, pagesize);
> +    munmap(c, 2 * pagesize);
> +}
> +
>  int main(int argc, char **argv)
>  {
>         char tempname[] = "/tmp/.cmmapXXXXXX";
> @@ -468,7 +504,7 @@ int main(int argc, char **argv)
>
>         /* Assume pagesize is a power of two.  */
>         pagemask = pagesize - 1;
> -       dummybuf = malloc (pagesize);
> +       dummybuf = malloc (pagesize * 2);
>         printf ("pagesize=%u pagemask=%x\n", pagesize, pagemask);
>
>         test_fd = mkstemp(tempname);
> @@ -496,6 +532,7 @@ int main(int argc, char **argv)
>         check_file_fixed_eof_mmaps();
>         check_file_unfixed_eof_mmaps();
>         check_invalid_mmaps();
> +    check_shrink_mmaps();

I was going to complain about indent on this line, but the
problem seems to be that the file is incorrectly
indented with hardcoded tabs for parts of it.

>         /* Fails at the moment.  */
>         /* check_aligned_anonymous_fixed_mmaps_collide_with_host(); */

thanks
-- PMM
Re: [PATCH 4/4] tests: add tcg coverage for fixed mremap bugs
Posted by Matthew Lugg 1 month ago
I only just noticed I accidentally changed a line in a macro at the top 
of this
file ("FAILED" -> " FAILED"); that was unintentional. I'll omit that 
change from
future versions of this patch.

On 10/11/25 21:03, Matthew Lugg wrote:
> These tests cover the first two fixes in this patch series. The final
> patch is not covered because the bug it fixes is not easily observable
> by the guest.
>
> Signed-off-by: Matthew Lugg <mlugg@mlugg.co.uk>
> ---
>   tests/tcg/multiarch/test-mmap.c | 47 +++++++++++++++++++++++++++++----
>   1 file changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/tests/tcg/multiarch/test-mmap.c b/tests/tcg/multiarch/test-mmap.c
> index 96257f8ebe..64df694d1a 100644
> --- a/tests/tcg/multiarch/test-mmap.c
> +++ b/tests/tcg/multiarch/test-mmap.c
> @@ -22,6 +22,7 @@
>    * along with this program; if not, see <http://www.gnu.org/licenses/>.
>    */
>   
> +#define _GNU_SOURCE
>   #include <stdio.h>
>   #include <stdlib.h>
>   #include <stdint.h>
> @@ -36,12 +37,12 @@
>   do                                                             \
>   {                                                              \
>     if (!(x)) {                                                  \
> -    fprintf(stderr, "FAILED at %s:%d\n", __FILE__, __LINE__); \
> +    fprintf(stderr, " FAILED at %s:%d\n", __FILE__, __LINE__); \
>       exit (EXIT_FAILURE);                                       \
>     }                                                            \
>   } while (0)
>   
> -unsigned char *dummybuf;
> +unsigned char *dummybuf; /* length is 2*pagesize */
>   static unsigned int pagesize;
>   static unsigned int pagemask;
>   int test_fd;
> @@ -439,21 +440,56 @@ void check_invalid_mmaps(void)
>   {
>       unsigned char *addr;
>   
> +    fprintf(stdout, "%s", __func__);
> +
>       /* Attempt to map a zero length page.  */
>       addr = mmap(NULL, 0, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> -    fprintf(stdout, "%s addr=%p", __func__, (void *)addr);
>       fail_unless(addr == MAP_FAILED);
>       fail_unless(errno == EINVAL);
>   
>       /* Attempt to map a over length page.  */
>       addr = mmap(NULL, -4, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> -    fprintf(stdout, "%s addr=%p", __func__, (void *)addr);
>       fail_unless(addr == MAP_FAILED);
>       fail_unless(errno == ENOMEM);
>   
> +    /* Attempt to remap a region which exceeds the bounds of memory. */
> +    addr = mremap((void *)((uintptr_t)pagesize * 10), SIZE_MAX & ~(size_t)pagemask, pagesize, 0);
> +    fail_unless(addr == MAP_FAILED);
> +    fail_unless(errno == EFAULT);
> +
>       fprintf(stdout, " passed\n");
>   }
>   
> +void check_shrink_mmaps(void)
> +{
> +    unsigned char *a, *b, *c;
> +    a = mmap(NULL, pagesize * 2, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +    b = mmap(NULL, pagesize * 2, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +    c = mmap(NULL, pagesize * 2, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +
> +    fail_unless(a != MAP_FAILED);
> +    fail_unless(b != MAP_FAILED);
> +    fail_unless(c != MAP_FAILED);
> +
> +    /* Ensure we can read the full mappings */
> +    memcpy(dummybuf, a, 2 * pagesize);
> +    memcpy(dummybuf, b, 2 * pagesize);
> +    memcpy(dummybuf, c, 2 * pagesize);
> +
> +    /* Shrink the middle mapping in-place; the others should be unaffected */
> +    b = mremap(b, pagesize * 2, pagesize, 0);
> +    fail_unless(b != MAP_FAILED);
> +
> +    /* Ensure we can still access all valid mappings */
> +    memcpy(dummybuf, a, 2 * pagesize);
> +    memcpy(dummybuf, b, pagesize);
> +    memcpy(dummybuf, c, 2 * pagesize);
> +
> +    munmap(a, 2 * pagesize);
> +    munmap(b, pagesize);
> +    munmap(c, 2 * pagesize);
> +}
> +
>   int main(int argc, char **argv)
>   {
>   	char tempname[] = "/tmp/.cmmapXXXXXX";
> @@ -468,7 +504,7 @@ int main(int argc, char **argv)
>   
>   	/* Assume pagesize is a power of two.  */
>   	pagemask = pagesize - 1;
> -	dummybuf = malloc (pagesize);
> +	dummybuf = malloc (pagesize * 2);
>   	printf ("pagesize=%u pagemask=%x\n", pagesize, pagemask);
>   
>   	test_fd = mkstemp(tempname);
> @@ -496,6 +532,7 @@ int main(int argc, char **argv)
>   	check_file_fixed_eof_mmaps();
>   	check_file_unfixed_eof_mmaps();
>   	check_invalid_mmaps();
> +    check_shrink_mmaps();
>   
>   	/* Fails at the moment.  */
>   	/* check_aligned_anonymous_fixed_mmaps_collide_with_host(); */