On Thu, Dec 22, 2022 at 10:06:13PM +0000, bugzilla-daemon@xxxxxxxxxx wrote: > https://bugzilla.kernel.org/show_bug.cgi?id=216834 > > Bug ID: 216834 > Summary: fchmodat changes permission bits of symlink > Product: File System > Version: 2.5 > Kernel Version: 5.19.11-1rodete1-amd64, > 5.15.41-android13-8-00205-gf1bf82c3dacd-ab8747247, > "latest" as of 2022-12-22 > Hardware: All > OS: Linux > Tree: Mainline > Status: NEW > Severity: normal > Priority: P1 > Component: Other > Assignee: fs_other@xxxxxxxxxxxxxxxxxxxx > Reporter: rprichard@xxxxxxxxxx > Regression: No > > Created attachment 303456 > --> https://bugzilla.kernel.org/attachment.cgi?id=303456&action=edit > modify-symlink.c > > I've noticed that I'm able to change the permission bits of a symlink by > invoking __NR_fchmodat on a /proc/self/fd path for a symlink opened with > O_PATH. On some filesystems (ext4, btrfs, and xfs), the syscall fails with > ENOTSUP, but the permission bits are still changed. On f2fs, on the other hand, > the syscall fails with ENOTSUP and the bits are unchanged. > > Using the attached modify-symlink.c, I see this output: > > -1 [Operation not supported] > lrw---x-w- 1 rprichard primarygroup 1 Dec 22 13:58 A -> B > -rw-r--r-- 1 rprichard primarygroup 0 Dec 22 13:58 B > > I see this behavior with the kernel on my gLinux (Debian) machine, as well as > various Android kernels. I showed the problem to jaegeuk@xxxxxxxxxx, who also > tested it on a recent kernel. Yeah, this is a known issue. Or at least I'm aware of it and it's a bit wonky as this is at the intersection of two issues. Wouldn't be fun if it wasn't: (1) /proc/self/fd/<nr> doesn't honor permission bits. For example, you can easily upgrade permission bits by reopening an fd via /proc/self/fd<nr>. This is well-known and next year we'll hopefully be able to further restrict this with upgrade masks specifiable in openat2(). Consequently, /proc/self/fd/<nr> easily let's you change permission bits and other stuff. It's not really like an fd. (2) Symlinks don't support xattrs and thus don't support POSIX ACLs. If you change the mode of a file and the file has POSIX ACLs then the permission mask stored in the POSIX ACLs needs to be updated. Since symlinks don't support POSIX ACLs this is where it fails and this is where the EOPNOTSUPP comes from. For example, xfs keeps changing POSIX ACLs out of the transaction path. IOW, it calls posix_acl_chmod() after it has already updated the inode and completed the transaction. The same behavior exists for ext4 and btrfs. In a way, your chmod() is successful it's just that updating POSIX ACLs fails which is ok since symlinks don't support them. One way to improve this situation is to just make posix_acl_chmod() not bother reporting errors for acls on symlinks instead of reporting a failure. IOW, diff --git a/fs/posix_acl.c b/fs/posix_acl.c index 74dc0f571dc9..a9246934a4ca 100644 --- a/fs/posix_acl.c +++ b/fs/posix_acl.c @@ -594,7 +594,7 @@ int struct posix_acl *acl; int ret = 0; - if (!IS_POSIXACL(inode)) + if (S_ISLNK(mode) || !IS_POSIXACL(dir)) return 0; if (!inode->i_op->set_acl) return -EOPNOTSUPP;
