Re: [PATCH v3 2/5] selftests/mm: run_vmtests: remove sudo and conform to tap

On 2/1/24 5:04 PM, Ryan Roberts wrote:

On 25/01/2024 15:46, Muhammad Usama Anjum wrote:

...

Remove sudo as some test running environments may not have sudo available. Instead skip the test if root privileges aren't available in the test.

Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com

Changes since v1:

• Added this patch in v2

We are allocating 2*RLIMIT_MEMLOCK.rlim_max memory and mmap() isn't failing. This seems like true bug in the kernel. Even the root user shouldn't be able to allocate more memory than allowed MEMLOCKed memory. Any ideas?

---

tools/testing/selftests/mm/on-fault-limit.c | 36 ++++++++++----------- tools/testing/selftests/mm/run_vmtests.sh | 2 +- 2 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/mm/on-fault-limit.c b/tools/testing/selftests/mm/on-fault-limit.c index b5888d613f34e..0ea98ffab3589 100644 --- a/tools/testing/selftests/mm/on-fault-limit.c +++ b/tools/testing/selftests/mm/on-fault-limit.c @@ -5,40 +5,38 @@ #include <string.h> #include <sys/time.h> #include <sys/resource.h> +#include "../kselftest.h" -static int test_limit(void) +static void test_limit(void) {

• int ret = 1; struct rlimit lims; void *map;

• if (getrlimit(RLIMIT_MEMLOCK, &lims)) {

• perror("getrlimit");
  

• return ret;
  

• }

• if (getrlimit(RLIMIT_MEMLOCK, &lims))

• ksft_exit_fail_msg("getrlimit: %s\n", strerror(errno));
  

• if (mlockall(MCL_ONFAULT | MCL_FUTURE)) {

• perror("mlockall");
  

• return ret;
  

• }

• if (mlockall(MCL_ONFAULT | MCL_FUTURE))

• ksft_exit_fail_msg("mlockall: %s\n", strerror(errno));
  

map = mmap(NULL, 2 * lims.rlim_max, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_POPULATE, -1, 0);

• ksft_test_result(map == MAP_FAILED, "Failed mmap\n");
• if (map != MAP_FAILED)

• printf("mmap should have failed, but didn't\n");
  

• else {

• ret = 0;
  

  munmap(map, 2 * lims.rlim_max);
• }
• munlockall();
• return ret;

} int main(int argc, char **argv) {

• int ret = 0;

• ksft_print_header();
• ksft_set_plan(1);
• if (getuid())

• ksft_test_result_skip("Require root privileges to run\n");
  

I'd sent a patch to fix this behavior today. This test should run without root privileges. https://lore.kernel.org/all/20240201071307.592317-1-usama.anjum@collabora.co...

...

• else

• test_limit();
  

• ret += test_limit();
• return ret;

• ksft_finished();

} diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh index 246d53a5d7f28..e373d592dbf5c 100755 --- a/tools/testing/selftests/mm/run_vmtests.sh +++ b/tools/testing/selftests/mm/run_vmtests.sh @@ -291,7 +291,7 @@ echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages CATEGORY="compaction" run_test ./compaction_test -CATEGORY="mlock" run_test sudo -u nobody ./on-fault-limit +CATEGORY="mlock" run_test ./on-fault-limit

I think changing this is going to give unintended results. run_vmtests.sh must already be running as root. "sudo -u nobody" is deprivieging the test to run as nobody. The rlimit is not enforced for root so this test must run unprivileged to work. See man page for getrlimit():

Since Linux 2.6.9, no limits are placed on the amount of memory that a privileged process may lock, and this limit instead governs the amount of memory that an unprivileged process may lock

So I think the correct fix is actually to install sudo on your CI.

run_vmtests.sh is invoked without sudo with following: make -C tools/testing/selftests/mm run_tests

Installing sudo in rootfs wouldn't be trivial enough on the CI side. Alternatively, we can check if sudo is present before executing this test to avoid error that sudo isn't found.

...

CATEGORY="mmap" run_test ./map_populate