Skip to content

Conversation

@mikhailramalho
Copy link
Member

We define SYS_fchmodat2 on libc but the syscall is not available on old kernels, so prefer the SYS_fchmodat version when possible.

We define SYS_fchmodat2 on libc but the syscall is not available on
old kernels, so prefer the SYS_fchmodat version when possible.
@llvmbot
Copy link
Member

llvmbot commented May 3, 2025

@llvm/pr-subscribers-libc

Author: Mikhail R. Gadelha (mikhailramalho)

Changes

We define SYS_fchmodat2 on libc but the syscall is not available on old kernels, so prefer the SYS_fchmodat version when possible.


Full diff: https://github.com/llvm/llvm-project/pull/138427.diff

1 Files Affected:

  • (modified) libc/src/sys/stat/linux/chmod.cpp (+3-3)
diff --git a/libc/src/sys/stat/linux/chmod.cpp b/libc/src/sys/stat/linux/chmod.cpp
index 57d5bae6b8191..1b787e47e7c68 100644
--- a/libc/src/sys/stat/linux/chmod.cpp
+++ b/libc/src/sys/stat/linux/chmod.cpp
@@ -23,12 +23,12 @@ namespace LIBC_NAMESPACE_DECL {
 LLVM_LIBC_FUNCTION(int, chmod, (const char *path, mode_t mode)) {
 #ifdef SYS_chmod
   int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_chmod, path, mode);
-#elif defined(SYS_fchmodat2)
-  int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_fchmodat2, AT_FDCWD, path,
-                                              mode, 0, AT_SYMLINK_NOFOLLOW);
 #elif defined(SYS_fchmodat)
   int ret =
       LIBC_NAMESPACE::syscall_impl<int>(SYS_fchmodat, AT_FDCWD, path, mode, 0);
+#elif defined(SYS_fchmodat2)
+  int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_fchmodat2, AT_FDCWD, path,
+                                              mode, 0, AT_SYMLINK_NOFOLLOW);
 #else
 #error "chmod, fchmodat and fchmodat2 syscalls not available."
 #endif

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch is fine with me, but I'm a bit confused by the reasoning. If the fchmodat2 syscall isn't available from the kernel, shouldn't it also not be defined by the headers?

@mikhailramalho
Copy link
Member Author

Yeah, maybe it's a weird kernel shipped for the bpi-f3?

mgadelha@archlinux include $ uname -a
Linux archlinux 6.1.15-legacy-k1 #9 SMP PREEMPT Mon Aug 12 15:06:24 UTC 2024 riscv64 GNU/Linux
mgadelha@archlinux include $ pwd
/usr/include
mgadelha@archlinux include $ grep -r fchmodat2 *
asm/unistd_32.h:#define __NR_fchmodat2 452
asm/unistd_64.h:#define __NR_fchmodat2 452
asm-generic/unistd.h:#define __NR_fchmodat2 452
asm-generic/unistd.h:__SYSCALL(__NR_fchmodat2, sys_fchmodat2)
bits/syscall.h:#ifdef __NR_fchmodat2
bits/syscall.h:# define SYS_fchmodat2 __NR_fchmodat2
seccomp-syscalls.h:#define __SNR_fchmodat2			__NR_fchmodat2
valgrind/vki/vki-scnums-mips32-linux.h:#define __NR_fchmodat2                  (__NR_Linux + 452)
valgrind/vki/vki-scnums-shared-linux.h:#define __NR_fchmodat2		452

The defines are there, and we crash when trying to do the syscall.

@mikhailramalho mikhailramalho merged commit 230f332 into llvm:main May 5, 2025
18 checks passed
@mikhailramalho mikhailramalho deleted the libc-rv-chmod branch May 5, 2025 21:49
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
We define SYS_fchmodat2 on libc but the syscall is not available on old
kernels, so prefer the SYS_fchmodat version when possible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants