On Fri 26-04-24 18:20:14, Christian Göttsche wrote:
From: Christian Göttsche <cgzones@xxxxxxxxxxxxxx> Add the four syscalls setxattrat(), getxattrat(), listxattrat() and removexattrat(). Those can be used to operate on extended attributes, especially security related ones, either relative to a pinned directory or on a file descriptor without read access, avoiding a /proc/<pid>/fd/<fd> detour, requiring a mounted procfs. One use case will be setfiles(8) setting SELinux file contexts ("security.selinux") without race conditions and without a file descriptor opened with read access requiring SELinux read permission. Use the do_{name}at() pattern from fs/open.c. Pass the value of the extended attribute, its length, and for setxattrat(2) the command (XATTR_CREATE or XATTR_REPLACE) via an added struct xattr_args to not exceed six syscall arguments and not merging the AT_* and XATTR_* flags. Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
The patch looks good to me. Just a few nits below:
-static int path_setxattr(const char __user *pathname, +static int do_setxattrat(int dfd, const char __user *pathname, unsigned int at_flags,
Can we please stay within 80 columns (happens in multiple places in the patch)? I don't insist but it makes things easier to read in some setups so I prefer it.
@@ -852,13 +908,21 @@ listxattr(struct dentry *d, char __user *list, size_t size) return error; } -static ssize_t path_listxattr(const char __user *pathname, char __user *list, - size_t size, unsigned int lookup_flags) +static ssize_t do_listxattrat(int dfd, const char __user *pathname, char __user *list, + size_t size, int flags)
So I like how in previous syscalls you have 'at_flags', 'lookup_flags', and 'xattr_flags'. That makes things much easier to digest. Can you please stay with that convention here as well and call this argument 'at_flags'? Also I think the argument ordering like "dfd, pathname, at_flags, list, size" is more consistent with other syscalls you define.
@@ -870,16 +934,22 @@ static ssize_t path_listxattr(const char __user *pathname, char __user *list, return error; } +SYSCALL_DEFINE5(listxattrat, int, dfd, const char __user *, pathname, char __user *, list, + size_t, size, int, flags) +{ + return do_listxattrat(dfd, pathname, list, size, flags); +} +
Same comment as above - "flags" -> "at_flags" and reorder args please.
@@ -917,13 +987,21 @@ removexattr(struct mnt_idmap *idmap, struct dentry *d, return vfs_removexattr(idmap, d, kname); } -static int path_removexattr(const char __user *pathname, - const char __user *name, unsigned int lookup_flags) +static int do_removexattrat(int dfd, const char __user *pathname, + const char __user *name, int flags) {
Same comment as above - "flags" -> "at_flags" and reorder args please.
@@ -939,16 +1017,22 @@ static int path_removexattr(const char __user *pathname, return error; } +SYSCALL_DEFINE4(removexattrat, int, dfd, const char __user *, pathname, + const char __user *, name, int, flags) +{
Same comment as above - "flags" -> "at_flags" and reorder args please. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR