From: Vivek Goyal <vgoyal@redhat.com>
To: viro@zeniv.linux.org.uk
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
virtio-fs@redhat.com, dwalsh@redhat.com, dgilbert@redhat.com,
vgoyal@redhat.com, christian.brauner@ubuntu.com,
casey.schaufler@intel.com, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, tytso@mit.edu, miklos@szeredi.hu,
gscrivan@redhat.com, bfields@redhat.com,
stephen.smalley.work@gmail.com, agruenba@redhat.com,
david@fromorbit.com
Subject: [PATCH v3 1/1] xattr: Allow user.* xattr on symlink and special files
Date: Thu, 2 Sep 2021 11:22:28 -0400 [thread overview]
Message-ID: <20210902152228.665959-2-vgoyal@redhat.com> (raw)
In-Reply-To: <20210902152228.665959-1-vgoyal@redhat.com>
Currently user.* xattr are not allowed on symlink and special files.
man xattr and recent discussion suggested that primary reason for this
restriction is how file permissions for symlinks and special files
are little different from regular files and directories.
For symlinks, they are world readable/writable and if user xattr were
to be permitted, it will allow unpriviliged users to dump a huge amount
of user.* xattrs on symlinks without any control. (I think quota control
still works with symlinks, just that quota is not typically deployed).
For special files, permissions typically control capability to read/write
from devices (and not necessarily from filesystem). So if a user can
write to device (/dev/null), does not necessarily mean it should be allowed
to write large number of user.* xattrs on the filesystem device node is
residing in.
This patch proposes to relax the restrictions a bit and allow file owner
or privileged user (CAP_FOWNER), to be able to read/write user.* xattrs
on symlink and special files.
Note, for special files, file mode bits represent permission to access
device and not necessarily permission to read/write xattrs.
Hence, inode_permission() is not called on special files and just
being owner (or CAP_FOWNER) is enough to read/write user extended
xattrs on special files.
LSM will still get a chance to allow/deny this operation as xattr
related security hooks are still called. (security_inode_setxattr(),
security_inode_getxattr(), security_inode_removexattr(),
security_inode_listxattr())
virtiofs daemon has a need to store user.* xatrrs on all the files
(including symlinks and special files), and currently that fails. This
patch should help.
Link: https://lore.kernel.org/linux-fsdevel/20210625191229.1752531-1-vgoyal@redhat.com/
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
fs/xattr.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/fs/xattr.c b/fs/xattr.c
index 5c8c5175b385..69be1681477f 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -120,13 +120,26 @@ xattr_permission(struct user_namespace *mnt_userns, struct inode *inode,
}
/*
- * In the user.* namespace, only regular files and directories can have
- * extended attributes. For sticky directories, only the owner and
- * privileged users can write attributes.
+ * In the user.* namespace, for symlinks and special files, only
+ * the owner and priviliged users can read/write attributes.
+ * For sticky directories, only the owner and privileged users can
+ * write attributes.
*/
if (!strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN)) {
- if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
- return (mask & MAY_WRITE) ? -EPERM : -ENODATA;
+ if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode)) {
+ if (!inode_owner_or_capable(mnt_userns, inode)) {
+ return (mask & MAY_WRITE) ? -EPERM : -ENODATA;
+ }
+ /*
+ * This is special file and file mode bits represent
+ * permission to access device and not
+ * necessarily permission to read/write xattrs.
+ * Hence do not call inode_permission() and return
+ * success.
+ */
+ if (!S_ISLNK(inode->i_mode))
+ return 0;
+ }
if (S_ISDIR(inode->i_mode) && (inode->i_mode & S_ISVTX) &&
(mask & MAY_WRITE) &&
!inode_owner_or_capable(mnt_userns, inode))
--
2.31.1
next prev parent reply other threads:[~2021-09-02 15:22 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-02 15:22 [PATCH v3 0/1] Relax restrictions on user.* xattr Vivek Goyal
2021-09-02 15:22 ` Vivek Goyal [this message]
2021-09-02 15:38 ` [PATCH 2/1] man-pages: xattr.7: Update text for user extended xattr behavior change Vivek Goyal
2021-09-02 15:43 ` [PATCH v3 0/1] Relax restrictions on user.* xattr Casey Schaufler
2021-09-02 17:05 ` Vivek Goyal
2021-09-02 17:42 ` Vivek Goyal
2021-09-02 18:55 ` Casey Schaufler
2021-09-02 20:06 ` Vivek Goyal
2021-09-02 22:34 ` Casey Schaufler
2021-09-03 15:26 ` Vivek Goyal
2021-09-03 18:49 ` Casey Schaufler
2021-09-06 7:45 ` [Virtio-fs] " Sergio Lopez
2021-09-06 14:55 ` Dr. David Alan Gilbert
2021-09-13 19:05 ` Casey Schaufler
2021-09-14 12:51 ` Vivek Goyal
2021-09-14 13:56 ` Casey Schaufler
2021-09-14 13:59 ` Bruce Fields
2021-09-14 14:32 ` Vivek Goyal
2021-09-14 15:01 ` Bruce Fields
2021-09-15 16:33 ` Dr. Greg
2021-09-02 15:47 ` [PATCH 3/1] xfstests: generic/062: Do not run on newer kernels Vivek Goyal
2021-09-03 4:55 ` Dave Chinner
2021-09-03 6:31 ` Andreas Gruenbacher
2021-09-03 6:56 ` Andreas Gruenbacher
2021-09-03 14:42 ` Bruce Fields
2021-09-03 15:43 ` Vivek Goyal
2021-09-03 15:50 ` Bruce Fields
2021-09-03 16:01 ` Casey Schaufler
2021-09-03 16:03 ` Vivek Goyal
2021-09-03 6:31 ` Zorro Lang
2021-09-02 15:50 ` [PATCH 4/1] xfstest: Add a new test to test xattr operations Vivek Goyal
2021-09-02 17:52 ` [PATCH v3 0/1] Relax restrictions on user.* xattr Andreas Gruenbacher
2021-09-02 18:48 ` Vivek Goyal
2021-09-02 19:19 ` Casey Schaufler
2021-09-06 14:39 ` Dr. David Alan Gilbert
2021-09-06 14:56 ` Miklos Szeredi
2021-09-07 21:40 ` Vivek Goyal
2021-09-08 7:37 ` Miklos Szeredi
2021-09-08 14:20 ` Eric W. Biederman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210902152228.665959-2-vgoyal@redhat.com \
--to=vgoyal@redhat.com \
--cc=agruenba@redhat.com \
--cc=bfields@redhat.com \
--cc=casey.schaufler@intel.com \
--cc=christian.brauner@ubuntu.com \
--cc=david@fromorbit.com \
--cc=dgilbert@redhat.com \
--cc=dwalsh@redhat.com \
--cc=gscrivan@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=selinux@vger.kernel.org \
--cc=stephen.smalley.work@gmail.com \
--cc=tytso@mit.edu \
--cc=viro@zeniv.linux.org.uk \
--cc=virtio-fs@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).