diff mbox series

[30/40] xfs: clean up stale fsverity metadata before starting

Message ID 171069246392.2684506.14484170564314714404.stgit@frogsfrogsfrogs (mailing list archive)
State Deferred, archived
Headers show
Series [01/40] fsverity: remove hash page spin lock | expand

Commit Message

Darrick J. Wong March 17, 2024, 4:31 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

Before we let fsverity begin writing merkle tree blocks to the file,
let's perform a minor effort to clean up any stale metadata from a
previous attempt to enable fsverity.  This can only happen if the system
crashes /and/ the file shrinks, which is unlikely.  But we could do a
better job of cleaning up anyway.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/xfs_verity.c |   42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

Comments

Andrey Albershteyn March 18, 2024, 5:50 p.m. UTC | #1 
On 2024-03-17 09:31:13, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Before we let fsverity begin writing merkle tree blocks to the file,
> let's perform a minor effort to clean up any stale metadata from a
> previous attempt to enable fsverity.  This can only happen if the system
> crashes /and/ the file shrinks, which is unlikely.  But we could do a
> better job of cleaning up anyway.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>

Looks good to me:
Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>

> ---
>  fs/xfs/xfs_verity.c |   42 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 40 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_verity.c b/fs/xfs/xfs_verity.c
> index c19fa47d1f76..db43e017f10e 100644
> --- a/fs/xfs/xfs_verity.c
> +++ b/fs/xfs/xfs_verity.c
> @@ -413,6 +413,44 @@ xfs_verity_get_descriptor(
>  	return args.valuelen;
>  }
>  
> +/*
> + * Clear out old fsverity metadata before we start building a new one.  This
> + * could happen if, say, we crashed while building fsverity data.
> + */
> +static int
> +xfs_verity_drop_old_metadata(
> +	struct xfs_inode		*ip,
> +	u64				new_tree_size,
> +	unsigned int			tree_blocksize)
> +{
> +	struct xfs_verity_merkle_key	name;
> +	struct xfs_da_args		args = {
> +		.dp			= ip,
> +		.whichfork		= XFS_ATTR_FORK,
> +		.attr_filter		= XFS_ATTR_VERITY,
> +		.op_flags		= XFS_DA_OP_REMOVE,
> +		.name			= (const uint8_t *)&name,
> +		.namelen		= sizeof(struct xfs_verity_merkle_key),
> +		/* NULL value make xfs_attr_set remove the attr */
> +		.value			= NULL,
> +	};
> +	u64				offset;
> +	int				error = 0;
> +
> +	/*
> +	 * Delete as many merkle tree blocks in increasing blkno order until we
> +	 * don't find any more.  That ought to be good enough for avoiding
> +	 * dead bloat without excessive runtime.
> +	 */
> +	for (offset = new_tree_size; !error; offset += tree_blocksize) {
> +		xfs_verity_merkle_key_to_disk(&name, offset);
> +		error = xfs_attr_set(&args);
> +	}
> +	if (error == -ENOATTR)
> +		return 0;
> +	return error;
> +}
> +
>  static int
>  xfs_verity_begin_enable(
>  	struct file		*filp,
> @@ -421,7 +459,6 @@ xfs_verity_begin_enable(
>  {
>  	struct inode		*inode = file_inode(filp);
>  	struct xfs_inode	*ip = XFS_I(inode);
> -	int			error = 0;
>  
>  	xfs_assert_ilocked(ip, XFS_IOLOCK_EXCL);
>  
> @@ -431,7 +468,8 @@ xfs_verity_begin_enable(
>  	if (xfs_iflags_test_and_set(ip, XFS_VERITY_CONSTRUCTION))
>  		return -EBUSY;
>  
> -	return error;
> +	return xfs_verity_drop_old_metadata(ip, merkle_tree_size,
> +			tree_blocksize);
>  }
>  
>  static int
>
diff mbox series

Patch

diff --git a/fs/xfs/xfs_verity.c b/fs/xfs/xfs_verity.c
index c19fa47d1f76..db43e017f10e 100644
--- a/fs/xfs/xfs_verity.c
+++ b/fs/xfs/xfs_verity.c
@@ -413,6 +413,44 @@  xfs_verity_get_descriptor(
 	return args.valuelen;
 }
 
+/*
+ * Clear out old fsverity metadata before we start building a new one.  This
+ * could happen if, say, we crashed while building fsverity data.
+ */
+static int
+xfs_verity_drop_old_metadata(
+	struct xfs_inode		*ip,
+	u64				new_tree_size,
+	unsigned int			tree_blocksize)
+{
+	struct xfs_verity_merkle_key	name;
+	struct xfs_da_args		args = {
+		.dp			= ip,
+		.whichfork		= XFS_ATTR_FORK,
+		.attr_filter		= XFS_ATTR_VERITY,
+		.op_flags		= XFS_DA_OP_REMOVE,
+		.name			= (const uint8_t *)&name,
+		.namelen		= sizeof(struct xfs_verity_merkle_key),
+		/* NULL value make xfs_attr_set remove the attr */
+		.value			= NULL,
+	};
+	u64				offset;
+	int				error = 0;
+
+	/*
+	 * Delete as many merkle tree blocks in increasing blkno order until we
+	 * don't find any more.  That ought to be good enough for avoiding
+	 * dead bloat without excessive runtime.
+	 */
+	for (offset = new_tree_size; !error; offset += tree_blocksize) {
+		xfs_verity_merkle_key_to_disk(&name, offset);
+		error = xfs_attr_set(&args);
+	}
+	if (error == -ENOATTR)
+		return 0;
+	return error;
+}
+
 static int
 xfs_verity_begin_enable(
 	struct file		*filp,
@@ -421,7 +459,6 @@  xfs_verity_begin_enable(
 {
 	struct inode		*inode = file_inode(filp);
 	struct xfs_inode	*ip = XFS_I(inode);
-	int			error = 0;
 
 	xfs_assert_ilocked(ip, XFS_IOLOCK_EXCL);
 
@@ -431,7 +468,8 @@  xfs_verity_begin_enable(
 	if (xfs_iflags_test_and_set(ip, XFS_VERITY_CONSTRUCTION))
 		return -EBUSY;
 
-	return error;
+	return xfs_verity_drop_old_metadata(ip, merkle_tree_size,
+			tree_blocksize);
 }
 
 static int