From patchwork Thu May 10 23:13:56 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 10392541 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0ED7D6053D for ; Thu, 10 May 2018 23:14:13 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EE32A28D96 for ; Thu, 10 May 2018 23:14:12 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DEBDC28DA9; Thu, 10 May 2018 23:14:12 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C58F428D96 for ; Thu, 10 May 2018 23:14:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751890AbeEJXOJ (ORCPT ); Thu, 10 May 2018 19:14:09 -0400 Received: from mail.kernel.org ([198.145.29.99]:58760 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750798AbeEJXOG (ORCPT ); Thu, 10 May 2018 19:14:06 -0400 Received: from garbanzo.do-not-panic.com (c-73-15-241-2.hsd1.ca.comcast.net [73.15.241.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 87EB521839; Thu, 10 May 2018 23:14:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1525994046; bh=2UxlwAdRX3uiTOJl0ghAvix92veCgy0nWrwzKvtAJ2I=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=2csDFq8rvl91mKoYvQJd1EZB/vlOtG9E3Sf7AgLDaPwtgF5JFYrNzBHcpoMXe6p9t LIA4MWoB6C6PajcLcxSz5TTg6gd6JJU6AUNzB3iHA+prKukLLRxPn7LCH+dQDM7sCk z+hBOXflnjLiYTcu9dxzjpYpMBqFkMspKyX7w8os= From: "Luis R. Rodriguez" To: viro@zeniv.linux.org.uk, darrick.wong@oracle.com, tytso@mit.edu, adilger.kernel@dilger.ca, clm@fb.com, jbacik@fb.com, dsterba@suse.com Cc: sandeen@sandeen.net, dhowells@redhat.com, fliu@suse.com, jack@suse.cz, jeffm@suse.com, nborisov@suse.com, jake.norris@suse.com, mtk.manpages@gmail.com, linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-ext4@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org, "Luis R. Rodriguez" Subject: [RFC v2 1/4] vfs: skip extra attributes check on removal for symlinks Date: Thu, 10 May 2018 16:13:56 -0700 Message-Id: <20180510231359.16899-2-mcgrof@kernel.org> X-Mailer: git-send-email 2.17.0 In-Reply-To: <20180510231359.16899-1-mcgrof@kernel.org> References: <20180510231359.16899-1-mcgrof@kernel.org> Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Linux filesystems cannot set extra file attributes (stx_attributes as per statx(2)) on a symbolic link. To set extra file attributes you issue ioctl(2) with FS_IOC_SETFLAGS, *all* ioctl(2) calls on a symbolic link yield EBADF. This is because ioctl(2) tries to obtain struct fd from the symbolic link file descriptor passed using fdget(), fdget() in turn always returns no file set when a file descriptor is open with O_PATH. As per symlink(2) O_PATH and O_NOFOLLOW must *always* be used when you want to get the file descriptor of a symbolic link, and this holds true for Linux, as such extra file attributes cannot possibly be set on symbolic links on Linux. Filesystems repair utilities should be updated to detect this as corruption and correct this, however, the VFS *does* respect these extra attributes on symlinks for removal. Since we cannot set these attributes we should special-case the immutable/append on delete for symlinks, this would be consistent with what we *do* allow on Linux for all filesystems. Since this is a clear sign to the VFS the filesystem must be corrupted filesystems can implement a verifier to catch this earlier. A generic warning issued for filesystems which don't implement these verifiers, and the VFS also lets users delete these pesky symlinks as otherwise users cannot get rid of them. The userspace utility chattr(1) cannot set these attributes on symlinks *and* other special files as well: # chattr -a symlink chattr: Operation not supported while reading flags on b The reason for this is different though. Refer to commit 023d111e92195 ("chattr.1.in: Document the compression attribute flags E, X, and ...") merged on e2fsprogs v1.28 since August 2002. This commit prevented issuing the ioctl() for symlink *and* special files in consideration for a buggy DRM driver where issuing lsattr on their special files crashed the system. For details refer to Debian bug 152029 [0]. You can craft your own tool to query the extra file attributes with the new shiny statx(2) tool, statx(2) will list the attributes if they were set for instance on a corrupt filesystem. However statx(2) is only used for *querying* -- not for setting the attributes. If you implement issuing your own ioctl() for FS_IOC_FSGETXATTR or FS_IOC_FSSETXATTR on special files (block, char, fifo) it will fail returning -1 and errno is set to ENOTTY (Inappropriate ioctl for device). The reason for this is different than for symlinks. For special files this fails on vfs_ioctl() when the filesystem f_op callbacks are not set for these special files: long vfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { int error = -ENOTTY; if (!filp->f_op->unlocked_ioctl) goto out; error = filp->f_op->unlocked_ioctl(filp, cmd, arg); if (error == -ENOIOCTLCMD) error = -ENOTTY; out: return error; } The same applies to PF_LOCAL named sockets. Since this varies by filesystem for special files, only make a special rule to respect the immutable and append attribute on symlinks. [0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=152029 Signed-off-by: Luis R. Rodriguez --- fs/namei.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index e861b409c241..23ebc14805dc 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2760,6 +2760,26 @@ int __check_sticky(struct inode *dir, struct inode *inode) } EXPORT_SYMBOL(__check_sticky); +/* Process extra file attributes only when they make sense */ +static bool may_delete_stx_attributes(struct inode *inode) +{ + /* + * The VFS does not allow setting append/immutable on symlinks. + * + * Filesystems can implement their own verifier which would avoid this + * generic splat, this generic splat is desirable if the respective + * filesystem repair utility won't implement a fix for this, otherwise + * users end up with a nagging dangling file which is impossible to + * fix in userspace. + */ + if (S_ISLNK(inode->i_mode)) { + WARN_ONCE((IS_APPEND(inode) || IS_IMMUTABLE(inode)), + "Immutable or append flag set on symlink. VFS does not allow this, must be a filesystem corruption. Allowing deletion though"); + } else if (IS_APPEND(inode) || IS_IMMUTABLE(inode)) + return false; + return true; +} + /* * Check whether we can remove a link victim from directory dir, check * whether the type of victim is right. @@ -2798,8 +2818,8 @@ static int may_delete(struct inode *dir, struct dentry *victim, bool isdir) if (IS_APPEND(dir)) return -EPERM; - if (check_sticky(dir, inode) || IS_APPEND(inode) || - IS_IMMUTABLE(inode) || IS_SWAPFILE(inode) || HAS_UNMAPPED_ID(inode)) + if (check_sticky(dir, inode) || !may_delete_stx_attributes(inode) || + IS_SWAPFILE(inode) || HAS_UNMAPPED_ID(inode)) return -EPERM; if (isdir) { if (!d_is_dir(victim))