• Christian Brauner's avatar
    attr: block mode changes of symlinks · 5d1f903f
    Christian Brauner authored
    Changing the mode of symlinks is meaningless as the vfs doesn't take the
    mode of a symlink into account during path lookup permission checking.
    
    However, the vfs doesn't block mode changes on symlinks. This however,
    has lead to an untenable mess roughly classifiable into the following
    two categories:
    
    (1) Filesystems that don't implement a i_op->setattr() for symlinks.
    
        Such filesystems may or may not know that without i_op->setattr()
        defined, notify_change() falls back to simple_setattr() causing the
        inode's mode in the inode cache to be changed.
    
        That's a generic issue as this will affect all non-size changing
        inode attributes including ownership changes.
    
        Example: afs
    
    (2) Filesystems that fail with EOPNOTSUPP but change the mode of the
        symlink nonetheless.
    
        Some filesystems will happily update the mode of a symlink but still
        return EOPNOTSUPP. This is the biggest source of confusion for
        userspace.
    
        The EOPNOTSUPP in this case comes from POSIX ACLs. Specifically it
        comes from filesystems that call posix_acl_chmod(), e.g., btrfs via
    
            if (!err && attr->ia_valid & ATTR_MODE)
                    err = posix_acl_chmod(idmap, dentry, inode->i_mode);
    
        Filesystems including btrfs don't implement i_op->set_acl() so
        posix_acl_chmod() will report EOPNOTSUPP.
    
        When posix_acl_chmod() is called, most filesystems will have
        finished updating the inode.
    
        Perversely, this has the consequences that this behavior may depend
        on two kconfig options and mount options:
    
        * CONFIG_POSIX_ACL={y,n}
        * CONFIG_${FSTYPE}_POSIX_ACL={y,n}
        * Opt_acl, Opt_noacl
    
        Example: btrfs, ext4, xfs
    
    The only way to change the mode on a symlink currently involves abusing
    an O_PATH file descriptor in the following manner:
    
            fd = openat(-1, "/path/to/link", O_CLOEXEC | O_PATH | O_NOFOLLOW);
    
            char path[PATH_MAX];
            snprintf(path, sizeof(path), "/proc/self/fd/%d", fd);
            chmod(path, 0000);
    
    But for most major filesystems with POSIX ACL support such as btrfs,
    ext4, ceph, tmpfs, xfs and others this will fail with EOPNOTSUPP with
    the mode still updated due to the aforementioned posix_acl_chmod()
    nonsense.
    
    So, given that for all major filesystems this would fail with EOPNOTSUPP
    and that both glibc (cf. [1]) and musl (cf. [2]) outright block mode
    changes on symlinks we should just try and block mode changes on
    symlinks directly in the vfs and have a clean break with this nonsense.
    
    If this causes any regressions, we do the next best thing and fix up all
    filesystems that do return EOPNOTSUPP with the mode updated to not call
    posix_acl_chmod() on symlinks.
    
    But as usual, let's try the clean cut solution first. It's a simple
    patch that can be easily reverted. Not marking this for backport as I'll
    do that manually if we're reasonably sure that this works and there are
    no strong objections.
    
    We could block this in chmod_common() but it's more appropriate to do it
    notify_change() as it will also mean that we catch filesystems that
    change symlink permissions explicitly or accidently.
    
    Similar proposals were floated in the past as in [3] and [4] and again
    recently in [5]. There's also a couple of bugs about this inconsistency
    as in [6] and [7].
    
    Link: https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/fchmodat.c;h=99527a3727e44cb8661ee1f743068f108ec93979;hb=HEAD [1]
    Link: https://git.musl-libc.org/cgit/musl/tree/src/stat/fchmodat.c [2]
    Link: https://lore.kernel.org/all/20200911065733.GA31579@infradead.org [3]
    Link: https://sourceware.org/legacy-ml/libc-alpha/2020-02/msg00518.html [4]
    Link: https://lore.kernel.org/lkml/87lefmbppo.fsf@oldenburg.str.redhat.com [5]
    Link: https://sourceware.org/legacy-ml/libc-alpha/2020-02/msg00467.html [6]
    Link: https://sourceware.org/bugzilla/show_bug.cgi?id=14578#c17 [7]
    Reviewed-by: default avatarAleksa Sarai <cyphar@cyphar.com>
    Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
    Cc: stable@vger.kernel.org # please backport to all LTSes but not before v6.6-rc2 is tagged
    Suggested-by: default avatarChristoph Hellwig <hch@lst.de>
    Suggested-by: default avatarFlorian Weimer <fweimer@redhat.com>
    Message-Id: <20230712-vfs-chmod-symlinks-v2-1-08cfb92b61dd@kernel.org>
    Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
    5d1f903f
attr.c 15.4 KB