From c03c33ecef9e40b657c5dfa94d501ae101fae3de Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Sun, 2 Aug 2026 21:27:07 +1000 Subject: [PATCH] syscall: preserve ordinary mode when setgid is denied macOS's fchmodat(..., AT_SYMLINK_NOFOLLOW) returns EPERM and applies NOTHING when the requested setgid bit is ungrantable, so "rsync -a --chmod=D2750,F0640" exits 23 and leaves the destination at 0755/0700 where 3.4.4 leaves 0750. fchmod() on an already-open descriptor does the right thing: it succeeds, drops the setgid bit it cannot grant, and applies the ordinary bits. That fd path existed but was fenced behind "#if defined __linux__". Guard it on O_NOFOLLOW so every platform that can open a leaf without following a symlink uses it, and leave the fchmodat/fchmodat2 fallbacks under __linux__. Measured on macOS with an ungrantable group: fchmodat(2750, NOFOLLOW) EPERM, 0700 -> 0700 (dir and FIFO alike) fchmod(fd, 2750) ok, 0700 -> 0750 fchmodat(0750, NOFOLLOW) ok, 0700 -> 0750 A FIFO observed by the lstat takes the pathname call instead, then one retry without S_ISGID. Opening a FIFO -- even O_NONBLOCK -- makes this process a reader for as long as the descriptor lives, which wakes a writer blocked in open(O_WRONLY) and can cost it a SIGPIPE or the bytes it writes before we close; the third line above is why that is avoidable. This does not make the function FIFO-open-free: the type comes from the lstat, so a leaf swapped to a FIFO after it is still opened, and set_file_attrs() opens FIFOs elsewhere for ACL/xattr work. Closing that needs the open constrained to the observed type, which is tracked separately. The retry is a pathname call and does not pin the inode, so a leaf swapped for another object of the same name is chmod'd instead; AT_SYMLINK_NOFOLLOW still keeps it off a symlink's target, and the held parent fd still confines the ancestors, so the out-of-tree boundary is unaffected. Only S_ISGID is retried -- clearing S_ISUID too could discard a bit that was grantable when only setgid caused the failure. Linux keeps the fd-first order it has always had. The raced-to-a-symlink refusal after openat() also accepts EMLINK and EFTYPE. ELOOP is not universal for O_NOFOLLOW on a symlink -- FreeBSD documents EMLINK and NetBSD EFTYPE -- so those two silently skipped the refusal and fell through to the (still symlink-safe) pathname call. Verified on real macOS: the regression test FAILS on 93a67aa9 with rc=23 and modes 0755/0700, and PASSES here with 0750/0750. (cherry picked from commit 5f0f8f298e9148a14602e0242324f9fbfd801f2b) --- syscall.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 72 insertions(+), 8 deletions(-) diff --git a/syscall.c b/syscall.c index 79533664..c5d73cfb 100644 --- a/syscall.c +++ b/syscall.c @@ -1437,20 +1437,30 @@ int do_chmod(const char *path, mode_t mode) * attacker swapping the leaf to a symlink that fchmodat(...,0) would follow out * of the tree). * - * Never follows the leaf: a regular file, dir or FIFO is pinned via + * Never follows the leaf: a regular file or dir is pinned via * openat(O_NOFOLLOW) and chmod'd with fchmod() (leaf-safe, every kernel, and * fakeroot-wrappable unlike the raw fchmodat2() syscall); a symlink leaf is - * refused (ELOOP). Other types or an open failure fall to - * fchmodat(AT_SYMLINK_NOFOLLOW) (a real no-follow chmod on glibc>=2.32 / - * Linux>=6.6), then the raw fchmodat2() syscall. If no no-follow primitive - * exists we skip with a warning rather than follow the leaf. */ + * refused (ELOOP, or EMLINK/EFTYPE on the BSDs). Other types or an open + * failure fall to fchmodat(AT_SYMLINK_NOFOLLOW) (a real no-follow chmod on + * glibc>=2.32 / Linux>=6.6), then the raw fchmodat2() syscall. If no + * no-follow primitive exists we skip with a warning rather than follow the + * leaf. + * + * A FIFO takes the fd path on Linux and the pathname path elsewhere -- see the + * S_ISFIFO arm below for why, and for what that costs. Note the type used to + * choose between them comes from the lstat above, so a leaf swapped between + * that and the open is classified by what it WAS: an observed regular file or + * dir that becomes a FIFO is still opened. O_NOFOLLOW rejects symlinks, not + * type changes. Constraining the open to the observed type would close that; + * it is not done here. */ static int do_fchmodat_nofollow(int dfd, const char *name, mode_t mode) { #if defined AT_FDCWD && defined AT_SYMLINK_NOFOLLOW mode &= CHMOD_BITS; -# if defined __linux__ +# ifdef O_NOFOLLOW { STRUCT_STAT st; + int oflags = O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_NOCTTY; if (do_lstat_atfd(dfd, name, &st) < 0) return -1; if (S_ISLNK(st.st_mode)) { @@ -1458,18 +1468,72 @@ static int do_fchmodat_nofollow(int dfd, const char *name, mode_t mode) return -1; } if (S_ISREG(st.st_mode) || S_ISDIR(st.st_mode) || S_ISFIFO(st.st_mode)) { - int fd = openat(dfd, name, O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_NOCTTY | O_CLOEXEC); + int fd; +# ifndef __linux__ + /* Never open a FIFO here. Opening one -- even O_NONBLOCK -- + * makes this process a reader for as long as the descriptor + * lives, which wakes a writer blocked in open(O_WRONLY) and + * can cost it a SIGPIPE or the bytes it writes before we + * close. The pathname call reaches the same end state + * without that: it succeeds outright when the mode is + * grantable, and when macOS refuses an ungrantable setgid + * with EPERM (having applied nothing), asking again without + * that bit gives exactly what fchmod() would have -- it drops + * the bit it cannot grant and applies the ordinary ones. + * Measured on macOS: fchmodat(2750) EPERM leaving 0600, + * fchmodat(0750) ok giving 0750, for a FIFO and a directory + * alike. + * + * This is a pathname call, so unlike the descriptor path it + * does not pin the inode; a leaf swapped for another object + * of the same name is chmod'd instead. AT_SYMLINK_NOFOLLOW + * still keeps it off a symlink's target. That trade buys + * away the reader hazard, and only for FIFOs. + * + * Only S_ISGID is retried. An ungrantable S_ISUID would + * still fail where fchmod() would have cleared it, but + * setuid is meaningless on a FIFO and the behaviour is + * undemonstrated, so it is not coded for. + * + * Linux keeps the fd-first order it has always had. */ + if (S_ISFIFO(st.st_mode)) { + if (fchmodat(dfd, name, mode, AT_SYMLINK_NOFOLLOW) == 0) + return 0; + if (errno == EPERM && (mode & S_ISGID) + && fchmodat(dfd, name, mode & ~S_ISGID, + AT_SYMLINK_NOFOLLOW) == 0) + return 0; + return -1; + } +# endif +# ifdef O_CLOEXEC + oflags |= O_CLOEXEC; +# endif + fd = openat(dfd, name, oflags); if (fd >= 0) { int r = fchmod(fd, mode), e = errno; close(fd); errno = e; return r; } - if (errno == ELOOP) + /* A leaf swapped for a symlink between the lstat above and + * this open: refuse rather than fall through. The errno is + * not the same everywhere -- Linux/Solaris ELOOP, FreeBSD + * EMLINK, NetBSD EFTYPE. */ + if (errno == ELOOP +# ifdef EMLINK + || errno == EMLINK +# endif +# ifdef EFTYPE + || errno == EFTYPE +# endif + ) return -1; /* raced to a symlink: refuse */ /* otherwise (e.g. EACCES on an unreadable file) fall through */ } } +# endif +# if defined __linux__ { int r = fchmodat(dfd, name, mode, AT_SYMLINK_NOFOLLOW); if (r == 0)