| Message ID | 20221213172935.680971-4-aalbersh@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fs-verity support for XFS | expand |
On 12/13/22 11:29 AM, Andrey Albershteyn wrote: > The Merkle tree pages and descriptor are stored in the extended > attributes of the inode. Add new attribute type for fs-verity > metadata. Skip fs-verity attributes for getfattr as it can not parse > binary page names. > > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com> > DECLARE_EVENT_CLASS(xfs_attr_list_class, > diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c > index 5b57f6348d630..acbfa29d04af0 100644 > --- a/fs/xfs/xfs_xattr.c > +++ b/fs/xfs/xfs_xattr.c > @@ -237,6 +237,9 @@ xfs_xattr_put_listent( > if (flags & XFS_ATTR_PARENT) > return; > > + if (flags & XFS_ATTR_VERITY) > + return; > + Just a nitpick, but now that there are already 2 cases like this, I wonder if it would be wise to #define something like an XFS_ATTR_VISIBLE_MASK (or maybe XFS_ATTR_INTERNAL_MASK) and use that to decide, rather than testing each one individually? Thanks, -Eric
On Tue, Dec 13, 2022 at 11:43:42AM -0600, Eric Sandeen wrote: > On 12/13/22 11:29 AM, Andrey Albershteyn wrote: > > The Merkle tree pages and descriptor are stored in the extended > > attributes of the inode. Add new attribute type for fs-verity > > metadata. Skip fs-verity attributes for getfattr as it can not parse > > binary page names. > > > > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com> > > > > DECLARE_EVENT_CLASS(xfs_attr_list_class, > > diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c > > index 5b57f6348d630..acbfa29d04af0 100644 > > --- a/fs/xfs/xfs_xattr.c > > +++ b/fs/xfs/xfs_xattr.c > > @@ -237,6 +237,9 @@ xfs_xattr_put_listent( > > if (flags & XFS_ATTR_PARENT) > > return; > > > > + if (flags & XFS_ATTR_VERITY) > > + return; > > + > > Just a nitpick, but now that there are already 2 cases like this, I wonder > if it would be wise to #define something like an XFS_ATTR_VISIBLE_MASK > (or maybe XFS_ATTR_INTERNAL_MASK) and use that to decide, rather than > testing each one individually? Seems like a good idea to me. There's also a couple of other spots that a comment about internal vs externally visible xattr namespaces might be appropriate. e.g xfs_attr_filter() in the ioctl code should never have internal xattr namespaces added to it, and similarly a comment at the top of fs/xfs/xfs_xattr.c that the interfaces implemented in the file are only for exposing externally visible xattr namespaces to users. That way it becomes more obvious that we handle internal vs external xattr namespaces very differently. Cheers, Dave.
On Wed, Dec 14, 2022 at 12:03:56PM +1100, Dave Chinner wrote: > On Tue, Dec 13, 2022 at 11:43:42AM -0600, Eric Sandeen wrote: > > On 12/13/22 11:29 AM, Andrey Albershteyn wrote: > > > The Merkle tree pages and descriptor are stored in the extended > > > attributes of the inode. Add new attribute type for fs-verity > > > metadata. Skip fs-verity attributes for getfattr as it can not parse > > > binary page names. > > > > > > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com> > > > > > > > DECLARE_EVENT_CLASS(xfs_attr_list_class, > > > diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c > > > index 5b57f6348d630..acbfa29d04af0 100644 > > > --- a/fs/xfs/xfs_xattr.c > > > +++ b/fs/xfs/xfs_xattr.c > > > @@ -237,6 +237,9 @@ xfs_xattr_put_listent( > > > if (flags & XFS_ATTR_PARENT) > > > return; > > > > > > + if (flags & XFS_ATTR_VERITY) > > > + return; > > > + > > > > Just a nitpick, but now that there are already 2 cases like this, I wonder > > if it would be wise to #define something like an XFS_ATTR_VISIBLE_MASK > > (or maybe XFS_ATTR_INTERNAL_MASK) and use that to decide, rather than > > testing each one individually? > > Seems like a good idea to me. Agreed. > > There's also a couple of other spots that a comment about internal > vs externally visible xattr namespaces might be appropriate. e.g > xfs_attr_filter() in the ioctl code should never have internal xattr > namespaces added to it, and similarly a comment at the top of > fs/xfs/xfs_xattr.c that the interfaces implemented in the file are > only for exposing externally visible xattr namespaces to users. Thanks, will describe that. > > That way it becomes more obvious that we handle internal vs external > xattr namespaces very differently. > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com >
diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h index 75b13807145d1..778bf2b476618 100644 --- a/fs/xfs/libxfs/xfs_da_format.h +++ b/fs/xfs/libxfs/xfs_da_format.h @@ -689,14 +689,17 @@ struct xfs_attr3_leafblock { #define XFS_ATTR_ROOT_BIT 1 /* limit access to trusted attrs */ #define XFS_ATTR_SECURE_BIT 2 /* limit access to secure attrs */ #define XFS_ATTR_PARENT_BIT 3 /* parent pointer attrs */ +#define XFS_ATTR_VERITY_BIT 4 /* verity merkle tree and descriptor */ #define XFS_ATTR_INCOMPLETE_BIT 7 /* attr in middle of create/delete */ #define XFS_ATTR_LOCAL (1u << XFS_ATTR_LOCAL_BIT) #define XFS_ATTR_ROOT (1u << XFS_ATTR_ROOT_BIT) #define XFS_ATTR_SECURE (1u << XFS_ATTR_SECURE_BIT) #define XFS_ATTR_PARENT (1u << XFS_ATTR_PARENT_BIT) +#define XFS_ATTR_VERITY (1u << XFS_ATTR_VERITY_BIT) #define XFS_ATTR_INCOMPLETE (1u << XFS_ATTR_INCOMPLETE_BIT) #define XFS_ATTR_NSP_ONDISK_MASK \ - (XFS_ATTR_ROOT | XFS_ATTR_SECURE | XFS_ATTR_PARENT) + (XFS_ATTR_ROOT | XFS_ATTR_SECURE | XFS_ATTR_PARENT | \ + XFS_ATTR_VERITY) /* * Alignment for namelist and valuelist entries (since they are mixed diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h index 727b5a8580285..678eacb7925c9 100644 --- a/fs/xfs/libxfs/xfs_log_format.h +++ b/fs/xfs/libxfs/xfs_log_format.h @@ -968,6 +968,7 @@ struct xfs_icreate_log { #define XFS_ATTRI_FILTER_MASK (XFS_ATTR_ROOT | \ XFS_ATTR_SECURE | \ XFS_ATTR_PARENT | \ + XFS_ATTR_VERITY | \ XFS_ATTR_INCOMPLETE) /* diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index 372d871bccc5e..5eceb259cc5f7 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -78,6 +78,7 @@ struct xfs_icwalk; #define XFS_ATTR_FILTER_FLAGS \ { XFS_ATTR_ROOT, "ROOT" }, \ { XFS_ATTR_SECURE, "SECURE" }, \ + { XFS_ATTR_VERITY, "VERITY" }, \ { XFS_ATTR_INCOMPLETE, "INCOMPLETE" } DECLARE_EVENT_CLASS(xfs_attr_list_class, diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c index 5b57f6348d630..acbfa29d04af0 100644 --- a/fs/xfs/xfs_xattr.c +++ b/fs/xfs/xfs_xattr.c @@ -237,6 +237,9 @@ xfs_xattr_put_listent( if (flags & XFS_ATTR_PARENT) return; + if (flags & XFS_ATTR_VERITY) + return; + if (flags & XFS_ATTR_ROOT) { #ifdef CONFIG_XFS_POSIX_ACL if (namelen == SGI_ACL_FILE_SIZE &&
The Merkle tree pages and descriptor are stored in the extended attributes of the inode. Add new attribute type for fs-verity metadata. Skip fs-verity attributes for getfattr as it can not parse binary page names. Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com> --- fs/xfs/libxfs/xfs_da_format.h | 5 ++++- fs/xfs/libxfs/xfs_log_format.h | 1 + fs/xfs/xfs_trace.h | 1 + fs/xfs/xfs_xattr.c | 3 +++ 4 files changed, 9 insertions(+), 1 deletion(-)