Re: [RFC PATCH] f*xattr: allow O_PATH descriptors

[Andreas Dilger <adilger@dilger.ca>]

[-- Attachment #1: Type: text/plain, Size: 3276 bytes --]

On Jun 7, 2022, at 9:31 AM, Christian Göttsche <cgzones@googlemail.com> wrote:
> 
> From: Miklos Szeredi <mszeredi@redhat.com>
> 
> Support file descriptors obtained via O_PATH for extended attribute
> operations.
> 
> Extended attributes are for example used by SELinux for the security
> context of file objects. To avoid time-of-check-time-of-use issues while
> setting those contexts it is advisable to pin the file in question and
> operate on a file descriptor instead of the path name. This can be
> emulated in userspace via /proc/self/fd/NN [1] but requires a procfs,
> which might not be mounted e.g. inside of chroots, see[2].

Will this allow get/set xattrs directly on symlinks?  That is one problem
that we have with some of the xattrs that are inherited on symlinks, but
there is no way to change them.  Allowing setxattr directly on a symlink
would be very useful.

Cheers, Andreas

> [1]: https://github.com/SELinuxProject/selinux/commit/7e979b56fd2cee28f647376a7233d2ac2d12ca50
> [2]: https://github.com/SELinuxProject/selinux/commit/de285252a1801397306032e070793889c9466845
> 
> Original patch by Miklos Szeredi <mszeredi@redhat.com>
> https://patchwork.kernel.org/project/linux-fsdevel/patch/20200505095915.11275-6-mszeredi@redhat.com/
> 
>> While this carries a minute risk of someone relying on the property of
>> xattr syscalls rejecting O_PATH descriptors, it saves the trouble of
>> introducing another set of syscalls.
>> 
>> Only file->f_path and file->f_inode are accessed in these functions.
>> 
>> Current versions return EBADF, hence easy to detect the presense of
>> this feature and fall back in case it's missing.
> 
> CC: linux-api@vger.kernel.org
> CC: linux-man@vger.kernel.org
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
> fs/xattr.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xattr.c b/fs/xattr.c
> index e8dd03e4561e..16360ac4eb1b 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -656,7 +656,7 @@ SYSCALL_DEFINE5(lsetxattr, const char __user *, pathname,
> SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
> 		const void __user *,value, size_t, size, int, flags)
> {
> -	struct fd f = fdget(fd);
> +	struct fd f = fdget_raw(fd);
> 	int error = -EBADF;
> 
> 	if (!f.file)
> @@ -768,7 +768,7 @@ SYSCALL_DEFINE4(lgetxattr, const char __user *, pathname,
> SYSCALL_DEFINE4(fgetxattr, int, fd, const char __user *, name,
> 		void __user *, value, size_t, size)
> {
> -	struct fd f = fdget(fd);
> +	struct fd f = fdget_raw(fd);
> 	ssize_t error = -EBADF;
> 
> 	if (!f.file)
> @@ -844,7 +844,7 @@ SYSCALL_DEFINE3(llistxattr, const char __user *, pathname, char __user *, list,
> 
> SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
> {
> -	struct fd f = fdget(fd);
> +	struct fd f = fdget_raw(fd);
> 	ssize_t error = -EBADF;
> 
> 	if (!f.file)
> @@ -910,7 +910,7 @@ SYSCALL_DEFINE2(lremovexattr, const char __user *, pathname,
> 
> SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
> {
> -	struct fd f = fdget(fd);
> +	struct fd f = fdget_raw(fd);
> 	int error = -EBADF;
> 
> 	if (!f.file)
> --
> 2.36.1
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]