fsverity: Set S_VERITY only after rollback is no longer possible

Commit f77f281b61 ("fsverity: use a hashtable to find the
fsverity_info") changed the implementation of FS_IOC_ENABLE_VERITY to
make concurrent reads start going through fsverity verification slightly
earlier than they did before.

Instead of it happening when enable_verity() calls fsverity_set_info(),
it now happens when ->end_enable_verity() sets S_VERITY.

However, ext4 and btrfs can roll back verity after they set S_VERITY,
causing a use-after-free.

Fix that by making ext4 and btrfs set S_VERITY only after rollback is no
longer possible.  On failure, don't roll back S_VERITY, but do roll back
the filesystem-specific verity flag.

Also update enable_verity() to handle any future instances of this bug
more safely by warning and *not* freeing the fsverity_info.

Finally, tweak some comments.  Clarify one, and move information from
another to its proper place in the comment for ->end_enable_verity().

Fixes: f77f281b61 ("fsverity: use a hashtable to find the fsverity_info")
Cc: stable@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://patch.msgid.link/20261025204807.122875-1-ebiggers@kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
This commit is contained in:
Eric Biggers 2026-10-25 13:48:07 -07:00
parent 28d74059fd
commit c349b233d6
4 changed files with 19 additions and 16 deletions

View File

@ -483,8 +483,6 @@ static int rollback_verity(struct btrfs_inode *inode)
inode->vfs_inode.i_ino);
goto out;
}
inode->ro_flags &= ~BTRFS_INODE_RO_VERITY;
btrfs_sync_inode_flags_to_i_flags(inode);
ret = btrfs_update_inode(trans, inode);
if (unlikely(ret)) {
btrfs_abort_transaction(trans, ret);
@ -549,16 +547,19 @@ static int finish_verity(struct btrfs_inode *inode, const void *desc,
if (IS_ERR(trans))
return PTR_ERR(trans);
inode->ro_flags |= BTRFS_INODE_RO_VERITY;
btrfs_sync_inode_flags_to_i_flags(inode);
ret = btrfs_update_inode(trans, inode);
if (ret)
goto end_trans;
ret = del_orphan(trans, inode);
if (ret)
goto end_trans;
/* Now that rollback is no longer possible, set S_VERITY. */
btrfs_sync_inode_flags_to_i_flags(inode);
clear_bit(BTRFS_INODE_VERITY_IN_PROGRESS, &inode->runtime_flags);
btrfs_set_fs_compat_ro(root->fs_info, VERITY);
end_trans:
if (ret)
inode->ro_flags &= ~BTRFS_INODE_RO_VERITY;
btrfs_end_transaction(trans);
return ret;

View File

@ -216,17 +216,19 @@ static int ext4_end_enable_verity(struct file *filp, const void *desc,
goto stop_and_cleanup;
ext4_set_inode_flag(inode, EXT4_INODE_VERITY);
ext4_set_inode_flags(inode, false);
err = ext4_mark_iloc_dirty(handle, inode, &iloc);
if (err)
goto stop_and_cleanup;
ext4_journal_stop(handle);
/* Now that rollback is no longer possible, set S_VERITY. */
ext4_set_inode_flags(inode, false);
ext4_clear_inode_state(inode, EXT4_STATE_VERITY_IN_PROGRESS);
return 0;
stop_and_cleanup:
ext4_clear_inode_flag(inode, EXT4_INODE_VERITY);
ext4_journal_stop(handle);
cleanup:
/*

View File

@ -276,8 +276,8 @@ static int enable_verity(struct file *filp,
* initialization so that we don't have to undo the enabling when memory
* allocation for the hash table fails. This is safe because looking up
* the fsverity_info always first checks the S_VERITY flag on the inode,
* which will only be set at the very end of the ->end_enable_verity
* method.
* which will only be set at the very end of a successful call to
* ->end_enable_verity().
*/
err = fsverity_set_info(vi);
if (err) {
@ -287,10 +287,7 @@ static int enable_verity(struct file *filp,
/*
* Tell the filesystem to finish enabling verity on the file.
* Serialized with ->begin_enable_verity() by the inode lock. The file
* system needs to set the S_VERITY flag on the inode at the very end of
* the method, at which point the fsverity information can be accessed
* by other threads.
* Serialized with ->begin_enable_verity() by the inode lock.
*/
inode_lock(inode);
err = vops->end_enable_verity(filp, desc, desc_size, params.tree_size);
@ -298,7 +295,8 @@ static int enable_verity(struct file *filp,
if (err) {
fsverity_err(inode, "%ps() failed with err %d",
vops->end_enable_verity, err);
fsverity_remove_info(vi);
if (!WARN_ON_ONCE(IS_VERITY(inode)))
fsverity_remove_info(vi);
} else if (WARN_ON_ONCE(!IS_VERITY(inode))) {
fsverity_remove_info(vi);
err = -EINVAL;

View File

@ -54,11 +54,13 @@ struct fsverity_operations {
* @merkle_tree_size: total bytes the Merkle tree took up
*
* If desc == NULL, then enabling verity failed and the filesystem only
* must do any necessary cleanups. Else, it must also store the given
* verity descriptor to a fs-specific location associated with the inode
* and do any fs-specific actions needed to mark the inode as a verity
* inode, e.g. setting a bit in the on-disk inode. The filesystem is
* also responsible for setting the S_VERITY flag in the VFS inode.
* must do any necessary cleanups.
*
* Else, the filesystem must store the given verity descriptor to a
* fs-specific location associated with the inode and do any fs-specific
* actions needed to mark the inode as a verity inode, e.g. set a bit in
* the on-disk inode. Finally, on success (only when rollback is no
* longer possible!) it must set S_VERITY in the VFS inode.
*
* i_rwsem is held for write, but it may have been dropped between
* ->begin_enable_verity() and ->end_enable_verity().