diff mbox series

[07/16] fs: add inode operations to get/set/remove fscaps

Message ID 20231129-idmap-fscap-refactor-v1-7-da5a26058a5b@kernel.org (mailing list archive)
State Handled Elsewhere
Headers show
Series fs: use type-safe uid representation for filesystem capabilities | expand

Commit Message

Seth Forshee (DigitalOcean) Nov. 29, 2023, 9:50 p.m. UTC
Add inode operations for getting, setting and removing filesystem
capabilities rather than passing around raw xattr data. This provides
better type safety for ids contained within xattrs.

Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org>
---
 include/linux/fs.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Amir Goldstein Nov. 30, 2023, 5:32 a.m. UTC | #1
On Wed, Nov 29, 2023 at 11:51 PM Seth Forshee (DigitalOcean)
<sforshee@kernel.org> wrote:
>
> Add inode operations for getting, setting and removing filesystem
> capabilities rather than passing around raw xattr data. This provides
> better type safety for ids contained within xattrs.
>
> Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org>
> ---
>  include/linux/fs.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 98b7a7a8c42e..a0a77f67b999 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2002,6 +2002,11 @@ struct inode_operations {
>                                      int);
>         int (*set_acl)(struct mnt_idmap *, struct dentry *,
>                        struct posix_acl *, int);
> +       int (*get_fscaps)(struct mnt_idmap *, struct dentry *,
> +                         struct vfs_caps *);
> +       int (*set_fscaps)(struct mnt_idmap *, struct dentry *,
> +                         const struct vfs_caps *, int flags);
> +       int (*remove_fscaps)(struct mnt_idmap *, struct dentry *);
>         int (*fileattr_set)(struct mnt_idmap *idmap,
>                             struct dentry *dentry, struct fileattr *fa);
>         int (*fileattr_get)(struct dentry *dentry, struct fileattr *fa);
>

Please document in Documentation/filesystems/{vfs,locking}.rst

Thanks,
Amir.
Seth Forshee (DigitalOcean) Nov. 30, 2023, 3:36 p.m. UTC | #2
On Thu, Nov 30, 2023 at 07:32:19AM +0200, Amir Goldstein wrote:
> On Wed, Nov 29, 2023 at 11:51 PM Seth Forshee (DigitalOcean)
> <sforshee@kernel.org> wrote:
> >
> > Add inode operations for getting, setting and removing filesystem
> > capabilities rather than passing around raw xattr data. This provides
> > better type safety for ids contained within xattrs.
> >
> > Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org>
> > ---
> >  include/linux/fs.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 98b7a7a8c42e..a0a77f67b999 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -2002,6 +2002,11 @@ struct inode_operations {
> >                                      int);
> >         int (*set_acl)(struct mnt_idmap *, struct dentry *,
> >                        struct posix_acl *, int);
> > +       int (*get_fscaps)(struct mnt_idmap *, struct dentry *,
> > +                         struct vfs_caps *);
> > +       int (*set_fscaps)(struct mnt_idmap *, struct dentry *,
> > +                         const struct vfs_caps *, int flags);
> > +       int (*remove_fscaps)(struct mnt_idmap *, struct dentry *);
> >         int (*fileattr_set)(struct mnt_idmap *idmap,
> >                             struct dentry *dentry, struct fileattr *fa);
> >         int (*fileattr_get)(struct dentry *dentry, struct fileattr *fa);
> >
> 
> Please document in Documentation/filesystems/{vfs,locking}.rst

Done for v2.
Christian Brauner Dec. 1, 2023, 5:02 p.m. UTC | #3
On Wed, Nov 29, 2023 at 03:50:25PM -0600, Seth Forshee (DigitalOcean) wrote:
> Add inode operations for getting, setting and removing filesystem
> capabilities rather than passing around raw xattr data. This provides
> better type safety for ids contained within xattrs.
> 
> Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org>
> ---
>  include/linux/fs.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 98b7a7a8c42e..a0a77f67b999 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2002,6 +2002,11 @@ struct inode_operations {
>  				     int);
>  	int (*set_acl)(struct mnt_idmap *, struct dentry *,
>  		       struct posix_acl *, int);
> +	int (*get_fscaps)(struct mnt_idmap *, struct dentry *,
> +			  struct vfs_caps *);
> +	int (*set_fscaps)(struct mnt_idmap *, struct dentry *,
> +			  const struct vfs_caps *, int flags);

If it's really a flags argument, then unsigned int, please,
Reviewed-by: Christian Brauner <brauner@kernel.org>

> +	int (*remove_fscaps)(struct mnt_idmap *, struct dentry *);
>  	int (*fileattr_set)(struct mnt_idmap *idmap,
>  			    struct dentry *dentry, struct fileattr *fa);
>  	int (*fileattr_get)(struct dentry *dentry, struct fileattr *fa);

Ofc we managed to add get/set_foo() and bar_get/set().
Seth Forshee (DigitalOcean) Dec. 1, 2023, 5:38 p.m. UTC | #4
On Fri, Dec 01, 2023 at 06:02:55PM +0100, Christian Brauner wrote:
> On Wed, Nov 29, 2023 at 03:50:25PM -0600, Seth Forshee (DigitalOcean) wrote:
> > Add inode operations for getting, setting and removing filesystem
> > capabilities rather than passing around raw xattr data. This provides
> > better type safety for ids contained within xattrs.
> > 
> > Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org>
> > ---
> >  include/linux/fs.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 98b7a7a8c42e..a0a77f67b999 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -2002,6 +2002,11 @@ struct inode_operations {
> >  				     int);
> >  	int (*set_acl)(struct mnt_idmap *, struct dentry *,
> >  		       struct posix_acl *, int);
> > +	int (*get_fscaps)(struct mnt_idmap *, struct dentry *,
> > +			  struct vfs_caps *);
> > +	int (*set_fscaps)(struct mnt_idmap *, struct dentry *,
> > +			  const struct vfs_caps *, int flags);
> 
> If it's really a flags argument, then unsigned int, please,

This is the flags for setxattr, which is an int everywhere. Or almost
everywhere; I just noticed that it is actually an unsigned int in struct
xattr_ctx. But for consistency I think it makes sense to have it be an
int here too. Though maybe naming it setxattr_flags would be helpful for
clarity.
Christian Brauner Dec. 5, 2023, 11:50 a.m. UTC | #5
On Fri, Dec 01, 2023 at 11:38:33AM -0600, Seth Forshee (DigitalOcean) wrote:
> On Fri, Dec 01, 2023 at 06:02:55PM +0100, Christian Brauner wrote:
> > On Wed, Nov 29, 2023 at 03:50:25PM -0600, Seth Forshee (DigitalOcean) wrote:
> > > Add inode operations for getting, setting and removing filesystem
> > > capabilities rather than passing around raw xattr data. This provides
> > > better type safety for ids contained within xattrs.
> > > 
> > > Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org>
> > > ---
> > >  include/linux/fs.h | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > > index 98b7a7a8c42e..a0a77f67b999 100644
> > > --- a/include/linux/fs.h
> > > +++ b/include/linux/fs.h
> > > @@ -2002,6 +2002,11 @@ struct inode_operations {
> > >  				     int);
> > >  	int (*set_acl)(struct mnt_idmap *, struct dentry *,
> > >  		       struct posix_acl *, int);
> > > +	int (*get_fscaps)(struct mnt_idmap *, struct dentry *,
> > > +			  struct vfs_caps *);
> > > +	int (*set_fscaps)(struct mnt_idmap *, struct dentry *,
> > > +			  const struct vfs_caps *, int flags);
> > 
> > If it's really a flags argument, then unsigned int, please,
> 
> This is the flags for setxattr, which is an int everywhere. Or almost

Ah right. Ugh, we should clean that up but not necessarily in this
series.
diff mbox series

Patch

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 98b7a7a8c42e..a0a77f67b999 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2002,6 +2002,11 @@  struct inode_operations {
 				     int);
 	int (*set_acl)(struct mnt_idmap *, struct dentry *,
 		       struct posix_acl *, int);
+	int (*get_fscaps)(struct mnt_idmap *, struct dentry *,
+			  struct vfs_caps *);
+	int (*set_fscaps)(struct mnt_idmap *, struct dentry *,
+			  const struct vfs_caps *, int flags);
+	int (*remove_fscaps)(struct mnt_idmap *, struct dentry *);
 	int (*fileattr_set)(struct mnt_idmap *idmap,
 			    struct dentry *dentry, struct fileattr *fa);
 	int (*fileattr_get)(struct dentry *dentry, struct fileattr *fa);