Re: [PATCH v5 06/24] fsverity: pass tree_blocksize to end_enable_verity()

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

On Mon, Mar 04, 2024 at 04:52:42PM -0800, Eric Biggers wrote:
> On Mon, Mar 04, 2024 at 08:10:29PM +0100, Andrey Albershteyn wrote:
> > XFS will need to know tree_blocksize to remove the tree in case of an
> > error. The size is needed to calculate offsets of particular Merkle
> > tree blocks.
> > 
> > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> > ---
> >  fs/btrfs/verity.c        | 4 +++-
> >  fs/ext4/verity.c         | 3 ++-
> >  fs/f2fs/verity.c         | 3 ++-
> >  fs/verity/enable.c       | 6 ++++--
> >  include/linux/fsverity.h | 4 +++-
> >  5 files changed, 14 insertions(+), 6 deletions(-)
> 
> How will XFS handle dropping a file's incomplete tree if the system crashes
> while it's being built?

AFAICT it simply leaves the half-constructed tree in the xattrs data.

> I think this is why none of the other filesystems have needed the tree_blocksize
> in ->end_enable_verity() yet.  They need to be able to drop the tree from just
> the information the filesystem has on-disk anyway.  ext4 and f2fs just truncate
> past EOF, while btrfs has some code that finds all the verity metadata items and
> deletes them (see btrfs_drop_verity_items() in fs/btrfs/verity.c).
> 
> Technically you don't *have* to drop incomplete trees, since it shouldn't cause
> a behavior difference.  But it seems like something that should be cleaned up.

If it's required that a failed FS_IOC_ENABLE_VERITY clean up the
unfinished merkle tree, then you'd have to introduce some kind of log
intent item to roll back blocks from an unfinished tree.  That log
item has to be committed as the first item in a chain of transactions,
each of which adds a merkle tree block and relogs the rollback item.
When we finish the tree, we log a done item to whiteout the intent.

Log recovery, upon finding an intent without the done item, will replay
the intent, which (in this case) will remove all the blocks.  In theory
a failed merkle tree commit also should do this, but most likely that
will cause an fs shutdown anyway.

(That's a fair amount of work.)

Or you could leave the unfinished tree as-is; that will waste space, but
if userspace tries again, the xattr code will replace the old merkle
tree block contents with the new ones.  This assumes that we're not
using XATTR_CREATE during FS_IOC_ENABLE_VERITY.

--D

> 
> - Eric
>