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.0On 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
On 10/20/25 16:26, Peter Maydell wrote: >> +#define _GNU_SOURCE > > Why do we need to define this now ? 'mremap' isn't POSIX, so on Linux at least is only exposed if _GNU_SOURCE is defined. >> - 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. > >> [snip] > > 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); Sorry about all of this---I got quite confused by the printing logic in this file, mainly because AFAICT it's pretty buggy. For instance, the old prints in 'check_invalid_mmaps' would have caused output something like this: check_invalid_mmap addr=0x1234check_invalid_mmap addr=0x1234 passed I agree that this is a separate issue though, so I'll put all of the printing logic back how it was and add a matching fprintf for my new subcase. I'll leave the cleanup here to someone else. >> @@ -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. Oops! I mostly work in a language where my editor config forces space indentation, so occasionally forget to set space indentation when I do need to. Will correct (and double-check indentation in the rest of the series) in the next revision. Thanks for the feedback! -- Matthew
On 11/17/25 15:13, Matthew Lugg wrote: > On 10/20/25 16:26, Peter Maydell wrote: >> 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. > > Oops! I mostly work in a language where my editor config forces space > indentation, so occasionally forget to set space indentation when I do > need to. Will correct (and double-check indentation in the rest of the > series) in the next revision. While working on this commit I just realised that you meant that *existing* parts of the file were indented with hard tabs (causing the apparent mismatch in the patch). Because there are quite a few hard tabs in the file, I'll leave them as-is to avoid a big whitespace diff, but will keep my added line correctly space-indented, as in the original patch. You (or whoever is involved in the final merge) can of course add a patch to fix the existing whitespace if you want.
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(); */
© 2016 - 2026 Red Hat, Inc.