[Bug Report] Wrong value of __NR_userfaultfd in asm-generic/unistd.h

[Bug Report] Wrong value of __NR_userfaultfd in asm-generic/unistd.h

[Muhammad Usama Anjum]

Hi,

The asm-generic/unistd.h file has wrong __NR_userfaultfd syscall number which
doesn't even depend on the architecture. This has caused failure of a selftest
which was fixed recently [1]. 

grep -rnIF "#define __NR_userfaultfd"
tools/include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
arch/x86/include/generated/uapi/asm/unistd_32.h:374:#define __NR_userfaultfd 374
arch/x86/include/generated/uapi/asm/unistd_64.h:327:#define __NR_userfaultfd 323
arch/x86/include/generated/uapi/asm/unistd_x32.h:282:#define __NR_userfaultfd (__X32_SYSCALL_BIT + 323)
arch/arm/include/generated/uapi/asm/unistd-eabi.h:347:#define __NR_userfaultfd (__NR_SYSCALL_BASE + 388)
arch/arm/include/generated/uapi/asm/unistd-oabi.h:359:#define __NR_userfaultfd (__NR_SYSCALL_BASE + 388)
include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282

The number is dependent on the architecture. The above data shows that it
is different for different arch:
x86	374
x86_64	323
ARM     347/358

It seems include/uapi/asm-generic/unistd has wrong 282 value in it. Maybe I'm
missing some context.. Please have a look at it.

The __NR_userfaultfd was added to include/uapi/asm-generic/unistd.h in
09f7298100ea ("Subject: [PATCH] userfaultfd: register uapi generic syscall (aarch64)").

[1] https://lore.kernel.org/all/20240912103151.1520254-1-usama.anjum@collabora.com 

-- 
BR,
/Muhammad Usama Anjum

Re: [Bug Report] Wrong value of __NR_userfaultfd in asm-generic/unistd.h

[David Hildenbrand]

Am 21.10.24 um 08:48 schrieb Muhammad Usama Anjum:
> Hi,
> 
> The asm-generic/unistd.h file has wrong __NR_userfaultfd syscall number which
> doesn't even depend on the architecture. This has caused failure of a selftest
> which was fixed recently [1].
> 
> grep -rnIF "#define __NR_userfaultfd"
> tools/include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
> arch/x86/include/generated/uapi/asm/unistd_32.h:374:#define __NR_userfaultfd 374
> arch/x86/include/generated/uapi/asm/unistd_64.h:327:#define __NR_userfaultfd 323
> arch/x86/include/generated/uapi/asm/unistd_x32.h:282:#define __NR_userfaultfd (__X32_SYSCALL_BIT + 323)
> arch/arm/include/generated/uapi/asm/unistd-eabi.h:347:#define __NR_userfaultfd (__NR_SYSCALL_BASE + 388)
> arch/arm/include/generated/uapi/asm/unistd-oabi.h:359:#define __NR_userfaultfd (__NR_SYSCALL_BASE + 388)
> include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
> 
> The number is dependent on the architecture. The above data shows that it
> is different for different arch:
> x86	374
> x86_64	323
> ARM     347/358
> 
> It seems include/uapi/asm-generic/unistd has wrong 282 value in it. Maybe I'm
> missing some context.. Please have a look at it.
> 
> The __NR_userfaultfd was added to include/uapi/asm-generic/unistd.h in
> 09f7298100ea ("Subject: [PATCH] userfaultfd: register uapi generic syscall (aarch64)").

This is not specific to __NR_userfaultfd, just take a look at some of the other 
syscalls (e.g., __NR_membarrier).

Now, some of the files you list above are "generated". Doing it on a clean tree:

$ grep -rnIF "#define __NR_userfaultfd"
arch/arm64/include/asm/unistd32.h:789:#define __NR_userfaultfd 388
tools/include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282
include/uapi/asm-generic/unistd.h:681:#define __NR_userfaultfd 282


But now comes the tricky part: an architecture defines whether it wants to

(a) Use the asm-generic unistd.h
(b) Use a custom one

E.g.,

$ cat include/uapi/linux/unistd.h
/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
#ifndef _LINUX_UNISTD_H_
#define _LINUX_UNISTD_H_

/*
  * Include machine specific syscall numbers
  */
#include <asm/unistd.h>

#endif /* _LINUX_UNISTD_H_ */


For example on riscv arch/riscv/include/asm/unistd.h  will include 
arch/riscv/include/uapi/asm/unistd.h which will include "asm-generic/unistd.h".

If you follow the flow on x86, you'll find that it will not include that 
asm-generic one as default.

So the asm-generic variant only applies if an arch wants to do it in the generic 
way.

$ find tools -name unistd.h
tools/arch/x86/include/uapi/asm/unistd.h
tools/arch/arc/include/uapi/asm/unistd.h
tools/arch/riscv/include/uapi/asm/unistd.h
tools/arch/hexagon/include/uapi/asm/unistd.h
tools/arch/arm64/include/uapi/asm/unistd.h
tools/arch/loongarch/include/uapi/asm/unistd.h
tools/include/uapi/asm-generic/unistd.h
tools/include/nolibc/unistd.h

Consequently, the asm-generic one should never be used directly.

-- 
Cheers,

David / dhildenb

Re: [Bug Report] Wrong value of __NR_userfaultfd in asm-generic/unistd.h

[John Hubbard]

On 10/21/24 2:33 AM, David Hildenbrand wrote:
> Am 21.10.24 um 08:48 schrieb Muhammad Usama Anjum:
...
> But now comes the tricky part: an architecture defines whether it wants to
> 
> (a) Use the asm-generic unistd.h
> (b) Use a custom one
> 
> E.g.,
> 
> $ cat include/uapi/linux/unistd.h
> /* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> #ifndef _LINUX_UNISTD_H_
> #define _LINUX_UNISTD_H_
> 
> /*
>   * Include machine specific syscall numbers
>   */
> #include <asm/unistd.h>
> 
> #endif /* _LINUX_UNISTD_H_ */
> 
> 
> For example on riscv arch/riscv/include/asm/unistd.h  will include arch/riscv/include/uapi/asm/unistd.h which will include "asm-generic/unistd.h".
> 
> If you follow the flow on x86, you'll find that it will not include that asm-generic one as default.
> 
> So the asm-generic variant only applies if an arch wants to do it in the generic way.
> 
> $ find tools -name unistd.h
> tools/arch/x86/include/uapi/asm/unistd.h
> tools/arch/arc/include/uapi/asm/unistd.h
> tools/arch/riscv/include/uapi/asm/unistd.h
> tools/arch/hexagon/include/uapi/asm/unistd.h
> tools/arch/arm64/include/uapi/asm/unistd.h
> tools/arch/loongarch/include/uapi/asm/unistd.h
> tools/include/uapi/asm-generic/unistd.h
> tools/include/nolibc/unistd.h
> 
> Consequently, the asm-generic one should never be used directly.

ohhh, I think I may have inadvertently started this problem! Via a few
commits such as:

     a5c6bc590094 ("selftests/mm: remove local __NR_* definitions")

, which did things like this:

-#include <unistd.h>
+#include <asm-generic/unistd.h>

So it seems that it should have been:

-#include <unistd.h>
+#include <asm/unistd.h>

...and each arch's unistd.h needs to be checked to ensure that it is
up to date with all the symbols that kselftests need.

thanks,
-- 
John Hubbard