On 7/9/2021 8:27 AM, Vivek Goyal wrote:

On Fri, Jul 09, 2021 at 11:19:15AM +0200, Christian Brauner wrote:

quoted

On Thu, Jul 08, 2021 at 01:57:38PM -0400, Vivek Goyal wrote:

quoted

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.

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 priviliged user (CAP_FOWNER), to be able to read/write user.* xattrs
on symlink and special files.

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/ (local)
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---

Seems reasonable and useful.
Acked-by: Christian Brauner <redacted>

One question, do all filesystem supporting xattrs deal with setting them
on symlinks/device files correctly?

Wrote a simple bash script to do setfattr/getfattr user.foo xattr on
symlink and device node on ext4, xfs and btrfs and it works fine.

How about nfs, tmpfs, overlayfs and/or some of the other less conventional
filesystems?

https://github.com/rhvgoyal/misc/blob/master/generic-programs/user-xattr-special-files.sh

I probably can add some more filesystems to test.

Thanks
Vivek

quoted

quoted

 fs/xattr.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index 5c8c5175b385..2f1855c8b620 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c

@@ -120,12 +120,14 @@ 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))
+		if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode) &&
+		    !inode_owner_or_capable(mnt_userns, inode))
 			return (mask & MAY_WRITE) ? -EPERM : -ENODATA;
 		if (S_ISDIR(inode->i_mode) && (inode->i_mode & S_ISVTX) &&
 		    (mask & MAY_WRITE) &&

-- 
2.25.4