[PATCH] syscalls: fix syscall macros for newfstat/newfstatat

[PATCH] syscalls: fix syscall macros for newfstat/newfstatat

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The __NR_newfstat and __NR_newfstatat macros accidentally got renamed
in the conversion to the syscall.tbl format, dropping the 'new' portion
of the name.

In an unrelated change, the two syscalls are no longer architecture
specific but are once more defined on all 64-bit architectures, so the
'newstat' ABI keyword can be dropped from the table as a simplification.

Fixes: Fixes: 4fe53bf2ba0a ("syscalls: add generic scripts/syscall.tbl")
Closes: https://lore.kernel.org/lkml/838053e0-b186-4e9f-9668-9a3384a71f23@app.fastmail.com/T/#t
Reported-by: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm64/kernel/Makefile.syscalls     | 2 +-
 arch/loongarch/kernel/Makefile.syscalls | 3 ++-
 arch/riscv/kernel/Makefile.syscalls     | 2 +-
 scripts/syscall.tbl                     | 4 ++--
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kernel/Makefile.syscalls b/arch/arm64/kernel/Makefile.syscalls
index 3cfafd003b2d..0542a718871a 100644
--- a/arch/arm64/kernel/Makefile.syscalls
+++ b/arch/arm64/kernel/Makefile.syscalls
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
 syscall_abis_32 +=
-syscall_abis_64 += renameat newstat rlimit memfd_secret
+syscall_abis_64 += renameat rlimit memfd_secret
 
 syscalltbl = arch/arm64/tools/syscall_%.tbl
diff --git a/arch/loongarch/kernel/Makefile.syscalls b/arch/loongarch/kernel/Makefile.syscalls
index 523bb411a3bc..ab7d9baa2915 100644
--- a/arch/loongarch/kernel/Makefile.syscalls
+++ b/arch/loongarch/kernel/Makefile.syscalls
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 
-syscall_abis_64 += newstat
+# No special ABIs on loongarch so far
+syscall_abis_64 +=
diff --git a/arch/riscv/kernel/Makefile.syscalls b/arch/riscv/kernel/Makefile.syscalls
index 52087a023b3d..9668fd1faf60 100644
--- a/arch/riscv/kernel/Makefile.syscalls
+++ b/arch/riscv/kernel/Makefile.syscalls
@@ -1,4 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 
 syscall_abis_32 += riscv memfd_secret
-syscall_abis_64 += riscv newstat rlimit memfd_secret
+syscall_abis_64 += riscv rlimit memfd_secret
diff --git a/scripts/syscall.tbl b/scripts/syscall.tbl
index 797e20ea99a2..4586a18dfe9b 100644
--- a/scripts/syscall.tbl
+++ b/scripts/syscall.tbl
@@ -98,9 +98,9 @@
 77	common	tee				sys_tee
 78	common	readlinkat			sys_readlinkat
 79	stat64	fstatat64			sys_fstatat64
-79	newstat	fstatat				sys_newfstatat
+79	64	newfstatat			sys_newfstatat
 80	stat64	fstat64				sys_fstat64
-80	newstat	fstat				sys_newfstat
+80	64	newfstat			sys_newfstat
 81	common	sync				sys_sync
 82	common	fsync				sys_fsync
 83	common	fdatasync			sys_fdatasync
-- 
2.39.2

Re: [PATCH] syscalls: fix syscall macros for newfstat/newfstatat

[Rudi Heitbaum]

On Thu, Aug 01, 2024 at 02:32:33PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The __NR_newfstat and __NR_newfstatat macros accidentally got renamed
> in the conversion to the syscall.tbl format, dropping the 'new' portion
> of the name.
> 
> In an unrelated change, the two syscalls are no longer architecture
> specific but are once more defined on all 64-bit architectures, so the
> 'newstat' ABI keyword can be dropped from the table as a simplification.
> 
> Fixes: Fixes: 4fe53bf2ba0a ("syscalls: add generic scripts/syscall.tbl")
> Closes: https://lore.kernel.org/lkml/838053e0-b186-4e9f-9668-9a3384a71f23@app.fastmail.com/T/#t
> Reported-by: Florian Weimer <fweimer@redhat.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Hi Arnd,

With this patch to kernel 6.11-rc1 the error whilst compiling gcc-14.2 for
aarch64 with libsanitizer has changed from '__NR_newfstatat' to
'__NR_fstat', so some change, but still fail8ng.

sanitizer_syscall_linux_aarch64.inc:13:23: error: '__NR_newfstatat' was not declared in this scope
   13 | #define SYSCALL(name) __NR_ ## name
      |                       ^~~~~

sanitizer_syscall_linux_aarch64.inc:13:23: error: '__NR_fstat' was not declared in this scope
   13 | #define SYSCALL(name) __NR_ ## name
      |                       ^~~~~

Regards
Rudi

Re: [PATCH] syscalls: fix syscall macros for newfstat/newfstatat

[Jakub Jelinek]

On Sat, Aug 03, 2024 at 05:14:41AM +0000, Rudi Heitbaum wrote:
> With this patch to kernel 6.11-rc1 the error whilst compiling gcc-14.2 for
> aarch64 with libsanitizer has changed from '__NR_newfstatat' to
> '__NR_fstat', so some change, but still fail8ng.
> 
> sanitizer_syscall_linux_aarch64.inc:13:23: error: '__NR_newfstatat' was not declared in this scope
>    13 | #define SYSCALL(name) __NR_ ## name
>       |                       ^~~~~
> 
> sanitizer_syscall_linux_aarch64.inc:13:23: error: '__NR_fstat' was not declared in this scope
>    13 | #define SYSCALL(name) __NR_ ## name
>       |                       ^~~~~

Probably it would be useful to check
echo '#include <asm/unistd.h>' | gcc -E -dD -xc - | grep '#define __NR_' | sort
for all arches between 6.10 and the latest git, diff them and resolve any
unintended differences.

	Jakub

Re: [PATCH] syscalls: fix syscall macros for newfstat/newfstatat

[Arnd Bergmann]

On Sat, Aug 3, 2024, at 10:00, Jakub Jelinek wrote:
> On Sat, Aug 03, 2024 at 05:14:41AM +0000, Rudi Heitbaum wrote:
>> With this patch to kernel 6.11-rc1 the error whilst compiling gcc-14.2 for
>> aarch64 with libsanitizer has changed from '__NR_newfstatat' to
>> '__NR_fstat', so some change, but still fail8ng.
>> 
>> sanitizer_syscall_linux_aarch64.inc:13:23: error: '__NR_newfstatat' was not declared in this scope
>>    13 | #define SYSCALL(name) __NR_ ## name
>>       |                       ^~~~~
>> 
>> sanitizer_syscall_linux_aarch64.inc:13:23: error: '__NR_fstat' was not declared in this scope
>>    13 | #define SYSCALL(name) __NR_ ## name
>>       |                       ^~~~~
>

Thanks for the report!

Yes, I think I messed it up again by changing both fstatat and
fstat. Our internal names in the kernel are a bit inconsistent
and I failed to realize that only newfstatat uses the "new" name
in the user-facing macro.

> Probably it would be useful to check
> echo '#include <asm/unistd.h>' | gcc -E -dD -xc - | grep '#define __NR_' | sort
> for all arches between 6.10 and the latest git, diff them and resolve any
> unintended differences.

Right, I should have done that before the original series really:
I spent a lot of time validating the kernel's internal changes for
consistency (which found a dozen bugs that were unrelated to my
series) but missed the unintended changes to the external header
contents.

I'll do that now and send another fixup.

    Arnd

Re: [PATCH] syscalls: fix syscall macros for newfstat/newfstatat

[Jakub Jelinek]

On Sat, Aug 03, 2024 at 10:12:47AM +0200, Arnd Bergmann wrote:
> > Probably it would be useful to check
> > echo '#include <asm/unistd.h>' | gcc -E -dD -xc - | grep '#define __NR_' | sort
> > for all arches between 6.10 and the latest git, diff them and resolve any
> > unintended differences.
> 
> Right, I should have done that before the original series really:
> I spent a lot of time validating the kernel's internal changes for
> consistency (which found a dozen bugs that were unrelated to my
> series) but missed the unintended changes to the external header
> contents.
> 
> I'll do that now and send another fixup.

I've done 6.10 to 6.11-rc2 <asm/unistd.h> comparison just for the Fedora
arches (x86_64, aarch64, ppc64le, s390x, i686).  Full details in
https://bugzilla.redhat.com/show_bug.cgi?id=2301919#c8
On i686, ppc64le and s390x there are no changes.
On x86_64
#define __NR_uretprobe 335
has been added, perhaps that is intentional, haven't checked.
On aarch64 when going just after __NR_ defined macros and their values,
I see:
#define __NR_nfsservctl 42
#define __NR_fstat 80
#define __NR_arch_specific_syscall 244
#define __NR_syscalls 463
removal and
#define __NR_newfstat 80
and addition and then
#define __NR3264_fcntl 25
#define __NR3264_statfs 43
#define __NR3264_fstatfs 44
#define __NR3264_truncate 45
#define __NR3264_ftruncate 46
#define __NR3264_lseek 62
#define __NR3264_sendfile 71
#define __NR3264_fstatat 79
#define __NR3264_fstat 80
#define __NR3264_mmap 222
#define __NR3264_fadvise64 223
macros are removed as well (let's hope it is an implementation detail and
nothing uses those macros, but some search would be helpful).

	Jakub

Re: [PATCH] syscalls: fix syscall macros for newfstat/newfstatat

[Arnd Bergmann]

On Mon, Aug 5, 2024, at 21:53, Jakub Jelinek wrote:
> On Sat, Aug 03, 2024 at 10:12:47AM +0200, Arnd Bergmann wrote:
>> > Probably it would be useful to check
>> > echo '#include <asm/unistd.h>' | gcc -E -dD -xc - | grep '#define __NR_' | sort
>> > for all arches between 6.10 and the latest git, diff them and resolve any
>> > unintended differences.
>> 
>> Right, I should have done that before the original series really:
>> I spent a lot of time validating the kernel's internal changes for
>> consistency (which found a dozen bugs that were unrelated to my
>> series) but missed the unintended changes to the external header
>> contents.
>> 
>> I'll do that now and send another fixup.
>
> I've done 6.10 to 6.11-rc2 <asm/unistd.h> comparison just for the Fedora
> arches (x86_64, aarch64, ppc64le, s390x, i686).  Full details in
> https://bugzilla.redhat.com/show_bug.cgi?id=2301919#c8
> On i686, ppc64le and s390x there are no changes.
> On x86_64
> #define __NR_uretprobe 335

This was a bit of a wild ride, but I think this number one should be
final, and only exist on x86-64.

> has been added, perhaps that is intentional, haven't checked.
> On aarch64 when going just after __NR_ defined macros and their values,
> I see:
> #define __NR_nfsservctl 42
> #define __NR_fstat 80

__NR_fstat was a very embarrassing bug that I introduced
while trying to fix __NR_newfstatat. I got confused by the
the way that the kernel internally uses newfstat() and
newfstatat() as the name, while the only macro name
for fstat() is missing the 'new'.

I also added a patch to add back __NR_nfsservctl to avoid
the unintended change, but this one is less clear since we
already dropped that macro on some architectures while
converting them to the new format but left the macro on
other architectures.

The syscall itself was removed in linux-3.2.

> #define __NR_arch_specific_syscall 244
> #define __NR_syscalls 463
> #define __NR3264_fcntl 25
> #define __NR3264_statfs 43
> #define __NR3264_fstatfs 44
> #define __NR3264_truncate 45
> #define __NR3264_ftruncate 46
> #define __NR3264_lseek 62
> #define __NR3264_sendfile 71
> #define __NR3264_fstatat 79
> #define __NR3264_fstat 80
> #define __NR3264_mmap 222
> #define __NR3264_fadvise64 223
> macros are removed as well (let's hope it is an implementation detail and
> nothing uses those macros, but some search would be helpful).

I did a Debian codesearch search for these and did not find
anything other than a couple of language bindings that copied
these from the kernel headers but nothing actually using them.

All of the above are implementation details that only
existed on arm64/riscv/loongarch/arc/openrisc/xtensa/nios2
because they were still using the old unistd.h format, and
they don't exist on any architecture that generates the
files from syscall.tbl.

      Arnd