linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Andrey Albershteyn <aalbersh@redhat.com>
Cc: linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	fsverity@lists.linux.dev, ebiggers@kernel.org,
	david@fromorbit.com, dchinner@redhat.com
Subject: Re: [PATCH v3 25/28] xfs: add fs-verity support
Date: Tue, 10 Oct 2023 18:39:40 -0700	[thread overview]
Message-ID: <20231011013940.GJ21298@frogsfrogsfrogs> (raw)
In-Reply-To: <20231006184922.252188-26-aalbersh@redhat.com>

On Fri, Oct 06, 2023 at 08:49:19PM +0200, Andrey Albershteyn wrote:
> Add integration with fs-verity. The XFS store fs-verity metadata in
> the extended attributes. The metadata consist of verity descriptor
> and Merkle tree blocks.
> 
> The descriptor is stored under "verity_descriptor" extended
> attribute. The Merkle tree blocks are stored under binary indexes.
> 
> When fs-verity is enabled on an inode, the XFS_IVERITY_CONSTRUCTION
> flag is set meaning that the Merkle tree is being build. The
> initialization ends with storing of verity descriptor and setting
> inode on-disk flag (XFS_DIFLAG2_VERITY).
> 
> The verification on read is done in iomap. Based on the inode verity
> flag the IOMAP_F_READ_VERITY is set in xfs_read_iomap_begin() to let
> iomap know that verification is needed.
> 
> Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> ---
>  fs/xfs/Makefile                 |   1 +
>  fs/xfs/libxfs/xfs_attr.c        |  13 ++
>  fs/xfs/libxfs/xfs_attr_leaf.c   |  17 ++-
>  fs/xfs/libxfs/xfs_attr_remote.c |   8 +-
>  fs/xfs/libxfs/xfs_da_format.h   |  16 ++
>  fs/xfs/xfs_inode.h              |   3 +-
>  fs/xfs/xfs_iomap.c              |   3 +
>  fs/xfs/xfs_ondisk.h             |   4 +
>  fs/xfs/xfs_super.c              |   8 +
>  fs/xfs/xfs_verity.c             | 257 ++++++++++++++++++++++++++++++++
>  fs/xfs/xfs_verity.h             |  37 +++++
>  11 files changed, 360 insertions(+), 7 deletions(-)
>  create mode 100644 fs/xfs/xfs_verity.c
>  create mode 100644 fs/xfs/xfs_verity.h
> 
> diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
> index 7762c01a85cf..c1a58ed8b419 100644
> --- a/fs/xfs/Makefile
> +++ b/fs/xfs/Makefile
> @@ -130,6 +130,7 @@ xfs-$(CONFIG_XFS_POSIX_ACL)	+= xfs_acl.o
>  xfs-$(CONFIG_SYSCTL)		+= xfs_sysctl.o
>  xfs-$(CONFIG_COMPAT)		+= xfs_ioctl32.o
>  xfs-$(CONFIG_EXPORTFS_BLOCK_OPS)	+= xfs_pnfs.o
> +xfs-$(CONFIG_FS_VERITY)		+= xfs_verity.o
>  
>  # notify failure
>  ifeq ($(CONFIG_MEMORY_FAILURE),y)
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 298b74245267..25e1f829e01e 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -26,6 +26,7 @@
>  #include "xfs_trace.h"
>  #include "xfs_attr_item.h"
>  #include "xfs_xattr.h"
> +#include "xfs_verity.h"
>  
>  struct kmem_cache		*xfs_attr_intent_cache;
>  
> @@ -1635,6 +1636,18 @@ xfs_attr_namecheck(
>  		return xfs_verify_pptr(mp, (struct xfs_parent_name_rec *)name);
>  	}
>  
> +	if (flags & XFS_ATTR_VERITY) {
> +		/* Merkle tree pages are stored under u64 indexes */
> +		if (length == sizeof(struct xfs_fsverity_merkle_key))
> +			return true;
> +
> +		/* Verity descriptor blocks are held in a named attribute. */
> +		if (length == XFS_VERITY_DESCRIPTOR_NAME_LEN)
> +			return true;
> +
> +		return false;
> +	}
> +
>  	return xfs_str_attr_namecheck(name, length);
>  }
>  
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index a84795d70de1..36d1f88d972f 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -29,6 +29,7 @@
>  #include "xfs_log.h"
>  #include "xfs_ag.h"
>  #include "xfs_errortag.h"
> +#include "xfs_verity.h"
>  
>  
>  /*
> @@ -518,7 +519,12 @@ xfs_attr_copy_value(
>  		return -ERANGE;
>  	}
>  
> -	if (!args->value) {
> +	/*
> +	 * We don't want to allocate memory for fs-verity Merkle tree blocks
> +	 * (fs-verity descriptor is fine though). They will be stored in
> +	 * underlying xfs_buf
> +	 */
> +	if (!args->value && !xfs_verity_merkle_block(args)) {

Hmm, why isn't this simply !(args->op_flags & XFS_DA_OP_BUFFER) ?

>  		args->value = kvmalloc(valuelen, GFP_KERNEL | __GFP_NOLOCKDEP);
>  		if (!args->value)
>  			return -ENOMEM;
> @@ -537,7 +543,14 @@ xfs_attr_copy_value(
>  	 */
>  	if (!value)
>  		return -EINVAL;
> -	memcpy(args->value, value, valuelen);
> +	/*
> +	 * We won't copy Merkle tree block to the args->value as we want it be
> +	 * in the xfs_buf. And we didn't allocate any memory in args->value.
> +	 */
> +	if (xfs_verity_merkle_block(args))
> +		args->value = value;
> +	else
> +		memcpy(args->value, value, valuelen);
>  	return 0;
>  }
>  
> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
> index 7657daf7cff3..7b4424e3454b 100644
> --- a/fs/xfs/libxfs/xfs_attr_remote.c
> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
> @@ -22,6 +22,7 @@
>  #include "xfs_attr_remote.h"
>  #include "xfs_trace.h"
>  #include "xfs_error.h"
> +#include "xfs_verity.h"
>  
>  #define ATTR_RMTVALUE_MAPSIZE	1	/* # of map entries at once */
>  
> @@ -401,11 +402,10 @@ xfs_attr_rmtval_get(
>  	ASSERT(args->rmtvaluelen == args->valuelen);
>  
>  	/*
> -	 * We also check for _OP_BUFFER as we want to trigger on
> -	 * verity blocks only, not on verity_descriptor
> +	 * For fs-verity we want additional space in the xfs_buf. This space is
> +	 * used to copy xattr value without leaf headers (crc header).
>  	 */
> -	if (args->attr_filter & XFS_ATTR_VERITY &&
> -			args->op_flags & XFS_DA_OP_BUFFER)
> +	if (xfs_verity_merkle_block(args))
>  		flags = XBF_DOUBLE_ALLOC;

Hmm, not sure what DOUBLE_ALLOC does, but I'll get there as I go
backwards through the XFS patches...

>  
>  	valuelen = args->rmtvaluelen;
> diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
> index b56bdae83563..a678ad5e4a08 100644
> --- a/fs/xfs/libxfs/xfs_da_format.h
> +++ b/fs/xfs/libxfs/xfs_da_format.h
> @@ -903,4 +903,20 @@ struct xfs_parent_name_irec {
>  	uint8_t			p_namelen;
>  };
>  
> +/*
> + * fs-verity attribute name format
> + *
> + * Merkle tree blocks are stored under extended attributes of the inode. The
> + * name of the attributes are offsets into merkle tree.

Are these offsets byte offsets?

> + */
> +struct xfs_fsverity_merkle_key {
> +	__be64 merkleoff;
> +};
> +
> +static inline void
> +xfs_fsverity_merkle_key_to_disk(struct xfs_fsverity_merkle_key *key, loff_t pos)
> +{
> +	key->merkleoff = cpu_to_be64(pos);
> +}
> +
>  #endif /* __XFS_DA_FORMAT_H__ */
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 0c5bdb91152e..e6c30a69e8d1 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -342,7 +342,8 @@ static inline bool xfs_inode_has_large_extent_counts(struct xfs_inode *ip)
>   * inactivation completes, both flags will be cleared and the inode is a
>   * plain old IRECLAIMABLE inode.
>   */
> -#define XFS_INACTIVATING	(1 << 13)
> +#define XFS_INACTIVATING		(1 << 13)
> +#define XFS_IVERITY_CONSTRUCTION	(1 << 14) /* merkle tree construction */

Under construction?  Or already built?

>  
>  /* Quotacheck is running but inode has not been added to quota counts. */
>  #define XFS_IQUOTAUNCHECKED	(1 << 14)

These two iflags have the same definition.

> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 18c8f168b153..80b249c42067 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -132,6 +132,9 @@ xfs_bmbt_to_iomap(
>  	    (ip->i_itemp->ili_fsync_fields & ~XFS_ILOG_TIMESTAMP))
>  		iomap->flags |= IOMAP_F_DIRTY;
>  
> +	if (fsverity_active(VFS_I(ip)))
> +		iomap->flags |= IOMAP_F_READ_VERITY;
> +
>  	iomap->validity_cookie = sequence_cookie;
>  	iomap->folio_ops = &xfs_iomap_folio_ops;
>  	return 0;
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index c4cc99b70dd3..accbbdeb7624 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -190,6 +190,10 @@ xfs_check_ondisk_structs(void)
>  	XFS_CHECK_VALUE(XFS_DQ_BIGTIME_EXPIRY_MIN << XFS_DQ_BIGTIME_SHIFT, 4);
>  	XFS_CHECK_VALUE(XFS_DQ_BIGTIME_EXPIRY_MAX << XFS_DQ_BIGTIME_SHIFT,
>  			16299260424LL);
> +
> +	/* fs-verity descriptor xattr name */
> +	XFS_CHECK_VALUE(strlen(XFS_VERITY_DESCRIPTOR_NAME),
> +			XFS_VERITY_DESCRIPTOR_NAME_LEN);
>  }
>  
>  #endif /* __XFS_ONDISK_H */
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 6a3b5285044a..f32392add622 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -30,6 +30,7 @@
>  #include "xfs_filestream.h"
>  #include "xfs_quota.h"
>  #include "xfs_sysfs.h"
> +#include "xfs_verity.h"
>  #include "xfs_ondisk.h"
>  #include "xfs_rmap_item.h"
>  #include "xfs_refcount_item.h"
> @@ -1526,6 +1527,9 @@ xfs_fs_fill_super(
>  	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ;
>  #endif
>  	sb->s_op = &xfs_super_operations;
> +#ifdef CONFIG_FS_VERITY
> +	sb->s_vop = &xfs_verity_ops;
> +#endif
>  
>  	/*
>  	 * Delay mount work if the debug hook is set. This is debug
> @@ -1735,6 +1739,10 @@ xfs_fs_fill_super(
>  		goto out_filestream_unmount;
>  	}
>  
> +	if (xfs_has_verity(mp))
> +		xfs_alert(mp,
> +	"EXPERIMENTAL fs-verity feature in use. Use at your own risk!");
> +
>  	error = xfs_mountfs(mp);
>  	if (error)
>  		goto out_filestream_unmount;
> diff --git a/fs/xfs/xfs_verity.c b/fs/xfs/xfs_verity.c
> new file mode 100644
> index 000000000000..a2db56974122
> --- /dev/null
> +++ b/fs/xfs/xfs_verity.c
> @@ -0,0 +1,257 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023 Red Hat, Inc.
> + */
> +#include "xfs.h"
> +#include "xfs_shared.h"
> +#include "xfs_format.h"
> +#include "xfs_da_format.h"
> +#include "xfs_da_btree.h"
> +#include "xfs_trans_resv.h"
> +#include "xfs_mount.h"
> +#include "xfs_inode.h"
> +#include "xfs_attr.h"
> +#include "xfs_verity.h"
> +#include "xfs_bmap_util.h"
> +#include "xfs_log_format.h"
> +#include "xfs_trans.h"
> +
> +static int

Hum, why isn't this ssize_t?  That's what other IO functions return when
returning the number of bytes acted upon.

> +xfs_get_verity_descriptor(
> +	struct inode		*inode,
> +	void			*buf,
> +	size_t			buf_size)
> +{
> +	struct xfs_inode	*ip = XFS_I(inode);
> +	int			error = 0;
> +	struct xfs_da_args	args = {
> +		.dp		= ip,
> +		.attr_filter	= XFS_ATTR_VERITY,
> +		.name		= (const uint8_t *)XFS_VERITY_DESCRIPTOR_NAME,
> +		.namelen	= XFS_VERITY_DESCRIPTOR_NAME_LEN,
> +		.value		= buf,
> +		.valuelen	= buf_size,
> +	};
> +
> +	/*
> +	 * The fact that (returned attribute size) == (provided buf_size) is
> +	 * checked by xfs_attr_copy_value() (returns -ERANGE)
> +	 */
> +	error = xfs_attr_get(&args);
> +	if (error)
> +		return error;
> +
> +	return args.valuelen;
> +}
> +
> +static int
> +xfs_begin_enable_verity(
> +	struct file	    *filp)
> +{
> +	struct inode	    *inode = file_inode(filp);
> +	struct xfs_inode    *ip = XFS_I(inode);
> +	int		    error = 0;
> +
> +	ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
> +
> +	if (IS_DAX(inode))
> +		return -EINVAL;
> +
> +	if (xfs_iflags_test_and_set(ip, XFS_IVERITY_CONSTRUCTION))
> +		return -EBUSY;
> +
> +	return error;
> +}
> +
> +static int
> +xfs_drop_merkle_tree(
> +	struct xfs_inode	*ip,
> +	u64			merkle_tree_size,
> +	u8			log_blocksize)
> +{
> +	struct xfs_fsverity_merkle_key	name;
> +	int			error = 0, index;
> +	u64			offset = 0;
> +	struct xfs_da_args	args = {
> +		.dp		= ip,
> +		.whichfork	= XFS_ATTR_FORK,
> +		.attr_filter	= XFS_ATTR_VERITY,
> +		.namelen	= sizeof(struct xfs_fsverity_merkle_key),
> +		/* NULL value make xfs_attr_set remove the attr */
> +		.value		= NULL,
> +	};
> +
> +	for (index = 1; offset < merkle_tree_size; index++) {
> +		xfs_fsverity_merkle_key_to_disk(&name, offset);
> +		args.name = (const uint8_t *)&name.merkleoff;
> +		args.attr_filter = XFS_ATTR_VERITY;

Why do these two args. fields need to be reset every time through the
loop?

> +		error = xfs_attr_set(&args);

Why is it ok to drop the error here?

> +		offset = index << log_blocksize;

Hm, ok, the merkle key offsets /are/ byte offsets.

Isn't this whole loop just:

	args.name = ...;
	args.attr_filter = ...;
	for (offset = 0; offset < merkle_tree_size; offset++) {
		xfs_fsverity_merkle_key_to_disk(&name, pos << log_blocksize);
		error = xfs_attr_set(&args);
		if (error)
			return error;
	}

> +	}
> +
> +	args.name = (const uint8_t *)XFS_VERITY_DESCRIPTOR_NAME;
> +	args.namelen = XFS_VERITY_DESCRIPTOR_NAME_LEN;
> +	args.attr_filter = XFS_ATTR_VERITY;
> +	error = xfs_attr_set(&args);
> +
> +	return error;
> +}
> +
> +static int
> +xfs_end_enable_verity(
> +	struct file		*filp,
> +	const void		*desc,
> +	size_t			desc_size,
> +	u64			merkle_tree_size,
> +	u8			log_blocksize)
> +{
> +	struct inode		*inode = file_inode(filp);
> +	struct xfs_inode	*ip = XFS_I(inode);
> +	struct xfs_mount	*mp = ip->i_mount;
> +	struct xfs_trans	*tp;
> +	struct xfs_da_args	args = {
> +		.dp		= ip,
> +		.whichfork	= XFS_ATTR_FORK,
> +		.attr_filter	= XFS_ATTR_VERITY,
> +		.attr_flags	= XATTR_CREATE,
> +		.name		= (const uint8_t *)XFS_VERITY_DESCRIPTOR_NAME,
> +		.namelen	= XFS_VERITY_DESCRIPTOR_NAME_LEN,
> +		.value		= (void *)desc,
> +		.valuelen	= desc_size,
> +	};
> +	int			error = 0;
> +
> +	ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
> +
> +	/* fs-verity failed, just cleanup */
> +	if (desc == NULL)
> +		goto out;
> +
> +	error = xfs_attr_set(&args);
> +	if (error)
> +		goto out;
> +
> +	/* Set fsverity inode flag */
> +	error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_ichange,
> +			0, 0, false, &tp);
> +	if (error)
> +		goto out;
> +
> +	/*
> +	 * Ensure that we've persisted the verity information before we enable
> +	 * it on the inode and tell the caller we have sealed the inode.
> +	 */
> +	ip->i_diflags2 |= XFS_DIFLAG2_VERITY;
> +
> +	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> +	xfs_trans_set_sync(tp);
> +
> +	error = xfs_trans_commit(tp);
> +	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> +
> +	if (!error)
> +		inode->i_flags |= S_VERITY;
> +
> +out:
> +	if (error)
> +		WARN_ON_ONCE(xfs_drop_merkle_tree(ip, merkle_tree_size,
> +						  log_blocksize));

(Don't WARNings panic some kernels?)

> +
> +	xfs_iflags_clear(ip, XFS_IVERITY_CONSTRUCTION);
> +	return error;
> +}
> +
> +int
> +xfs_read_merkle_tree_block(
> +	struct inode		*inode,
> +	unsigned int		pos,
> +	struct fsverity_block	*block,
> +	unsigned long		num_ra_pages)
> +{
> +	struct xfs_inode	*ip = XFS_I(inode);
> +	struct xfs_fsverity_merkle_key name;
> +	int			error = 0;
> +	struct xfs_da_args	args = {
> +		.dp		= ip,
> +		.attr_filter	= XFS_ATTR_VERITY,
> +		.namelen	= sizeof(struct xfs_fsverity_merkle_key),
> +	};
> +	xfs_fsverity_merkle_key_to_disk(&name, pos);
> +	args.name = (const uint8_t *)&name.merkleoff;
> +
> +	error = xfs_attr_get(&args);
> +	if (error)
> +		goto out;
> +
> +	WARN_ON_ONCE(!args.valuelen);
> +
> +	/* now we also want to get underlying xfs_buf */
> +	args.op_flags = XFS_DA_OP_BUFFER;

If XFS_DA_OP_BUFFER returns the (bhold'd) xfs_buf containing the value,
then ... can't we call xfs_attr_get once?

Can the xfs_buf contents change after we drop the I{,O}LOCK?

Can users (or the kernel) change or add xattrs on a verity file?

Are they allowed to move the file, and hence update the parent pointers?

Or is the point of XBF_DOUBLE_ALLOC that we'll snapshot the attr data
into the second half of the buffer, and that's what gets passed to
fsverity core code?

> +	error = xfs_attr_get(&args);
> +	if (error)
> +		goto out;
> +
> +	block->kaddr = args.value;
> +	block->len = args.valuelen;
> +	block->cached = args.bp->b_flags & XBF_VERITY_CHECKED;
> +	block->context = args.bp;
> +
> +	return error;
> +
> +out:
> +	kmem_free(args.value);
> +	if (args.bp)
> +		xfs_buf_rele(args.bp);
> +	return error;
> +}
> +
> +static int
> +xfs_write_merkle_tree_block(
> +	struct inode		*inode,
> +	const void		*buf,
> +	u64			pos,
> +	unsigned int		size)
> +{
> +	struct xfs_inode	*ip = XFS_I(inode);
> +	struct xfs_fsverity_merkle_key	name;
> +	struct xfs_da_args	args = {
> +		.dp		= ip,
> +		.whichfork	= XFS_ATTR_FORK,
> +		.attr_filter	= XFS_ATTR_VERITY,
> +		.attr_flags	= XATTR_CREATE,

What happens if merkle tree fails midway through writing the blobs to
the xattr tree?  If they're not removed, then won't XATTR_CREATE here
cause a second attempt to fail because there's a half-built tree?

> +		.namelen	= sizeof(struct xfs_fsverity_merkle_key),
> +		.value		= (void *)buf,
> +		.valuelen	= size,
> +	};
> +
> +	xfs_fsverity_merkle_key_to_disk(&name, pos);
> +	args.name = (const uint8_t *)&name.merkleoff;
> +
> +	return xfs_attr_set(&args);
> +}
> +
> +static void
> +xfs_drop_block(
> +	struct fsverity_block	*block)
> +{
> +	struct xfs_buf		*buf;
> +
> +	ASSERT(block != NULL);
> +
> +	buf = (struct xfs_buf *)block->context;
> +
> +	if (block->cached)
> +		buf->b_flags |= XBF_VERITY_CHECKED;
> +	xfs_buf_rele(buf);
> +
> +	kunmap_local(block->kaddr);
> +}
> +
> +const struct fsverity_operations xfs_verity_ops = {
> +	.begin_enable_verity		= &xfs_begin_enable_verity,
> +	.end_enable_verity		= &xfs_end_enable_verity,
> +	.get_verity_descriptor		= &xfs_get_verity_descriptor,
> +	.read_merkle_tree_block		= &xfs_read_merkle_tree_block,
> +	.write_merkle_tree_block	= &xfs_write_merkle_tree_block,
> +	.drop_block			= &xfs_drop_block,
> +};
> diff --git a/fs/xfs/xfs_verity.h b/fs/xfs/xfs_verity.h
> new file mode 100644
> index 000000000000..0f32fd212091
> --- /dev/null
> +++ b/fs/xfs/xfs_verity.h
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Red Hat, Inc.
> + */
> +#ifndef __XFS_VERITY_H__
> +#define __XFS_VERITY_H__
> +
> +#include "xfs.h"
> +#include "xfs_da_format.h"
> +#include "xfs_da_btree.h"
> +#include <linux/fsverity.h>
> +
> +#define XFS_VERITY_DESCRIPTOR_NAME "verity_descriptor"
> +#define XFS_VERITY_DESCRIPTOR_NAME_LEN 17

Want to shorten this by one for u64 alignment? ;)

> +
> +static inline bool
> +xfs_verity_merkle_block(
> +		struct xfs_da_args *args)
> +{
> +	if (!(args->attr_filter & XFS_ATTR_VERITY))
> +		return false;
> +
> +	if (!(args->op_flags & XFS_DA_OP_BUFFER))
> +		return false;
> +
> +	if (args->valuelen < 1024 || args->valuelen > PAGE_SIZE ||
> +			!is_power_of_2(args->valuelen))
> +		return false;

Why do we check the valuelen here?

Also, if you're passing the xfs_buf out, I thought the buffer cache
could handle weird sizes?

> +
> +	return true;
> +}
> +
> +#ifdef CONFIG_FS_VERITY
> +extern const struct fsverity_operations xfs_verity_ops;
> +#endif	/* CONFIG_FS_VERITY */
> +
> +#endif	/* __XFS_VERITY_H__ */
> -- 
> 2.40.1
> 

  parent reply	other threads:[~2023-10-11  1:39 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-06 18:48 [PATCH v3 00/28] fs-verity support for XFS Andrey Albershteyn
2023-10-06 18:48 ` [PATCH v3 01/28] xfs: Add new name to attri/d Andrey Albershteyn
2023-10-06 18:48 ` [PATCH v3 02/28] xfs: add parent pointer support to attribute code Andrey Albershteyn
2023-10-06 18:48 ` [PATCH v3 03/28] xfs: define parent pointer xattr format Andrey Albershteyn
2023-10-06 18:48 ` [PATCH v3 04/28] xfs: Add xfs_verify_pptr Andrey Albershteyn
2023-10-11  1:01   ` Darrick J. Wong
2023-10-11 11:09     ` Andrey Albershteyn
2023-10-06 18:48 ` [PATCH v3 05/28] fs: add FS_XFLAG_VERITY for fs-verity sealed inodes Andrey Albershteyn
2023-10-11  4:05   ` Eric Biggers
2023-10-11 11:06     ` Andrey Albershteyn
2023-10-06 18:49 ` [PATCH v3 06/28] fsverity: add drop_page() callout Andrey Albershteyn
2023-10-11  3:06   ` Eric Biggers
2023-10-11 11:11     ` Andrey Albershteyn
2023-10-06 18:49 ` [PATCH v3 07/28] fsverity: always use bitmap to track verified status Andrey Albershteyn
2023-10-11  3:15   ` Eric Biggers
2023-10-11 13:03     ` Andrey Albershteyn
2023-10-12  7:27       ` Eric Biggers
2023-10-13  3:12         ` Darrick J. Wong
2023-10-17  4:58           ` Eric Biggers
2023-10-18  2:35             ` Darrick J. Wong
2023-10-17  6:01           ` Christoph Hellwig
2023-10-16 11:52         ` Andrey Albershteyn
2023-10-17  5:57         ` Christoph Hellwig
2023-10-17 17:49           ` Eric Biggers
2023-10-06 18:49 ` [PATCH v3 08/28] fsverity: pass Merkle tree block size to ->read_merkle_tree_page() Andrey Albershteyn
2023-10-11  3:17   ` Eric Biggers
2023-10-11 11:13     ` Andrey Albershteyn
2023-10-06 18:49 ` [PATCH v3 09/28] fsverity: pass log_blocksize to end_enable_verity() Andrey Albershteyn
2023-10-11  3:19   ` Eric Biggers
2023-10-11 11:17     ` Andrey Albershteyn
2023-10-12  7:34       ` Eric Biggers
2023-10-06 18:49 ` [PATCH v3 10/28] fsverity: operate with Merkle tree blocks instead of pages Andrey Albershteyn
2023-10-07  4:02   ` kernel test robot
2023-10-11  3:56   ` Eric Biggers
2023-10-16 13:00   ` Christoph Hellwig
2023-10-06 18:49 ` [PATCH v3 11/28] iomap: pass readpage operation to read path Andrey Albershteyn
2023-10-11 18:31   ` Darrick J. Wong
2023-10-16 12:35     ` Andrey Albershteyn
2023-10-16  9:15   ` Christoph Hellwig
2023-10-16 12:32     ` Andrey Albershteyn
2023-10-16 12:58       ` Christoph Hellwig
2023-10-06 18:49 ` [PATCH v3 12/28] iomap: allow filesystem to implement read path verification Andrey Albershteyn
2023-10-11 18:39   ` Darrick J. Wong
2023-10-06 18:49 ` [PATCH v3 13/28] xfs: add XBF_VERITY_CHECKED xfs_buf flag Andrey Albershteyn
2023-10-11 18:54   ` Darrick J. Wong
2023-10-06 18:49 ` [PATCH v3 14/28] xfs: add XFS_DA_OP_BUFFER to make xfs_attr_get() return buffer Andrey Albershteyn
2023-10-06 18:49 ` [PATCH v3 15/28] xfs: introduce workqueue for post read IO work Andrey Albershteyn
2023-10-11 18:55   ` Darrick J. Wong
2023-10-16 12:37     ` Andrey Albershteyn
2023-10-06 18:49 ` [PATCH v3 16/28] xfs: add bio_set and submit_io for ioend post-processing Andrey Albershteyn
2023-10-11 18:47   ` Darrick J. Wong
2023-10-06 18:49 ` [PATCH v3 17/28] xfs: add attribute type for fs-verity Andrey Albershteyn
2023-10-11 18:48   ` Darrick J. Wong
2023-10-06 18:49 ` [PATCH v3 18/28] xfs: make xfs_buf_get() to take XBF_* flags Andrey Albershteyn
2023-10-06 18:49 ` [PATCH v3 19/28] xfs: add XBF_DOUBLE_ALLOC to increase size of the buffer Andrey Albershteyn
2023-10-06 18:49 ` [PATCH v3 20/28] xfs: add fs-verity ro-compat flag Andrey Albershteyn
2023-10-11 18:56   ` Darrick J. Wong
2023-10-06 18:49 ` [PATCH v3 21/28] xfs: add inode on-disk VERITY flag Andrey Albershteyn
2023-10-11 18:57   ` Darrick J. Wong
2023-10-06 18:49 ` [PATCH v3 22/28] xfs: initialize fs-verity on file open and cleanup on inode destruction Andrey Albershteyn
2023-10-06 18:49 ` [PATCH v3 23/28] xfs: don't allow to enable DAX on fs-verity sealsed inode Andrey Albershteyn
2023-10-11 19:00   ` Darrick J. Wong
2023-10-06 18:49 ` [PATCH v3 24/28] xfs: disable direct read path for fs-verity sealed files Andrey Albershteyn
2023-10-11 19:02   ` Darrick J. Wong
2023-10-06 18:49 ` [PATCH v3 25/28] xfs: add fs-verity support Andrey Albershteyn
2023-10-06 23:40   ` kernel test robot
2023-10-11  1:39   ` Darrick J. Wong [this message]
2023-10-11 14:36     ` Andrey Albershteyn
2023-10-18 19:18   ` kernel test robot
2023-10-06 18:49 ` [PATCH v3 26/28] xfs: make scrub aware of verity dinode flag Andrey Albershteyn
2023-10-11  1:06   ` Darrick J. Wong
2023-10-11 14:37     ` Andrey Albershteyn
2023-10-06 18:49 ` [PATCH v3 27/28] xfs: add fs-verity ioctls Andrey Albershteyn
2023-10-06 18:49 ` [PATCH v3 28/28] xfs: enable ro-compat fs-verity flag Andrey Albershteyn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231011013940.GJ21298@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=aalbersh@redhat.com \
    --cc=david@fromorbit.com \
    --cc=dchinner@redhat.com \
    --cc=ebiggers@kernel.org \
    --cc=fsverity@lists.linux.dev \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).