Re: [PATCH] selftests: mm: Fix build errors on armhf

[Jeff Xu <jeffxu@chromium.org>]

Hi Muhammad

On Fri, Aug 9, 2024 at 1:25 AM Muhammad Usama Anjum
<usama.anjum@collabora.com> wrote:
>
> The __NR_mmap isn't found on armhf. The mmap() is commonly available
> system call and its wrapper is presnet on all architectures. So it
> should be used directly. It solves problem for armhf and doesn't create
> problem for architectures as well. Remove sys_mmap() functions as they
> aren't doing anything else other than calling mmap(). There is no need
> to set errno = 0 manually as glibc always resets it.
>
The mseal_test should't have dependency on libc, and mmap() is
implemented by glibc, right ?

I just fixed a bug to switch mremap() to sys_mremap to address an
issue that different glibc version's behavior is slightly different
for mremap().

What is the reason that __NR_mmap not available in armhf ? (maybe it
is another name ?)  there must be a way to call syscall directly on
armhf, can we use that instead ?

Thanks
-Jeff


> For reference errors are as following:
>
>   CC       seal_elf
> seal_elf.c: In function 'sys_mmap':
> seal_elf.c:39:33: error: '__NR_mmap' undeclared (first use in this function)
>    39 |         sret = (void *) syscall(__NR_mmap, addr, len, prot,
>       |                                 ^~~~~~~~~
>
> mseal_test.c: In function 'sys_mmap':
> mseal_test.c:90:33: error: '__NR_mmap' undeclared (first use in this function)
>    90 |         sret = (void *) syscall(__NR_mmap, addr, len, prot,
>       |                                 ^~~~~~~~~
>
> Cc: stable@vger.kernel.org
> Fixes: 4926c7a52de7 ("selftest mm/mseal memory sealing")
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>  tools/testing/selftests/mm/mseal_test.c | 37 +++++++++----------------
>  tools/testing/selftests/mm/seal_elf.c   | 13 +--------
>  2 files changed, 14 insertions(+), 36 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/mseal_test.c b/tools/testing/selftests/mm/mseal_test.c
> index a818f010de479..bfcea5cf9a484 100644
> --- a/tools/testing/selftests/mm/mseal_test.c
> +++ b/tools/testing/selftests/mm/mseal_test.c
> @@ -81,17 +81,6 @@ static int sys_mprotect_pkey(void *ptr, size_t size, unsigned long orig_prot,
>         return sret;
>  }
>
> -static void *sys_mmap(void *addr, unsigned long len, unsigned long prot,
> -       unsigned long flags, unsigned long fd, unsigned long offset)
> -{
> -       void *sret;
> -
> -       errno = 0;
> -       sret = (void *) syscall(__NR_mmap, addr, len, prot,
> -               flags, fd, offset);
> -       return sret;
> -}
> -
>  static int sys_munmap(void *ptr, size_t size)
>  {
>         int sret;
> @@ -172,7 +161,7 @@ static void setup_single_address(int size, void **ptrOut)
>  {
>         void *ptr;
>
> -       ptr = sys_mmap(NULL, size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> +       ptr = mmap(NULL, size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>         *ptrOut = ptr;
>  }
>
> @@ -181,7 +170,7 @@ static void setup_single_address_rw(int size, void **ptrOut)
>         void *ptr;
>         unsigned long mapflags = MAP_ANONYMOUS | MAP_PRIVATE;
>
> -       ptr = sys_mmap(NULL, size, PROT_READ | PROT_WRITE, mapflags, -1, 0);
> +       ptr = mmap(NULL, size, PROT_READ | PROT_WRITE, mapflags, -1, 0);
>         *ptrOut = ptr;
>  }
>
> @@ -205,7 +194,7 @@ bool seal_support(void)
>         void *ptr;
>         unsigned long page_size = getpagesize();
>
> -       ptr = sys_mmap(NULL, page_size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> +       ptr = mmap(NULL, page_size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>         if (ptr == (void *) -1)
>                 return false;
>
> @@ -481,8 +470,8 @@ static void test_seal_zero_address(void)
>         int prot;
>
>         /* use mmap to change protection. */
> -       ptr = sys_mmap(0, size, PROT_NONE,
> -                       MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
> +       ptr = mmap(0, size, PROT_NONE,
> +                  MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
>         FAIL_TEST_IF_FALSE(ptr == 0);
>
>         size = get_vma_size(ptr, &prot);
> @@ -1209,8 +1198,8 @@ static void test_seal_mmap_overwrite_prot(bool seal)
>         }
>
>         /* use mmap to change protection. */
> -       ret2 = sys_mmap(ptr, size, PROT_NONE,
> -                       MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
> +       ret2 = mmap(ptr, size, PROT_NONE,
> +                   MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
>         if (seal) {
>                 FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
>                 FAIL_TEST_IF_FALSE(errno == EPERM);
> @@ -1240,8 +1229,8 @@ static void test_seal_mmap_expand(bool seal)
>         }
>
>         /* use mmap to expand. */
> -       ret2 = sys_mmap(ptr, size, PROT_READ,
> -                       MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
> +       ret2 = mmap(ptr, size, PROT_READ,
> +                   MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
>         if (seal) {
>                 FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
>                 FAIL_TEST_IF_FALSE(errno == EPERM);
> @@ -1268,8 +1257,8 @@ static void test_seal_mmap_shrink(bool seal)
>         }
>
>         /* use mmap to shrink. */
> -       ret2 = sys_mmap(ptr, 8 * page_size, PROT_READ,
> -                       MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
> +       ret2 = mmap(ptr, 8 * page_size, PROT_READ,
> +                   MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
>         if (seal) {
>                 FAIL_TEST_IF_FALSE(ret2 == MAP_FAILED);
>                 FAIL_TEST_IF_FALSE(errno == EPERM);
> @@ -1650,7 +1639,7 @@ static void test_seal_discard_ro_anon_on_filebacked(bool seal)
>         ret = fallocate(fd, 0, 0, size);
>         FAIL_TEST_IF_FALSE(!ret);
>
> -       ptr = sys_mmap(NULL, size, PROT_READ, mapflags, fd, 0);
> +       ptr = mmap(NULL, size, PROT_READ, mapflags, fd, 0);
>         FAIL_TEST_IF_FALSE(ptr != MAP_FAILED);
>
>         if (seal) {
> @@ -1680,7 +1669,7 @@ static void test_seal_discard_ro_anon_on_shared(bool seal)
>         int ret;
>         unsigned long mapflags = MAP_ANONYMOUS | MAP_SHARED;
>
> -       ptr = sys_mmap(NULL, size, PROT_READ, mapflags, -1, 0);
> +       ptr = mmap(NULL, size, PROT_READ, mapflags, -1, 0);
>         FAIL_TEST_IF_FALSE(ptr != (void *)-1);
>
>         if (seal) {
> diff --git a/tools/testing/selftests/mm/seal_elf.c b/tools/testing/selftests/mm/seal_elf.c
> index 7aa1366063e4e..d9f8ba8d5050b 100644
> --- a/tools/testing/selftests/mm/seal_elf.c
> +++ b/tools/testing/selftests/mm/seal_elf.c
> @@ -30,17 +30,6 @@ static int sys_mseal(void *start, size_t len)
>         return sret;
>  }
>
> -static void *sys_mmap(void *addr, unsigned long len, unsigned long prot,
> -       unsigned long flags, unsigned long fd, unsigned long offset)
> -{
> -       void *sret;
> -
> -       errno = 0;
> -       sret = (void *) syscall(__NR_mmap, addr, len, prot,
> -               flags, fd, offset);
> -       return sret;
> -}
> -
>  static inline int sys_mprotect(void *ptr, size_t size, unsigned long prot)
>  {
>         int sret;
> @@ -56,7 +45,7 @@ static bool seal_support(void)
>         void *ptr;
>         unsigned long page_size = getpagesize();
>
> -       ptr = sys_mmap(NULL, page_size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> +       ptr = mmap(NULL, page_size, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>         if (ptr == (void *) -1)
>                 return false;
>
> --
> 2.39.2
>