Re: [PATCH v3 17/28] xfs: add attribute type for fs-verity

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, Oct 06, 2023 at 08:49:11PM +0200, Andrey Albershteyn wrote:
> The Merkle tree blocks and descriptor are stored in the extended
> attributes of the inode. Add new attribute type for fs-verity
> metadata. Add XFS_ATTR_INTERNAL_MASK to skip parent pointer and
> fs-verity attributes as those are only for internal use. While we're
> at it add a few comments in relevant places that internally visible
> attributes are not suppose to be handled via interface defined in
> xfs_xattr.c.
> 
> Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_da_format.h  | 10 +++++++++-
>  fs/xfs/libxfs/xfs_log_format.h |  1 +
>  fs/xfs/xfs_ioctl.c             |  5 +++++
>  fs/xfs/xfs_trace.h             |  1 +
>  fs/xfs/xfs_xattr.c             |  9 +++++++++
>  5 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
> index 6deefe03207f..b56bdae83563 100644
> --- a/fs/xfs/libxfs/xfs_da_format.h
> +++ b/fs/xfs/libxfs/xfs_da_format.h
> @@ -699,14 +699,22 @@ 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)
> +
> +/*
> + * Internal attributes not exposed to the user
> + */
> +#define XFS_ATTR_INTERNAL_MASK (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 0bc1749fb7bb..c42cc58cd152 100644
> --- a/fs/xfs/libxfs/xfs_log_format.h
> +++ b/fs/xfs/libxfs/xfs_log_format.h
> @@ -975,6 +975,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_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 55bb01173cde..3d6d680b6cf3 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -351,6 +351,11 @@ static unsigned int
>  xfs_attr_filter(
>  	u32			ioc_flags)
>  {
> +	/*
> +	 * Only externally visible attributes should be specified here.
> +	 * Internally used attributes (such as parent pointers or fs-verity)
> +	 * should not be exposed to userspace.
> +	 */
>  	if (ioc_flags & XFS_IOC_ATTR_ROOT)
>  		return XFS_ATTR_ROOT;
>  	if (ioc_flags & XFS_IOC_ATTR_SECURE)
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 3926cf7f2a6e..3696709907bf 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -82,6 +82,7 @@ struct xfs_perag;
>  #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 a3975f325f4e..56f7f4122fcb 100644
> --- a/fs/xfs/xfs_xattr.c
> +++ b/fs/xfs/xfs_xattr.c
> @@ -20,6 +20,12 @@
>  
>  #include <linux/posix_acl_xattr.h>
>  
> +/*
> + * This file defines interface to work with externally visible extended
> + * attributes, such as those in system or security namespaces. This interface

"...such as those in user, system, or security namespaces."

With that fixed,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D


> + * should not be used for internally used attributes (consider xfs_attr.c).
> + */
> +
>  /*
>   * Get permission to use log-assisted atomic exchange of file extents.
>   *
> @@ -241,6 +247,9 @@ xfs_xattr_put_listent(
>  
>  	ASSERT(context->count >= 0);
>  
> +	if (flags & XFS_ATTR_INTERNAL_MASK)
> +		return;
> +
>  	if (flags & XFS_ATTR_ROOT) {
>  #ifdef CONFIG_XFS_POSIX_ACL
>  		if (namelen == SGI_ACL_FILE_SIZE &&
> -- 
> 2.40.1
>