| Message ID | 171069246408.2684506.9245902616854244173.stgit@frogsfrogsfrogs (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | [01/40] fsverity: remove hash page spin lock | expand |
On 2024-03-17 09:31:28, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > xfs_drop_merkle_tree is responsible for removing the fsverity metadata > after a failed attempt to enable fsverity for a file. However, if the > enablement process fails before the verity descriptor is written to the > file, the cleanup function will trip the WARN_ON. The error code in > that case is ENOATTR, which isn't worth logging about. > > Fix that return code handling, fix the tree block removal loop not to > return early with ENOATTR, and improve the logging so that we actually > capture what kind of error occurred. > > 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 | 25 ++++++++++++++++++------- > 1 file changed, 18 insertions(+), 7 deletions(-) > > > diff --git a/fs/xfs/xfs_verity.c b/fs/xfs/xfs_verity.c > index db43e017f10e..32891ae42c47 100644 > --- a/fs/xfs/xfs_verity.c > +++ b/fs/xfs/xfs_verity.c > @@ -472,15 +472,14 @@ xfs_verity_begin_enable( > tree_blocksize); > } > > +/* Try to remove all the fsverity metadata after a failed enablement. */ > static int > -xfs_drop_merkle_tree( > +xfs_verity_drop_incomplete_tree( > struct xfs_inode *ip, > u64 merkle_tree_size, > unsigned int tree_blocksize) > { > struct xfs_verity_merkle_key name; > - int error = 0; > - u64 offset = 0; > struct xfs_da_args args = { > .dp = ip, > .whichfork = XFS_ATTR_FORK, > @@ -491,6 +490,8 @@ xfs_drop_merkle_tree( > /* NULL value make xfs_attr_set remove the attr */ > .value = NULL, > }; > + u64 offset; > + int error; > > if (!merkle_tree_size) > return 0; > @@ -498,6 +499,8 @@ xfs_drop_merkle_tree( > for (offset = 0; offset < merkle_tree_size; offset += tree_blocksize) { > xfs_verity_merkle_key_to_disk(&name, offset); > error = xfs_attr_set(&args); > + if (error == -ENOATTR) > + error = 0; > if (error) > return error; > } > @@ -505,7 +508,8 @@ xfs_drop_merkle_tree( > args.name = (const uint8_t *)XFS_VERITY_DESCRIPTOR_NAME; > args.namelen = XFS_VERITY_DESCRIPTOR_NAME_LEN; > error = xfs_attr_set(&args); > - > + if (error == -ENOATTR) > + return 0; > return error; > } > > @@ -564,9 +568,16 @@ xfs_verity_end_enable( > inode->i_flags |= S_VERITY; > > out: > - if (error) > - WARN_ON_ONCE(xfs_drop_merkle_tree(ip, merkle_tree_size, > - tree_blocksize)); > + if (error) { > + int error2; > + > + error2 = xfs_verity_drop_incomplete_tree(ip, merkle_tree_size, > + tree_blocksize); > + if (error2) > + xfs_alert(ip->i_mount, > + "ino 0x%llx failed to clean up new fsverity metadata, err %d", > + ip->i_ino, error2); > + } > > xfs_iflags_clear(ip, XFS_VERITY_CONSTRUCTION); > return error; >
diff --git a/fs/xfs/xfs_verity.c b/fs/xfs/xfs_verity.c index db43e017f10e..32891ae42c47 100644 --- a/fs/xfs/xfs_verity.c +++ b/fs/xfs/xfs_verity.c @@ -472,15 +472,14 @@ xfs_verity_begin_enable( tree_blocksize); } +/* Try to remove all the fsverity metadata after a failed enablement. */ static int -xfs_drop_merkle_tree( +xfs_verity_drop_incomplete_tree( struct xfs_inode *ip, u64 merkle_tree_size, unsigned int tree_blocksize) { struct xfs_verity_merkle_key name; - int error = 0; - u64 offset = 0; struct xfs_da_args args = { .dp = ip, .whichfork = XFS_ATTR_FORK, @@ -491,6 +490,8 @@ xfs_drop_merkle_tree( /* NULL value make xfs_attr_set remove the attr */ .value = NULL, }; + u64 offset; + int error; if (!merkle_tree_size) return 0; @@ -498,6 +499,8 @@ xfs_drop_merkle_tree( for (offset = 0; offset < merkle_tree_size; offset += tree_blocksize) { xfs_verity_merkle_key_to_disk(&name, offset); error = xfs_attr_set(&args); + if (error == -ENOATTR) + error = 0; if (error) return error; } @@ -505,7 +508,8 @@ xfs_drop_merkle_tree( args.name = (const uint8_t *)XFS_VERITY_DESCRIPTOR_NAME; args.namelen = XFS_VERITY_DESCRIPTOR_NAME_LEN; error = xfs_attr_set(&args); - + if (error == -ENOATTR) + return 0; return error; } @@ -564,9 +568,16 @@ xfs_verity_end_enable( inode->i_flags |= S_VERITY; out: - if (error) - WARN_ON_ONCE(xfs_drop_merkle_tree(ip, merkle_tree_size, - tree_blocksize)); + if (error) { + int error2; + + error2 = xfs_verity_drop_incomplete_tree(ip, merkle_tree_size, + tree_blocksize); + if (error2) + xfs_alert(ip->i_mount, + "ino 0x%llx failed to clean up new fsverity metadata, err %d", + ip->i_ino, error2); + } xfs_iflags_clear(ip, XFS_VERITY_CONSTRUCTION); return error;