Re: [Bug 216834] New: fchmodat changes permission bits of symlink

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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;



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux