| Message ID | 20240405084410.256788-2-dev.jain@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | A new selftests/ directory for arm compatibility testing | expand |
On 4/5/24 1:44 PM, Dev Jain wrote: > This patch tests the 4GB VA restriction for 32-bit processes; it is required > to test the compat layer, whether the kernel knows that it is running a 32-bit > process or not. Chunks are allocated until the VA gets exhausted; mmap must > fail beyond 4GB. This is asserted against the VA mappings found > in /proc/self/maps. > > Signed-off-by: Dev Jain <dev.jain@arm.com> > --- > tools/testing/selftests/arm/mm/compat_va.c | 94 ++++++++++++++++++++++ > 1 file changed, 94 insertions(+) > create mode 100644 tools/testing/selftests/arm/mm/compat_va.c > > diff --git a/tools/testing/selftests/arm/mm/compat_va.c b/tools/testing/selftests/arm/mm/compat_va.c > new file mode 100644 > index 000000000000..3a78f240bc87 > --- /dev/null > +++ b/tools/testing/selftests/arm/mm/compat_va.c > @@ -0,0 +1,94 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2024 ARM Limited > + * > + * Author : Dev Jain <dev.jain@arm.com> > + * > + * Tests 4GB VA restriction for 32 bit process > + */ > + > +#define _GNU_SOURCE > +#include <stdio.h> > +#include <stdlib.h> > +#include <unistd.h> > +#include <sys/mman.h> > + > +#include <linux/sizes.h> > +#include <kselftest.h> > + > +#define MAP_CHUNK_SIZE SZ_1M > +#define NR_CHUNKS_4G (SZ_1G / MAP_CHUNK_SIZE) * 4 /* prevent overflow */ > + > +static int validate_address_hint(void) > +{ > + char *ptr; > + > + ptr = mmap((void *) (1UL << 29), MAP_CHUNK_SIZE, PROT_READ | > + PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); > + > + if (ptr == MAP_FAILED) > + return 0; > + > + return 1; Usually we return negative value instead of positive one which indicates error situation. > +} > + > +int main(int argc, char *argv[]) > +{ > + char *ptr[NR_CHUNKS_4G + 3]; > + char line[1000]; > + const char *file_name; > + int chunks; > + FILE *file; > + int i; > + > + ksft_print_header(); > + ksft_set_plan(1); There are multiple test cases. Instead of saying there is only 1 test. There should be multiple ksft_test_result{_pass,_fail} statements for each sub-tests. > + > + /* try allocation beyond 4 GB */ > + for (i = 0; i < NR_CHUNKS_4G + 3; ++i) { > + ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE, > + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); > + > + if (ptr[i] == MAP_FAILED) { > + if (validate_address_hint()) > + ksft_exit_fail_msg("VA exhaustion failed\n"); > + break; > + } > + } > + > + chunks = i; > + if (chunks >= NR_CHUNKS_4G) { > + ksft_test_result_fail("mmapped chunks beyond 4GB\n"); > + ksft_finished(); > + } > + > + /* parse /proc/self/maps, confirm 32 bit VA mappings */ > + file_name = "/proc/self/maps"; > + file = fopen(file_name, "r"); > + if (file == NULL) > + ksft_exit_fail_msg("/proc/self/maps cannot be opened\n"); > + > + while (fgets(line, sizeof(line), file)) { > + const char *whitespace_loc, *hyphen_loc; > + > + hyphen_loc = strchr(line, '-'); > + whitespace_loc = strchr(line, ' '); > + > + if (!(hyphen_loc && whitespace_loc)) { > + ksft_test_result_skip("Unexpected format"); > + ksft_finished(); I'm unable to follow as there are too many return statements. If you divide the test into multiple sub-tests, you can skip/pass/fail each sub-test easily. > + } > + > + if ((hyphen_loc - line > 8) || > + (whitespace_loc - hyphen_loc) > 9) { > + ksft_test_result_fail("Memory map more than 32 bits\n"); > + ksft_finished(); > + } > + } > + > + for (int i = 0; i < chunks; ++i) > + munmap(ptr[i], MAP_CHUNK_SIZE); > + > + ksft_test_result_pass("Test\n"); > + ksft_finished(); > +}
On 4/7/24 02:53, Muhammad Usama Anjum wrote: > On 4/5/24 1:44 PM, Dev Jain wrote: >> This patch tests the 4GB VA restriction for 32-bit processes; it is required >> to test the compat layer, whether the kernel knows that it is running a 32-bit >> process or not. Chunks are allocated until the VA gets exhausted; mmap must >> fail beyond 4GB. This is asserted against the VA mappings found >> in /proc/self/maps. >> >> Signed-off-by: Dev Jain <dev.jain@arm.com> >> --- >> tools/testing/selftests/arm/mm/compat_va.c | 94 ++++++++++++++++++++++ >> 1 file changed, 94 insertions(+) >> create mode 100644 tools/testing/selftests/arm/mm/compat_va.c >> >> diff --git a/tools/testing/selftests/arm/mm/compat_va.c b/tools/testing/selftests/arm/mm/compat_va.c >> new file mode 100644 >> index 000000000000..3a78f240bc87 >> --- /dev/null >> +++ b/tools/testing/selftests/arm/mm/compat_va.c >> @@ -0,0 +1,94 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2024 ARM Limited >> + * >> + * Author : Dev Jain <dev.jain@arm.com> >> + * >> + * Tests 4GB VA restriction for 32 bit process >> + */ >> + >> +#define _GNU_SOURCE >> +#include <stdio.h> >> +#include <stdlib.h> >> +#include <unistd.h> >> +#include <sys/mman.h> >> + >> +#include <linux/sizes.h> >> +#include <kselftest.h> >> + >> +#define MAP_CHUNK_SIZE SZ_1M >> +#define NR_CHUNKS_4G (SZ_1G / MAP_CHUNK_SIZE) * 4 /* prevent overflow */ >> + >> +static int validate_address_hint(void) >> +{ >> + char *ptr; >> + >> + ptr = mmap((void *) (1UL << 29), MAP_CHUNK_SIZE, PROT_READ | >> + PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); >> + >> + if (ptr == MAP_FAILED) >> + return 0; >> + >> + return 1; > Usually we return negative value instead of positive one which indicates > error situation. > >> +} >> + >> +int main(int argc, char *argv[]) >> +{ >> + char *ptr[NR_CHUNKS_4G + 3]; >> + char line[1000]; >> + const char *file_name; >> + int chunks; >> + FILE *file; >> + int i; >> + >> + ksft_print_header(); >> + ksft_set_plan(1); > There are multiple test cases. Instead of saying there is only 1 test. > There should be multiple ksft_test_result{_pass,_fail} statements for each > sub-tests. My initial idea was to treat this as a single logical test, as I am asserting the restriction on the number of chunks against the VMAs. I guess your approach is cleaner; thanks. > >> + >> + /* try allocation beyond 4 GB */ >> + for (i = 0; i < NR_CHUNKS_4G + 3; ++i) { >> + ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE, >> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); >> + >> + if (ptr[i] == MAP_FAILED) { >> + if (validate_address_hint()) >> + ksft_exit_fail_msg("VA exhaustion failed\n"); >> + break; >> + } >> + } >> + >> + chunks = i; >> + if (chunks >= NR_CHUNKS_4G) { >> + ksft_test_result_fail("mmapped chunks beyond 4GB\n"); >> + ksft_finished(); >> + } >> + >> + /* parse /proc/self/maps, confirm 32 bit VA mappings */ >> + file_name = "/proc/self/maps"; >> + file = fopen(file_name, "r"); >> + if (file == NULL) >> + ksft_exit_fail_msg("/proc/self/maps cannot be opened\n"); >> + >> + while (fgets(line, sizeof(line), file)) { >> + const char *whitespace_loc, *hyphen_loc; >> + >> + hyphen_loc = strchr(line, '-'); >> + whitespace_loc = strchr(line, ' '); >> + >> + if (!(hyphen_loc && whitespace_loc)) { >> + ksft_test_result_skip("Unexpected format"); >> + ksft_finished(); > I'm unable to follow as there are too many return statements. If you divide > the test into multiple sub-tests, you can skip/pass/fail each sub-test easily. > >> + } >> + >> + if ((hyphen_loc - line > 8) || >> + (whitespace_loc - hyphen_loc) > 9) { >> + ksft_test_result_fail("Memory map more than 32 bits\n"); >> + ksft_finished(); >> + } >> + } >> + >> + for (int i = 0; i < chunks; ++i) >> + munmap(ptr[i], MAP_CHUNK_SIZE); >> + >> + ksft_test_result_pass("Test\n"); >> + ksft_finished(); >> +}
On 4/10/24 09:45, Dev Jain wrote: > > On 4/7/24 02:53, Muhammad Usama Anjum wrote: >> On 4/5/24 1:44 PM, Dev Jain wrote: >>> This patch tests the 4GB VA restriction for 32-bit processes; it is >>> required >>> to test the compat layer, whether the kernel knows that it is >>> running a 32-bit >>> process or not. Chunks are allocated until the VA gets exhausted; >>> mmap must >>> fail beyond 4GB. This is asserted against the VA mappings found >>> in /proc/self/maps. >>> >>> Signed-off-by: Dev Jain <dev.jain@arm.com> >>> --- >>> tools/testing/selftests/arm/mm/compat_va.c | 94 >>> ++++++++++++++++++++++ >>> 1 file changed, 94 insertions(+) >>> create mode 100644 tools/testing/selftests/arm/mm/compat_va.c >>> >>> diff --git a/tools/testing/selftests/arm/mm/compat_va.c >>> b/tools/testing/selftests/arm/mm/compat_va.c >>> new file mode 100644 >>> index 000000000000..3a78f240bc87 >>> --- /dev/null >>> +++ b/tools/testing/selftests/arm/mm/compat_va.c >>> @@ -0,0 +1,94 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +/* >>> + * Copyright (C) 2024 ARM Limited >>> + * >>> + * Author : Dev Jain <dev.jain@arm.com> >>> + * >>> + * Tests 4GB VA restriction for 32 bit process >>> + */ >>> + >>> +#define _GNU_SOURCE >>> +#include <stdio.h> >>> +#include <stdlib.h> >>> +#include <unistd.h> >>> +#include <sys/mman.h> >>> + >>> +#include <linux/sizes.h> >>> +#include <kselftest.h> >>> + >>> +#define MAP_CHUNK_SIZE SZ_1M >>> +#define NR_CHUNKS_4G (SZ_1G / MAP_CHUNK_SIZE) * 4 /* prevent >>> overflow */ >>> + >>> +static int validate_address_hint(void) >>> +{ >>> + char *ptr; >>> + >>> + ptr = mmap((void *) (1UL << 29), MAP_CHUNK_SIZE, PROT_READ | >>> + PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); >>> + >>> + if (ptr == MAP_FAILED) >>> + return 0; >>> + >>> + return 1; >> Usually we return negative value instead of positive one which indicates >> error situation. >> >>> +} >>> + >>> +int main(int argc, char *argv[]) >>> +{ >>> + char *ptr[NR_CHUNKS_4G + 3]; >>> + char line[1000]; >>> + const char *file_name; >>> + int chunks; >>> + FILE *file; >>> + int i; >>> + >>> + ksft_print_header(); >>> + ksft_set_plan(1); >> There are multiple test cases. Instead of saying there is only 1 test. >> There should be multiple ksft_test_result{_pass,_fail} statements for >> each >> sub-tests. > > > My initial idea was to treat this as a single logical test, as I > > am asserting the restriction on the number of chunks against > > the VMAs. I guess your approach is cleaner; thanks. Thinking again, using a lot of return statements is in fact making the code easier to follow. If I just set a variable ret = 0/1 and use it to pass or fail, I'll have to unnecessarily use a lot of if/else statements. Take a look at the examples below. > >> >>> + >>> + /* try allocation beyond 4 GB */ >>> + for (i = 0; i < NR_CHUNKS_4G + 3; ++i) { >>> + ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE, >>> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); >>> + >>> + if (ptr[i] == MAP_FAILED) { >>> + if (validate_address_hint()) >>> + ksft_exit_fail_msg("VA exhaustion failed\n"); >>> + break; I will have to set ret value here, forcing two statements inside the if block. >>> + } >>> + } >>> + >>> + chunks = i; >>> + if (chunks >= NR_CHUNKS_4G) { >>> + ksft_test_result_fail("mmapped chunks beyond 4GB\n"); >>> + ksft_finished(); >>> + } >>> + >>> + /* parse /proc/self/maps, confirm 32 bit VA mappings */ >>> + file_name = "/proc/self/maps"; >>> + file = fopen(file_name, "r"); >>> + if (file == NULL) >>> + ksft_exit_fail_msg("/proc/self/maps cannot be opened\n"); >>> + I will have to set ret here, and enclose the below while statement inside an else block. In short, saving the value of ret will require if/else blocks if I were to use it in the end in ksft_test_result(). When I use ksft_exit statements, it is clear that a problem was spotted here, and there is no need to study the remaining code. >>> + while (fgets(line, sizeof(line), file)) { >>> + const char *whitespace_loc, *hyphen_loc; >>> + >>> + hyphen_loc = strchr(line, '-'); >>> + whitespace_loc = strchr(line, ' '); >>> + >>> + if (!(hyphen_loc && whitespace_loc)) { >>> + ksft_test_result_skip("Unexpected format"); >>> + ksft_finished(); >> I'm unable to follow as there are too many return statements. If you >> divide >> the test into multiple sub-tests, you can skip/pass/fail each >> sub-test easily. >> >>> + } >>> + >>> + if ((hyphen_loc - line > 8) || >>> + (whitespace_loc - hyphen_loc) > 9) { >>> + ksft_test_result_fail("Memory map more than 32 bits\n"); >>> + ksft_finished(); >>> + } >>> + } >>> + >>> + for (int i = 0; i < chunks; ++i) >>> + munmap(ptr[i], MAP_CHUNK_SIZE); >>> + >>> + ksft_test_result_pass("Test\n"); >>> + ksft_finished(); >>> +} > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/tools/testing/selftests/arm/mm/compat_va.c b/tools/testing/selftests/arm/mm/compat_va.c new file mode 100644 index 000000000000..3a78f240bc87 --- /dev/null +++ b/tools/testing/selftests/arm/mm/compat_va.c @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2024 ARM Limited + * + * Author : Dev Jain <dev.jain@arm.com> + * + * Tests 4GB VA restriction for 32 bit process + */ + +#define _GNU_SOURCE +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <sys/mman.h> + +#include <linux/sizes.h> +#include <kselftest.h> + +#define MAP_CHUNK_SIZE SZ_1M +#define NR_CHUNKS_4G (SZ_1G / MAP_CHUNK_SIZE) * 4 /* prevent overflow */ + +static int validate_address_hint(void) +{ + char *ptr; + + ptr = mmap((void *) (1UL << 29), MAP_CHUNK_SIZE, PROT_READ | + PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + + if (ptr == MAP_FAILED) + return 0; + + return 1; +} + +int main(int argc, char *argv[]) +{ + char *ptr[NR_CHUNKS_4G + 3]; + char line[1000]; + const char *file_name; + int chunks; + FILE *file; + int i; + + ksft_print_header(); + ksft_set_plan(1); + + /* try allocation beyond 4 GB */ + for (i = 0; i < NR_CHUNKS_4G + 3; ++i) { + ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + + if (ptr[i] == MAP_FAILED) { + if (validate_address_hint()) + ksft_exit_fail_msg("VA exhaustion failed\n"); + break; + } + } + + chunks = i; + if (chunks >= NR_CHUNKS_4G) { + ksft_test_result_fail("mmapped chunks beyond 4GB\n"); + ksft_finished(); + } + + /* parse /proc/self/maps, confirm 32 bit VA mappings */ + file_name = "/proc/self/maps"; + file = fopen(file_name, "r"); + if (file == NULL) + ksft_exit_fail_msg("/proc/self/maps cannot be opened\n"); + + while (fgets(line, sizeof(line), file)) { + const char *whitespace_loc, *hyphen_loc; + + hyphen_loc = strchr(line, '-'); + whitespace_loc = strchr(line, ' '); + + if (!(hyphen_loc && whitespace_loc)) { + ksft_test_result_skip("Unexpected format"); + ksft_finished(); + } + + if ((hyphen_loc - line > 8) || + (whitespace_loc - hyphen_loc) > 9) { + ksft_test_result_fail("Memory map more than 32 bits\n"); + ksft_finished(); + } + } + + for (int i = 0; i < chunks; ++i) + munmap(ptr[i], MAP_CHUNK_SIZE); + + ksft_test_result_pass("Test\n"); + ksft_finished(); +}
This patch tests the 4GB VA restriction for 32-bit processes; it is required to test the compat layer, whether the kernel knows that it is running a 32-bit process or not. Chunks are allocated until the VA gets exhausted; mmap must fail beyond 4GB. This is asserted against the VA mappings found in /proc/self/maps. Signed-off-by: Dev Jain <dev.jain@arm.com> --- tools/testing/selftests/arm/mm/compat_va.c | 94 ++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 tools/testing/selftests/arm/mm/compat_va.c