2024-03-21 10:36:55

by Dev Jain

[permalink] [raw]
Subject: [PATCH] selftests/mm: Confirm VA exhaustion without reliance on correctness of mmap()

Currently, VA exhaustion is being checked by passing a hint to mmap() and
expecting it to fail. This patch makes a stricter test by successful write()
calls from /proc/self/maps to a dump file, confirming that a free chunk is
indeed not available.

Signed-off-by: Dev Jain <[email protected]>
---
Merge dependency: https://lore.kernel.org/all/[email protected]/

.../selftests/mm/virtual_address_range.c | 69 +++++++++++++++++++
1 file changed, 69 insertions(+)

diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c
index 7bcf8d48256a..31063613dfd9 100644
--- a/tools/testing/selftests/mm/virtual_address_range.c
+++ b/tools/testing/selftests/mm/virtual_address_range.c
@@ -12,6 +12,8 @@
#include <errno.h>
#include <sys/mman.h>
#include <sys/time.h>
+#include <fcntl.h>
+
#include "../kselftest.h"

/*
@@ -93,6 +95,69 @@ static int validate_lower_address_hint(void)
return 1;
}

+static int validate_complete_va_space(void)
+{
+ unsigned long start_addr, end_addr, prev_end_addr;
+ char line[400];
+ char prot[6];
+ FILE *file;
+ int fd;
+
+ fd = open("va_dump", O_CREAT | O_WRONLY, 0600);
+ unlink("va_dump");
+ if (fd < 0) {
+ ksft_test_result_skip("cannot create or open dump file\n");
+ ksft_finished();
+ }
+
+ file = fopen("/proc/self/maps", "r");
+ if (file == NULL)
+ ksft_exit_fail_msg("cannot open /proc/self/maps\n");
+
+ prev_end_addr = 0;
+ while (fgets(line, sizeof(line), file)) {
+ unsigned long hop;
+ int ret;
+
+ ret = sscanf(line, "%lx-%lx %s[rwxp-]",
+ &start_addr, &end_addr, prot);
+ if (ret != 3)
+ ksft_exit_fail_msg("sscanf failed, cannot parse\n");
+
+ /* end of userspace mappings; ignore vsyscall mapping */
+ if (start_addr & (1UL << 63))
+ return 0;
+
+ /* /proc/self/maps must have gaps less than 1GB only */
+ if (start_addr - prev_end_addr >= SZ_1GB)
+ return 1;
+
+ prev_end_addr = end_addr;
+
+ if (prot[0] != 'r')
+ continue;
+
+ /*
+ * Confirm whether MAP_CHUNK_SIZE chunk can be found or not.
+ * If write succeeds, no need to check MAP_CHUNK_SIZE - 1
+ * addresses after that. If the address was not held by this
+ * process, write would fail with errno set to EFAULT.
+ * Anyways, if write returns anything apart from 1, exit the
+ * program since that would mean a bug in /proc/self/maps.
+ */
+ hop = 0;
+ while (start_addr + hop < end_addr) {
+ if (write(fd, (void *)(start_addr + hop), 1) != 1)
+ return 1;
+ else
+ lseek(fd, 0, SEEK_SET);
+
+ hop += MAP_CHUNK_SIZE;
+ }
+ }
+ return 0;
+}
+
int main(int argc, char *argv[])
{
char *ptr[NR_CHUNKS_LOW];
@@ -135,6 +200,10 @@ int main(int argc, char *argv[])
validate_addr(hptr[i], 1);
}
hchunks = i;
+ if (validate_complete_va_space()) {
+ ksft_test_result_fail("BUG in mmap() or /proc/self/maps\n");
+ ksft_finished();
+ }

for (i = 0; i < lchunks; i++)
munmap(ptr[i], MAP_CHUNK_SIZE);
--
2.34.1



2024-03-21 21:51:59

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] selftests/mm: Confirm VA exhaustion without reliance on correctness of mmap()

On Thu, 21 Mar 2024 16:05:22 +0530 Dev Jain <[email protected]> wrote:

> Currently, VA exhaustion is being checked by passing a hint to mmap() and
> expecting it to fail. This patch makes a stricter test by successful write()
> calls from /proc/self/maps to a dump file, confirming that a free chunk is
> indeed not available.

What's wrong with the current approach?

2024-03-22 05:13:05

by Dev Jain

[permalink] [raw]
Subject: Re: [PATCH] selftests/mm: Confirm VA exhaustion without reliance on correctness of mmap()


On 3/22/24 03:21, Andrew Morton wrote:
> On Thu, 21 Mar 2024 16:05:22 +0530 Dev Jain <[email protected]> wrote:
>
>> Currently, VA exhaustion is being checked by passing a hint to mmap() and
>> expecting it to fail. This patch makes a stricter test by successful write()
>> calls from /proc/self/maps to a dump file, confirming that a free chunk is
>> indeed not available.
> What's wrong with the current approach?
While populating the lower VA space, mmap() fails because we have
exhausted the space.

Then, in validate_lower_address_hint(), because mmap() fails, we confirm
that we have

indeed exhausted the space. There is a circular logic involved here.

Assume that there is a bug in mmap(), also assume that it exists
independent of whether

you pass a hint address or not; that for some reason it is not able to
find a 1GB chunk.

My idea is to assert the exhaustion against some other method.


Also, in the following line in validate_complete_va_space():

        if (start_addr - prev_end_addr >= SZ_1GB)

I made a small error, I forgot to use MAP_CHUNK_SIZE instead of SZ_1GB.

2024-03-22 19:19:18

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] selftests/mm: Confirm VA exhaustion without reliance on correctness of mmap()

On Fri, 22 Mar 2024 10:42:47 +0530 Dev Jain <[email protected]> wrote:

>
> On 3/22/24 03:21, Andrew Morton wrote:
> > On Thu, 21 Mar 2024 16:05:22 +0530 Dev Jain <[email protected]> wrote:
> >
> >> Currently, VA exhaustion is being checked by passing a hint to mmap() and
> >> expecting it to fail. This patch makes a stricter test by successful write()
> >> calls from /proc/self/maps to a dump file, confirming that a free chunk is
> >> indeed not available.
> > What's wrong with the current approach?
> While populating the lower VA space, mmap() fails because we have
> exhausted the space.
>
> Then, in validate_lower_address_hint(), because mmap() fails, we confirm
> that we have
>
> indeed exhausted the space. There is a circular logic involved here.
>
> Assume that there is a bug in mmap(), also assume that it exists
> independent of whether
>
> you pass a hint address or not; that for some reason it is not able to
> find a 1GB chunk.
>
> My idea is to assert the exhaustion against some other method.

Thanks. I added the above to the changelog.

>
> Also, in the following line in validate_complete_va_space():
>
> ??? ??? if (start_addr - prev_end_addr >= SZ_1GB)
>
> I made a small error, I forgot to use MAP_CHUNK_SIZE instead of SZ_1GB.

And the preceding comment might need an edit. Please send an updated
patch?