| Message ID | 20231129-idmap-fscap-refactor-v1-7-da5a26058a5b@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | fs: use type-safe uid representation for filesystem capabilities | expand |
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.
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.
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().
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.
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 --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);
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(+)