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

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

[Muhammad Usama Anjum]

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.

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

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

[Jeff Xu]

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
>

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

[Muhammad Usama Anjum]

On 8/14/24 3:29 AM, Jeff Xu wrote:
> 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 ?

It seems __NR_mmap syscall is deprecated for arm. Found this comment in
arch/arm/include/asm/unistd.h:
/*
 * The following syscalls are obsolete and no longer available for EABI:
 *  __NR_time
 *  __NR_umount
 *  __NR_stime
 *  __NR_alarm
 *  __NR_utime
 *  __NR_getrlimit
 *  __NR_select
 *  __NR_readdir
 *  __NR_mmap
 *  __NR_socketcall
 *  __NR_syscall
 *  __NR_ipc
 */

The glibc mmap() calls mmap2() these days by adjusting the parameters
internally. From man mmap:
C library/kernel differences:
This  page  describes the interface provided by the glibc mmap() wrapper
function.  Originally, this function invoked a system call of the same
name.  Since Linux 2.4, that system call has been superseded  by
mmap2(2), and nowadays the glibc mmap() wrapper function invokes
mmap2(2) with a suitably adjusted value for offset.

I'm not sure if behaviour of glibc mmap() and syscall mmap2() would be
same, but we should use glibc at most places which accounts for
different architectures correctly. Maybe the differences were only
present in case of mremap().

-- 
BR,
Muhammad Usama Anjum

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

[Jeff Xu]

Hi Muhammad

On Mon, Aug 19, 2024 at 3:05 AM Muhammad Usama Anjum
<Usama.Anjum@collabora.com> wrote:
>
> On 8/14/24 3:29 AM, Jeff Xu wrote:
> > 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 ?
>
> It seems __NR_mmap syscall is deprecated for arm. Found this comment in
> arch/arm/include/asm/unistd.h:
> /*
>  * The following syscalls are obsolete and no longer available for EABI:
>  *  __NR_time
>  *  __NR_umount
>  *  __NR_stime
>  *  __NR_alarm
>  *  __NR_utime
>  *  __NR_getrlimit
>  *  __NR_select
>  *  __NR_readdir
>  *  __NR_mmap
>  *  __NR_socketcall
>  *  __NR_syscall
>  *  __NR_ipc
>  */
>
> The glibc mmap() calls mmap2() these days by adjusting the parameters
> internally. From man mmap:
> C library/kernel differences:
> This  page  describes the interface provided by the glibc mmap() wrapper
> function.  Originally, this function invoked a system call of the same
> name.  Since Linux 2.4, that system call has been superseded  by
> mmap2(2), and nowadays the glibc mmap() wrapper function invokes
> mmap2(2) with a suitably adjusted value for offset.
>
> I'm not sure if behaviour of glibc mmap() and syscall mmap2() would be
> same, but we should use glibc at most places which accounts for
> different architectures correctly. Maybe the differences were only
> present in case of mremap().
>
We shouldn't use glibc to test mseal, mseal is a security feature, and
an attacker can access syscall directly, so the test needs to test
with as little layer as possible.

I think there are two options:
1> switch everything to use __NR_mmap2
2> switch to __NR_mmap2 only for ARM.

I'm not sure which one is more appropriate though.

Thanks
-Jeff



> --
> BR,
> Muhammad Usama Anjum
>

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

[Liam R. Howlett]

* Jeff Xu <jeffxu@chromium.org> [240910 10:15]:
> Hi Muhammad
> 
> On Mon, Aug 19, 2024 at 3:05 AM Muhammad Usama Anjum
> <Usama.Anjum@collabora.com> wrote:
> >
> > On 8/14/24 3:29 AM, Jeff Xu wrote:
> > > 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 ?
> >
> > It seems __NR_mmap syscall is deprecated for arm. Found this comment in
> > arch/arm/include/asm/unistd.h:
> > /*
> >  * The following syscalls are obsolete and no longer available for EABI:
> >  *  __NR_time
> >  *  __NR_umount
> >  *  __NR_stime
> >  *  __NR_alarm
> >  *  __NR_utime
> >  *  __NR_getrlimit
> >  *  __NR_select
> >  *  __NR_readdir
> >  *  __NR_mmap
> >  *  __NR_socketcall
> >  *  __NR_syscall
> >  *  __NR_ipc
> >  */
> >
> > The glibc mmap() calls mmap2() these days by adjusting the parameters
> > internally. From man mmap:
> > C library/kernel differences:
> > This  page  describes the interface provided by the glibc mmap() wrapper
> > function.  Originally, this function invoked a system call of the same
> > name.  Since Linux 2.4, that system call has been superseded  by
> > mmap2(2), and nowadays the glibc mmap() wrapper function invokes
> > mmap2(2) with a suitably adjusted value for offset.
> >
> > I'm not sure if behaviour of glibc mmap() and syscall mmap2() would be
> > same, but we should use glibc at most places which accounts for
> > different architectures correctly. Maybe the differences were only
> > present in case of mremap().
> >
> We shouldn't use glibc to test mseal, mseal is a security feature, and
> an attacker can access syscall directly, so the test needs to test
> with as little layer as possible.

This sounds like you are concerned about the use of a library
artificially reducing the attack surface that exists by bypassing the
library to access the syscall directly.

If you have an example of something that is restricted by the library
that can be used by the attacker, then we will need to roll our own
caller with wrappers so that this works on random archs.

It appears that the existing tests can use the library without reducing
the test coverage, so why would we maintain our own abstraction?

This patch is also upstream, so it is obviously not going to change at
this point.  I'm sure you are aware of that as you raised this concern
on the failed backport email [1].  Neither of these locations are the
right one for a discussion that you are trying to start.  It would be
better to produce an RFC patch and send it to the mm mailing list and cc
the people on this patch.

Thanks,
Liam

[1]. https://lore.kernel.org/all/CABi2SkV-FdDQy2bjDkpgpqz7hX7ybeTjCrUgUf6WcYqGkuxWMQ@mail.gmail.com/

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

[Jeff Xu]

On Mon, Aug 19, 2024 at 3:05 AM Muhammad Usama Anjum
<Usama.Anjum@collabora.com> wrote:
>
> On 8/14/24 3:29 AM, Jeff Xu wrote:
> > 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

What is armhf ?
is that arm64 ? I was able to build arm64 correctly.

-Jeff

> >> 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 ?
>
> It seems __NR_mmap syscall is deprecated for arm. Found this comment in
> arch/arm/include/asm/unistd.h:
> /*
>  * The following syscalls are obsolete and no longer available for EABI:
>  *  __NR_time
>  *  __NR_umount
>  *  __NR_stime
>  *  __NR_alarm
>  *  __NR_utime
>  *  __NR_getrlimit
>  *  __NR_select
>  *  __NR_readdir
>  *  __NR_mmap
>  *  __NR_socketcall
>  *  __NR_syscall
>  *  __NR_ipc
>  */
>
> The glibc mmap() calls mmap2() these days by adjusting the parameters
> internally. From man mmap:
> C library/kernel differences:
> This  page  describes the interface provided by the glibc mmap() wrapper
> function.  Originally, this function invoked a system call of the same
> name.  Since Linux 2.4, that system call has been superseded  by
> mmap2(2), and nowadays the glibc mmap() wrapper function invokes
> mmap2(2) with a suitably adjusted value for offset.
>
> I'm not sure if behaviour of glibc mmap() and syscall mmap2() would be
> same, but we should use glibc at most places which accounts for
> different architectures correctly. Maybe the differences were only
> present in case of mremap().
>
> --
> BR,
> Muhammad Usama Anjum
>

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

[Muhammad Usama Anjum]

On 9/14/24 3:33 AM, Jeff Xu wrote:
> On Mon, Aug 19, 2024 at 3:05 AM Muhammad Usama Anjum
> <Usama.Anjum@collabora.com> wrote:
>>
>> On 8/14/24 3:29 AM, Jeff Xu wrote:
>>> 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
> 
> What is armhf ?
> is that arm64 ? I was able to build arm64 correctly.
It is arm architecture. Use following toolchain with it:
make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf-

Please test your patches on it.

> 
> -Jeff
> 
>>>> 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 ?
>>
>> It seems __NR_mmap syscall is deprecated for arm. Found this comment in
>> arch/arm/include/asm/unistd.h:
>> /*
>>  * The following syscalls are obsolete and no longer available for EABI:
>>  *  __NR_time
>>  *  __NR_umount
>>  *  __NR_stime
>>  *  __NR_alarm
>>  *  __NR_utime
>>  *  __NR_getrlimit
>>  *  __NR_select
>>  *  __NR_readdir
>>  *  __NR_mmap
>>  *  __NR_socketcall
>>  *  __NR_syscall
>>  *  __NR_ipc
>>  */
>>
>> The glibc mmap() calls mmap2() these days by adjusting the parameters
>> internally. From man mmap:
>> C library/kernel differences:
>> This  page  describes the interface provided by the glibc mmap() wrapper
>> function.  Originally, this function invoked a system call of the same
>> name.  Since Linux 2.4, that system call has been superseded  by
>> mmap2(2), and nowadays the glibc mmap() wrapper function invokes
>> mmap2(2) with a suitably adjusted value for offset.
>>
>> I'm not sure if behaviour of glibc mmap() and syscall mmap2() would be
>> same, but we should use glibc at most places which accounts for
>> different architectures correctly. Maybe the differences were only
>> present in case of mremap().
>>
>> --
>> BR,
>> Muhammad Usama Anjum
>>

-- 
BR,
Muhammad Usama Anjum