The "locked-in-memory size" limit per process can be non-multiple of page_size. The mmap() fails if we try to allocate locked-in-memory with same size as the allowed limit if it isn't multiple of the page_size because mmap() rounds off the memory size to be allocated to next multiple of page_size.
Fix this by flooring the length to be allocated with mmap() to the previous multiple of the page_size.
Fixes: 76fe17ef588a ("secretmem: test: add basic selftest for memfd_secret(2)") Reported-by: "kernelci.org bot" bot@kernelci.org Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com --- tools/testing/selftests/mm/memfd_secret.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/mm/memfd_secret.c b/tools/testing/selftests/mm/memfd_secret.c index 957b9e18c729..9b298f6a04b3 100644 --- a/tools/testing/selftests/mm/memfd_secret.c +++ b/tools/testing/selftests/mm/memfd_secret.c @@ -62,6 +62,9 @@ static void test_mlock_limit(int fd) char *mem;
len = mlock_limit_cur; + if (len % page_size != 0) + len = (len/page_size) * page_size; + mem = mmap(NULL, len, prot, mode, fd, 0); if (mem == MAP_FAILED) { fail("unable to mmap secret memory\n");
On Thu, 14 Dec 2023 15:19:30 +0500 Muhammad Usama Anjum usama.anjum@collabora.com wrote:
The "locked-in-memory size" limit per process can be non-multiple of page_size. The mmap() fails if we try to allocate locked-in-memory with same size as the allowed limit if it isn't multiple of the page_size because mmap() rounds off the memory size to be allocated to next multiple of page_size.
Fix this by flooring the length to be allocated with mmap() to the previous multiple of the page_size.
I'd like to understand how this was noticed, what the ongoing effect might be, etc. To help decide which kernel version(s) need the patch.
Fixes: 76fe17ef588a ("secretmem: test: add basic selftest for memfd_secret(2)") Reported-by: "kernelci.org bot" bot@kernelci.org
Which is one of the reasons we're now placing a Closes: tag after a Reported-by:.
Hi Andrew,
On 12/15/23 12:40 AM, Andrew Morton wrote:
On Thu, 14 Dec 2023 15:19:30 +0500 Muhammad Usama Anjum usama.anjum@collabora.com wrote:
The "locked-in-memory size" limit per process can be non-multiple of page_size. The mmap() fails if we try to allocate locked-in-memory with same size as the allowed limit if it isn't multiple of the page_size because mmap() rounds off the memory size to be allocated to next multiple of page_size.
Fix this by flooring the length to be allocated with mmap() to the previous multiple of the page_size.
I'd like to understand how this was noticed, what the ongoing effect might be, etc. To help decide which kernel version(s) need the patch.
This was getting triggered on KernelCI regularly because of different ulimit settings which wasn't multiple of the page_size. Find logs here: https://linux.kernelci.org/test/plan/id/657654bd8e81e654fae13532/ The bug in was present from the time test was first added.
Fixes: 76fe17ef588a ("secretmem: test: add basic selftest for memfd_secret(2)") Reported-by: "kernelci.org bot" bot@kernelci.org
Which is one of the reasons we're now placing a Closes: tag after a Reported-by:.
I was looking for email report from KernelCI. But I didn't find it. Not sure if we can do something like following: Closes: https://linux.kernelci.org/test/plan/id/657654bd8e81e654fae13532/
On Thu, Dec 14, 2023 at 03:19:30PM +0500, Muhammad Usama Anjum wrote:
The "locked-in-memory size" limit per process can be non-multiple of page_size. The mmap() fails if we try to allocate locked-in-memory with same size as the allowed limit if it isn't multiple of the page_size because mmap() rounds off the memory size to be allocated to next multiple of page_size.
Fix this by flooring the length to be allocated with mmap() to the previous multiple of the page_size.
Fixes: 76fe17ef588a ("secretmem: test: add basic selftest for memfd_secret(2)") Reported-by: "kernelci.org bot" bot@kernelci.org Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com
tools/testing/selftests/mm/memfd_secret.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/mm/memfd_secret.c b/tools/testing/selftests/mm/memfd_secret.c index 957b9e18c729..9b298f6a04b3 100644 --- a/tools/testing/selftests/mm/memfd_secret.c +++ b/tools/testing/selftests/mm/memfd_secret.c @@ -62,6 +62,9 @@ static void test_mlock_limit(int fd) char *mem; len = mlock_limit_cur;
- if (len % page_size != 0)
len = (len/page_size) * page_size;
With mlock limit smaller than a page we get zero length here and mmap will fail with -EINVAL because of it. In this case I think we can just skip the first mmap and only check that mmaping more than mlock limit fails.
mem = mmap(NULL, len, prot, mode, fd, 0); if (mem == MAP_FAILED) { fail("unable to mmap secret memory\n"); -- 2.42.0
On 12/15/23 3:59 PM, Mike Rapoport wrote:
On Thu, Dec 14, 2023 at 03:19:30PM +0500, Muhammad Usama Anjum wrote:
The "locked-in-memory size" limit per process can be non-multiple of page_size. The mmap() fails if we try to allocate locked-in-memory with same size as the allowed limit if it isn't multiple of the page_size because mmap() rounds off the memory size to be allocated to next multiple of page_size.
Fix this by flooring the length to be allocated with mmap() to the previous multiple of the page_size.
Fixes: 76fe17ef588a ("secretmem: test: add basic selftest for memfd_secret(2)") Reported-by: "kernelci.org bot" bot@kernelci.org Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com
tools/testing/selftests/mm/memfd_secret.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/mm/memfd_secret.c b/tools/testing/selftests/mm/memfd_secret.c index 957b9e18c729..9b298f6a04b3 100644 --- a/tools/testing/selftests/mm/memfd_secret.c +++ b/tools/testing/selftests/mm/memfd_secret.c @@ -62,6 +62,9 @@ static void test_mlock_limit(int fd) char *mem; len = mlock_limit_cur;
- if (len % page_size != 0)
len = (len/page_size) * page_size;With mlock limit smaller than a page we get zero length here and mmap will fail with -EINVAL because of it.
This test has a initialization step in prepare() where it increases the limit to at least a page if it is less than a page. Hence we'll never get len = 0 here.
In this case I think we can just skip the first mmap and only check that mmaping more than mlock limit fails.
mem = mmap(NULL, len, prot, mode, fd, 0); if (mem == MAP_FAILED) { fail("unable to mmap secret memory\n"); -- 2.42.0
linux-kselftest-mirror@lists.linaro.org
-
Andrew Morton -
Mike Rapoport -
Muhammad Usama Anjum