Re: [Cluster-devel] [PATCH 2/2] gfs2: Add trusted xattr support
On 05/02/2021 19:04, Bob Peterson wrote:
Hi,
Comments below.
- Original Message -
From: Andreas Gruenbacher
Add support for an additional filesystem version (sb_fs_format = 1802).
When a filesystem with the new version is mounted, the filesystem
supports "trusted.*" xattrs.
In addition, version 1802 filesystems implement a form of forward
compatibility for xattrs: when xattrs with an unknown prefix (ea_type)
are found on a version 1802 filesystem, those attributes are not shown
by listxattr, and they are not accessible by getxattr, setxattr, or
removexattr.
This mechanism might turn out to be what we need in the future, but if
not, we can always bump the filesystem version and break compatibility
instead.
Signed-off-by: Andrew Price
If this is from Andreas, it should have his Signed-off-by:
Yes, it wasn't in the original patch (posted on bugzilla). With Andreas'
permission I'll add it to the next version.
---
fs/gfs2/ops_fstype.c | 14 +-
fs/gfs2/super.h | 4 ++-
fs/gfs2/xattr.c | 48 +---
include/uapi/linux/gfs2_ondisk.h | 5 ++--
4 files changed, 63 insertions(+), 8 deletions(-)
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 52fe78378faa..64ad19bb978c 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -489,6 +489,19 @@ static int init_sb(struct gfs2_sbd *sdp, int silent)
goto out;
}
+ switch(sdp->sd_sb.sb_fs_format) {
+ case GFS2_FS_FORMAT_MAX:
+ sb->s_xattr = gfs2_xattr_handlers_max;
+ break;
+
+ case GFS2_FS_FORMAT_MIN:
+ sb->s_xattr = gfs2_xattr_handlers_min;
+ break;
+
+ default:
+ BUG();
+ }
+
/* Set up the buffer cache and SB for real */
if (sdp->sd_sb.sb_bsize < bdev_logical_block_size(sb->s_bdev)) {
ret = -EINVAL;
@@ -1109,7 +1122,6 @@ static int gfs2_fill_super(struct super_block *sb,
struct fs_context *fc)
sb->s_op = _super_ops;
sb->s_d_op = _dops;
sb->s_export_op = _export_ops;
- sb->s_xattr = gfs2_xattr_handlers;
sb->s_qcop = _quotactl_ops;
sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP;
sb_dqopt(sb)->flags |= DQUOT_QUOTA_SYS_FILE;
diff --git a/fs/gfs2/super.h b/fs/gfs2/super.h
index 977079693bdc..08e502dec7ec 100644
--- a/fs/gfs2/super.h
+++ b/fs/gfs2/super.h
@@ -58,7 +58,9 @@ extern struct file_system_type gfs2meta_fs_type;
extern const struct export_operations gfs2_export_ops;
extern const struct super_operations gfs2_super_ops;
extern const struct dentry_operations gfs2_dops;
-extern const struct xattr_handler *gfs2_xattr_handlers[];
+
+extern const struct xattr_handler *gfs2_xattr_handlers_max[];
+extern const struct xattr_handler **gfs2_xattr_handlers_min;
#endif /* __SUPER_DOT_H__ */
diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
index 9d7667bc4292..a860a144f3d4 100644
--- a/fs/gfs2/xattr.c
+++ b/fs/gfs2/xattr.c
@@ -70,6 +70,20 @@ static int ea_check_size(struct gfs2_sbd *sdp, unsigned
int nsize, size_t dsize)
return 0;
}
+bool gfs2_eatype_valid(struct gfs2_sbd *sdp, u8 type)
This should be "static bool".
Ok, I'll update that.
+{
+ switch(sdp->sd_sb.sb_fs_format) {
+ case GFS2_FS_FORMAT_MAX:
+ return true;
+
+ case GFS2_FS_FORMAT_MIN:
+ return type <= GFS2_EATYPE_SECURITY;
+
+ default:
+ return false;
+ }
+}
+
typedef int (*ea_call_t) (struct gfs2_inode *ip, struct buffer_head *bh,
struct gfs2_ea_header *ea,
struct gfs2_ea_header *prev, void *private);
@@ -77,6 +91,7 @@ typedef int (*ea_call_t) (struct gfs2_inode *ip, struct
buffer_head *bh,
static int ea_foreach_i(struct gfs2_inode *ip, struct buffer_head *bh,
ea_call_t ea_call, void *data)
{
+ struct gfs2_sbd *sdp = GFS2_SB(>i_inode);
struct gfs2_ea_header *ea, *prev = NULL;
int error = 0;
@@ -89,9 +104,8 @@ static int ea_foreach_i(struct gfs2_inode *ip, struct
buffer_head *bh,
if (!(bh->b_data <= (char *)ea && (char *)GFS2_EA2NEXT(ea) <=
bh->b_data + bh->b_size))
goto fail;
- if (!GFS2_EATYPE_VALID(ea->ea_type))
+ if (!gfs2_eatype_valid(sdp, ea->ea_type))
goto fail;
-
error = ea_call(ip, bh, ea, prev, data);
if (error)
return error;
@@ -344,6 +358,7 @@ static int ea_list_i(struct gfs2_inode *ip, struct
buffer_head *bh,
struct gfs2_ea_header *ea, struct gfs2_ea_header *prev,
void *private)
{
+ struct gfs2_sbd *sdp = GFS2_SB(>i_inode);
struct ea_list *ei = private;
struct gfs2_ea_request *er = ei->ei_er;
Re: [Cluster-devel] [PATCH 2/2] gfs2: Add trusted xattr support
Hi,
Comments below.
- Original Message -
> From: Andreas Gruenbacher
>
> Add support for an additional filesystem version (sb_fs_format = 1802).
> When a filesystem with the new version is mounted, the filesystem
> supports "trusted.*" xattrs.
>
> In addition, version 1802 filesystems implement a form of forward
> compatibility for xattrs: when xattrs with an unknown prefix (ea_type)
> are found on a version 1802 filesystem, those attributes are not shown
> by listxattr, and they are not accessible by getxattr, setxattr, or
> removexattr.
>
> This mechanism might turn out to be what we need in the future, but if
> not, we can always bump the filesystem version and break compatibility
> instead.
>
> Signed-off-by: Andrew Price
If this is from Andreas, it should have his Signed-off-by:
> ---
> fs/gfs2/ops_fstype.c | 14 +-
> fs/gfs2/super.h | 4 ++-
> fs/gfs2/xattr.c | 48 +---
> include/uapi/linux/gfs2_ondisk.h | 5 ++--
> 4 files changed, 63 insertions(+), 8 deletions(-)
>
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index 52fe78378faa..64ad19bb978c 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -489,6 +489,19 @@ static int init_sb(struct gfs2_sbd *sdp, int silent)
> goto out;
> }
>
> + switch(sdp->sd_sb.sb_fs_format) {
> + case GFS2_FS_FORMAT_MAX:
> + sb->s_xattr = gfs2_xattr_handlers_max;
> + break;
> +
> + case GFS2_FS_FORMAT_MIN:
> + sb->s_xattr = gfs2_xattr_handlers_min;
> + break;
> +
> + default:
> + BUG();
> + }
> +
> /* Set up the buffer cache and SB for real */
> if (sdp->sd_sb.sb_bsize < bdev_logical_block_size(sb->s_bdev)) {
> ret = -EINVAL;
> @@ -1109,7 +1122,6 @@ static int gfs2_fill_super(struct super_block *sb,
> struct fs_context *fc)
> sb->s_op = _super_ops;
> sb->s_d_op = _dops;
> sb->s_export_op = _export_ops;
> - sb->s_xattr = gfs2_xattr_handlers;
> sb->s_qcop = _quotactl_ops;
> sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP;
> sb_dqopt(sb)->flags |= DQUOT_QUOTA_SYS_FILE;
> diff --git a/fs/gfs2/super.h b/fs/gfs2/super.h
> index 977079693bdc..08e502dec7ec 100644
> --- a/fs/gfs2/super.h
> +++ b/fs/gfs2/super.h
> @@ -58,7 +58,9 @@ extern struct file_system_type gfs2meta_fs_type;
> extern const struct export_operations gfs2_export_ops;
> extern const struct super_operations gfs2_super_ops;
> extern const struct dentry_operations gfs2_dops;
> -extern const struct xattr_handler *gfs2_xattr_handlers[];
> +
> +extern const struct xattr_handler *gfs2_xattr_handlers_max[];
> +extern const struct xattr_handler **gfs2_xattr_handlers_min;
>
> #endif /* __SUPER_DOT_H__ */
>
> diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
> index 9d7667bc4292..a860a144f3d4 100644
> --- a/fs/gfs2/xattr.c
> +++ b/fs/gfs2/xattr.c
> @@ -70,6 +70,20 @@ static int ea_check_size(struct gfs2_sbd *sdp, unsigned
> int nsize, size_t dsize)
> return 0;
> }
>
> +bool gfs2_eatype_valid(struct gfs2_sbd *sdp, u8 type)
This should be "static bool".
> +{
> + switch(sdp->sd_sb.sb_fs_format) {
> + case GFS2_FS_FORMAT_MAX:
> + return true;
> +
> + case GFS2_FS_FORMAT_MIN:
> + return type <= GFS2_EATYPE_SECURITY;
> +
> + default:
> + return false;
> + }
> +}
> +
> typedef int (*ea_call_t) (struct gfs2_inode *ip, struct buffer_head *bh,
> struct gfs2_ea_header *ea,
> struct gfs2_ea_header *prev, void *private);
> @@ -77,6 +91,7 @@ typedef int (*ea_call_t) (struct gfs2_inode *ip, struct
> buffer_head *bh,
> static int ea_foreach_i(struct gfs2_inode *ip, struct buffer_head *bh,
> ea_call_t ea_call, void *data)
> {
> + struct gfs2_sbd *sdp = GFS2_SB(>i_inode);
> struct gfs2_ea_header *ea, *prev = NULL;
> int error = 0;
>
> @@ -89,9 +104,8 @@ static int ea_foreach_i(struct gfs2_inode *ip, struct
> buffer_head *bh,
> if (!(bh->b_data <= (char *)ea && (char *)GFS2_EA2NEXT(ea) <=
> bh->b_data + bh->b_size))
> goto fail;
> - if (!GFS2_EATYPE_VALID(ea->ea_type))
> + if (!gfs2_eatype_valid(sdp, ea->ea_type))
> goto fail;
> -
> error = ea_call(ip, bh, ea, prev, data);
> if (error)
> return error;
> @@ -344,6 +358,7 @@ static int ea_list_i(struct gfs2_inode *ip, struct
> buffer_head *bh,
>struct gfs2_ea_header *ea, struct gfs2_ea_header *prev,
>void *private)
> {
> + struct gfs2_sbd *sdp = GFS2_SB(>i_inode);
> struct ea_list *ei = private;
> struct gfs2_ea_request *er = ei->ei_er;
> unsigned int ea_size;
> @@ -353,6 +368,8 @@ static int
[Cluster-devel] [PATCH 2/2] gfs2: Add trusted xattr support
From: Andreas Gruenbacher
Add support for an additional filesystem version (sb_fs_format = 1802).
When a filesystem with the new version is mounted, the filesystem
supports "trusted.*" xattrs.
In addition, version 1802 filesystems implement a form of forward
compatibility for xattrs: when xattrs with an unknown prefix (ea_type)
are found on a version 1802 filesystem, those attributes are not shown
by listxattr, and they are not accessible by getxattr, setxattr, or
removexattr.
This mechanism might turn out to be what we need in the future, but if
not, we can always bump the filesystem version and break compatibility
instead.
Signed-off-by: Andrew Price
---
fs/gfs2/ops_fstype.c | 14 +-
fs/gfs2/super.h | 4 ++-
fs/gfs2/xattr.c | 48 +---
include/uapi/linux/gfs2_ondisk.h | 5 ++--
4 files changed, 63 insertions(+), 8 deletions(-)
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 52fe78378faa..64ad19bb978c 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -489,6 +489,19 @@ static int init_sb(struct gfs2_sbd *sdp, int silent)
goto out;
}
+ switch(sdp->sd_sb.sb_fs_format) {
+ case GFS2_FS_FORMAT_MAX:
+ sb->s_xattr = gfs2_xattr_handlers_max;
+ break;
+
+ case GFS2_FS_FORMAT_MIN:
+ sb->s_xattr = gfs2_xattr_handlers_min;
+ break;
+
+ default:
+ BUG();
+ }
+
/* Set up the buffer cache and SB for real */
if (sdp->sd_sb.sb_bsize < bdev_logical_block_size(sb->s_bdev)) {
ret = -EINVAL;
@@ -1109,7 +1122,6 @@ static int gfs2_fill_super(struct super_block *sb, struct
fs_context *fc)
sb->s_op = _super_ops;
sb->s_d_op = _dops;
sb->s_export_op = _export_ops;
- sb->s_xattr = gfs2_xattr_handlers;
sb->s_qcop = _quotactl_ops;
sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP;
sb_dqopt(sb)->flags |= DQUOT_QUOTA_SYS_FILE;
diff --git a/fs/gfs2/super.h b/fs/gfs2/super.h
index 977079693bdc..08e502dec7ec 100644
--- a/fs/gfs2/super.h
+++ b/fs/gfs2/super.h
@@ -58,7 +58,9 @@ extern struct file_system_type gfs2meta_fs_type;
extern const struct export_operations gfs2_export_ops;
extern const struct super_operations gfs2_super_ops;
extern const struct dentry_operations gfs2_dops;
-extern const struct xattr_handler *gfs2_xattr_handlers[];
+
+extern const struct xattr_handler *gfs2_xattr_handlers_max[];
+extern const struct xattr_handler **gfs2_xattr_handlers_min;
#endif /* __SUPER_DOT_H__ */
diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
index 9d7667bc4292..a860a144f3d4 100644
--- a/fs/gfs2/xattr.c
+++ b/fs/gfs2/xattr.c
@@ -70,6 +70,20 @@ static int ea_check_size(struct gfs2_sbd *sdp, unsigned int
nsize, size_t dsize)
return 0;
}
+bool gfs2_eatype_valid(struct gfs2_sbd *sdp, u8 type)
+{
+ switch(sdp->sd_sb.sb_fs_format) {
+ case GFS2_FS_FORMAT_MAX:
+ return true;
+
+ case GFS2_FS_FORMAT_MIN:
+ return type <= GFS2_EATYPE_SECURITY;
+
+ default:
+ return false;
+ }
+}
+
typedef int (*ea_call_t) (struct gfs2_inode *ip, struct buffer_head *bh,
struct gfs2_ea_header *ea,
struct gfs2_ea_header *prev, void *private);
@@ -77,6 +91,7 @@ typedef int (*ea_call_t) (struct gfs2_inode *ip, struct
buffer_head *bh,
static int ea_foreach_i(struct gfs2_inode *ip, struct buffer_head *bh,
ea_call_t ea_call, void *data)
{
+ struct gfs2_sbd *sdp = GFS2_SB(>i_inode);
struct gfs2_ea_header *ea, *prev = NULL;
int error = 0;
@@ -89,9 +104,8 @@ static int ea_foreach_i(struct gfs2_inode *ip, struct
buffer_head *bh,
if (!(bh->b_data <= (char *)ea && (char *)GFS2_EA2NEXT(ea) <=
bh->b_data + bh->b_size))
goto fail;
- if (!GFS2_EATYPE_VALID(ea->ea_type))
+ if (!gfs2_eatype_valid(sdp, ea->ea_type))
goto fail;
-
error = ea_call(ip, bh, ea, prev, data);
if (error)
return error;
@@ -344,6 +358,7 @@ static int ea_list_i(struct gfs2_inode *ip, struct
buffer_head *bh,
struct gfs2_ea_header *ea, struct gfs2_ea_header *prev,
void *private)
{
+ struct gfs2_sbd *sdp = GFS2_SB(>i_inode);
struct ea_list *ei = private;
struct gfs2_ea_request *er = ei->ei_er;
unsigned int ea_size;
@@ -353,6 +368,8 @@ static int ea_list_i(struct gfs2_inode *ip, struct
buffer_head *bh,
if (ea->ea_type == GFS2_EATYPE_UNUSED)
return 0;
+ BUG_ON(ea->ea_type > GFS2_EATYPE_SECURITY &&
+ sdp->sd_sb.sb_fs_format == GFS2_FS_FORMAT_MIN);
switch (ea->ea_type) {
case
[Cluster-devel] [PATCH 2/2] gfs2: Add trusted xattr support
From: Andreas Gruenbacher
Add support for an additional filesystem version (sb_fs_format = 1802).
When a filesystem with the new version is mounted, the filesystem
supports "trusted.*" xattrs.
In addition, version 1802 filesystems implement a form of forward
compatibility for xattrs: when xattrs with an unknown prefix (ea_type)
are found on a version 1802 filesystem, those attributes are not shown
by listxattr, and they are not accessible by getxattr, setxattr, or
removexattr.
This mechanism might turn out to be what we need in the future, but if
not, we can always bump the filesystem version and break compatibility
instead.
[AP: Removed the GFS2_FORMAT_FS_MIN and _MAX values from gfs2_ondisk.h
and bumped the GFS2_EATYPE_LAST value]
Signed-off-by: Andrew Price
---
fs/gfs2/ops_fstype.c | 14 +-
fs/gfs2/super.h | 4 ++-
fs/gfs2/xattr.c | 48 +---
include/uapi/linux/gfs2_ondisk.h | 5 ++--
4 files changed, 63 insertions(+), 8 deletions(-)
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 52fe78378faa..64ad19bb978c 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -489,6 +489,19 @@ static int init_sb(struct gfs2_sbd *sdp, int silent)
goto out;
}
+ switch(sdp->sd_sb.sb_fs_format) {
+ case GFS2_FS_FORMAT_MAX:
+ sb->s_xattr = gfs2_xattr_handlers_max;
+ break;
+
+ case GFS2_FS_FORMAT_MIN:
+ sb->s_xattr = gfs2_xattr_handlers_min;
+ break;
+
+ default:
+ BUG();
+ }
+
/* Set up the buffer cache and SB for real */
if (sdp->sd_sb.sb_bsize < bdev_logical_block_size(sb->s_bdev)) {
ret = -EINVAL;
@@ -1109,7 +1122,6 @@ static int gfs2_fill_super(struct super_block *sb, struct
fs_context *fc)
sb->s_op = _super_ops;
sb->s_d_op = _dops;
sb->s_export_op = _export_ops;
- sb->s_xattr = gfs2_xattr_handlers;
sb->s_qcop = _quotactl_ops;
sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP;
sb_dqopt(sb)->flags |= DQUOT_QUOTA_SYS_FILE;
diff --git a/fs/gfs2/super.h b/fs/gfs2/super.h
index 977079693bdc..08e502dec7ec 100644
--- a/fs/gfs2/super.h
+++ b/fs/gfs2/super.h
@@ -58,7 +58,9 @@ extern struct file_system_type gfs2meta_fs_type;
extern const struct export_operations gfs2_export_ops;
extern const struct super_operations gfs2_super_ops;
extern const struct dentry_operations gfs2_dops;
-extern const struct xattr_handler *gfs2_xattr_handlers[];
+
+extern const struct xattr_handler *gfs2_xattr_handlers_max[];
+extern const struct xattr_handler **gfs2_xattr_handlers_min;
#endif /* __SUPER_DOT_H__ */
diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
index 9d7667bc4292..a860a144f3d4 100644
--- a/fs/gfs2/xattr.c
+++ b/fs/gfs2/xattr.c
@@ -70,6 +70,20 @@ static int ea_check_size(struct gfs2_sbd *sdp, unsigned int
nsize, size_t dsize)
return 0;
}
+bool gfs2_eatype_valid(struct gfs2_sbd *sdp, u8 type)
+{
+ switch(sdp->sd_sb.sb_fs_format) {
+ case GFS2_FS_FORMAT_MAX:
+ return true;
+
+ case GFS2_FS_FORMAT_MIN:
+ return type <= GFS2_EATYPE_SECURITY;
+
+ default:
+ return false;
+ }
+}
+
typedef int (*ea_call_t) (struct gfs2_inode *ip, struct buffer_head *bh,
struct gfs2_ea_header *ea,
struct gfs2_ea_header *prev, void *private);
@@ -77,6 +91,7 @@ typedef int (*ea_call_t) (struct gfs2_inode *ip, struct
buffer_head *bh,
static int ea_foreach_i(struct gfs2_inode *ip, struct buffer_head *bh,
ea_call_t ea_call, void *data)
{
+ struct gfs2_sbd *sdp = GFS2_SB(>i_inode);
struct gfs2_ea_header *ea, *prev = NULL;
int error = 0;
@@ -89,9 +104,8 @@ static int ea_foreach_i(struct gfs2_inode *ip, struct
buffer_head *bh,
if (!(bh->b_data <= (char *)ea && (char *)GFS2_EA2NEXT(ea) <=
bh->b_data + bh->b_size))
goto fail;
- if (!GFS2_EATYPE_VALID(ea->ea_type))
+ if (!gfs2_eatype_valid(sdp, ea->ea_type))
goto fail;
-
error = ea_call(ip, bh, ea, prev, data);
if (error)
return error;
@@ -344,6 +358,7 @@ static int ea_list_i(struct gfs2_inode *ip, struct
buffer_head *bh,
struct gfs2_ea_header *ea, struct gfs2_ea_header *prev,
void *private)
{
+ struct gfs2_sbd *sdp = GFS2_SB(>i_inode);
struct ea_list *ei = private;
struct gfs2_ea_request *er = ei->ei_er;
unsigned int ea_size;
@@ -353,6 +368,8 @@ static int ea_list_i(struct gfs2_inode *ip, struct
buffer_head *bh,
if (ea->ea_type == GFS2_EATYPE_UNUSED)
return 0;
+ BUG_ON(ea->ea_type > GFS2_EATYPE_SECURITY
