Skip to content

[libc] Swap order of syscall on chmod #138427

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 5, 2025

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