public inbox for [email protected]
 help / color / mirror / Atom feed
From: Jan Kara <[email protected]>
To: [email protected]
Cc: [email protected], [email protected],
	[email protected],
	[email protected], [email protected],
	[email protected], [email protected],
	[email protected], [email protected],
	[email protected], [email protected],
	[email protected], [email protected],
	[email protected], [email protected],
	[email protected], [email protected],
	[email protected],
	Richard Henderson <[email protected]>,
	Ivan Kokshaysky <[email protected]>,
	Matt Turner <[email protected]>,
	Russell King <[email protected]>,
	Catalin Marinas <[email protected]>,
	Will Deacon <[email protected]>,
	Geert Uytterhoeven <[email protected]>,
	Michal Simek <[email protected]>,
	Thomas Bogendoerfer <[email protected]>,
	"James E.J. Bottomley" <[email protected]>,
	Helge Deller <[email protected]>,
	Michael Ellerman <[email protected]>,
	Nicholas Piggin <[email protected]>,
	Christophe Leroy <[email protected]>,
	"Aneesh Kumar K.V" <[email protected]>,
	"Naveen N. Rao" <[email protected]>,
	Heiko Carstens <[email protected]>,
	Vasily Gorbik <[email protected]>,
	Alexander Gordeev <[email protected]>,
	Christian Borntraeger <[email protected]>,
	Sven Schnelle <[email protected]>,
	Yoshinori Sato <[email protected]>,
	Rich Felker <[email protected]>,
	John Paul Adrian Glaubitz <[email protected]>,
	"David S. Miller" <[email protected]>,
	Andreas Larsson <[email protected]>,
	Andy Lutomirski <[email protected]>,
	Thomas Gleixner <[email protected]>,
	Ingo Molnar <[email protected]>, Borislav Petkov <[email protected]>,
	Dave Hansen <[email protected]>,
	"H. Peter Anvin" <[email protected]>, Chris Zankel <[email protected]>,
	Max Filippov <[email protected]>,
	Alexander Viro <[email protected]>,
	Christian Brauner <[email protected]>, Jan Kara <[email protected]>,
	Paul Moore <[email protected]>, Eric Paris <[email protected]>,
	Arnd Bergmann <[email protected]>, Jens Axboe <[email protected]>,
	Pavel Begunkov <[email protected]>,
	Peter Zijlstra <[email protected]>,
	Sohil Mehta <[email protected]>,
	Palmer Dabbelt <[email protected]>,
	Miklos Szeredi <[email protected]>,
	Nhat Pham <[email protected]>,
	Casey Schaufler <[email protected]>,
	Florian Fainelli <[email protected]>,
	Kees Cook <[email protected]>,
	Rick Edgecombe <[email protected]>,
	Mark Rutland <[email protected]>,
	[email protected]
Subject: Re: [PATCH v3 2/2] fs/xattr: add *at family syscalls
Date: Tue, 30 Apr 2024 12:09:02 +0200	[thread overview]
Message-ID: <20240430100902.iwmeszr2jzv4wyo7@quack3> (raw)
In-Reply-To: <[email protected]>

On Fri 26-04-24 18:20:14, Christian Göttsche wrote:
> From: Christian Göttsche <[email protected]>
> 
> 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 <[email protected]>

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 <[email protected]>
SUSE Labs, CR

      parent reply	other threads:[~2024-04-30 10:09 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-26 16:20 [PATCH v3 2/2] fs/xattr: add *at family syscalls Christian Göttsche
2024-04-26 16:20 ` [PATCH v3 1/2] fs: rename struct xattr_ctx to kernel_xattr_ctx Christian Göttsche
2024-04-30 10:09   ` Jan Kara
2024-04-30 12:40   ` Christian Brauner
2024-04-26 17:38 ` [PATCH v3 2/2] fs/xattr: add *at family syscalls Arnd Bergmann
2024-04-30 10:09 ` Jan Kara [this message]

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=20240430100902.iwmeszr2jzv4wyo7@quack3 \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /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