[PATCH v3 07/28] fsverity: always use bitmap to track verified status

Andrey Albershteyn
2023-10-06T18:54:02Z

The bitmap is used to track verified status of the Merkle tree
blocks which are smaller than a PAGE. Blocks which fits exactly in a
page - use PageChecked() for tracking "verified" status.

This patch switches to always use bitmap to track verified status.
This is needed to move fs-verity away from page management and work
only with Merkle tree blocks.

Also, this patch removes spinlock. The lock was used to reset bits
in bitmap belonging to one page. This patch works only with one
Merkle tree block and won't reset other blocks status.

Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
---
fs/verity/fsverity_private.h | 1 -
fs/verity/open.c | 49 ++++++++++++-------------
fs/verity/verify.c | 71 +++++-------------------------------
3 files changed, 33 insertions(+), 88 deletions(-)

diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
index d071a6e32581..9611eeae3527 100644
--- a/fs/verity/fsverity_private.h
+++ b/fs/verity/fsverity_private.h
@@ -69,7 +69,6 @@ struct fsverity_info {
u8 file_digest[FS_VERITY_MAX_DIGEST_SIZE];
const struct inode *inode;
unsigned long *hash_block_verified;
- spinlock_t hash_page_init_lock;
};

#define FS_VERITY_MAX_SIGNATURE_SIZE (FS_VERITY_MAX_DESCRIPTOR_SIZE - \
diff --git a/fs/verity/open.c b/fs/verity/open.c
index 6c31a871b84b..dfb9fe6aaae9 100644
--- a/fs/verity/open.c
+++ b/fs/verity/open.c
@@ -182,6 +182,7 @@ struct fsverity_info *fsverity_create_info(const struct inode *inode,
{
struct fsverity_info *vi;
int err;
+ unsigned long num_bits;

vi = kmem_cache_zalloc(fsverity_info_cachep, GFP_KERNEL);
if (!vi)
@@ -213,33 +214,29 @@ struct fsverity_info *fsverity_create_info(const struct inode *inode,
if (err)
goto fail;

- if (vi->tree_params.block_size != PAGE_SIZE) {
- /*
- * When the Merkle tree block size and page size differ, we use
- * a bitmap to keep track of which hash blocks have been
- * verified. This bitmap must contain one bit per hash block,
- * including alignment to a page boundary at the end.
- *
- * Eventually, to support extremely large files in an efficient
- * way, it might be necessary to make pages of this bitmap
- * reclaimable. But for now, simply allocating the whole bitmap
- * is a simple solution that works well on the files on which
- * fsverity is realistically used. E.g., with SHA-256 and 4K
- * blocks, a 100MB file only needs a 24-byte bitmap, and the
- * bitmap for any file under 17GB fits in a 4K page.
- */
- unsigned long num_bits =
- vi->tree_params.tree_pages <<
- vi->tree_params.log_blocks_per_page;
+ /*
+ * We use a bitmap to keep track of which hash blocks have been
+ * verified. This bitmap must contain one bit per hash block,
+ * including alignment to a page boundary at the end.
+ *
+ * Eventually, to support extremely large files in an efficient
+ * way, it might be necessary to make pages of this bitmap
+ * reclaimable. But for now, simply allocating the whole bitmap
+ * is a simple solution that works well on the files on which
+ * fsverity is realistically used. E.g., with SHA-256 and 4K
+ * blocks, a 100MB file only needs a 24-byte bitmap, and the
+ * bitmap for any file under 17GB fits in a 4K page.
+ */
+ num_bits =
+ vi->tree_params.tree_pages <<
+ vi->tree_params.log_blocks_per_page;

- vi->hash_block_verified = kvcalloc(BITS_TO_LONGS(num_bits),
- sizeof(unsigned long),
- GFP_KERNEL);
- if (!vi->hash_block_verified) {
- err = -ENOMEM;
- goto fail;
- }
- spin_lock_init(&vi->hash_page_init_lock);
+ vi->hash_block_verified = kvcalloc(BITS_TO_LONGS(num_bits),
+ sizeof(unsigned long),
+ GFP_KERNEL);
+ if (!vi->hash_block_verified) {
+ err = -ENOMEM;
+ goto fail;
}

return vi;
diff --git a/fs/verity/verify.c b/fs/verity/verify.c
index 2fe7bd57b16e..e7b13d143ae9 100644
--- a/fs/verity/verify.c
+++ b/fs/verity/verify.c
@@ -13,69 +13,18 @@
static struct workqueue_struct *fsverity_read_workqueue;

/*
- * Returns true if the hash block with index @hblock_idx in the tree, located in
- * @hpage, has already been verified.
+ * Returns true if the hash block with index @hblock_idx in the tree has
+ * already been verified.
*/
-static bool is_hash_block_verified(struct fsverity_info *vi, struct page *hpage,
- unsigned long hblock_idx)
+static bool is_hash_block_verified(struct fsverity_info *vi,
+ unsigned long hblock_idx,
+ bool block_cached)
{
- bool verified;
- unsigned int blocks_per_page;
- unsigned int i;
-
- /*
- * When the Merkle tree block size and page size are the same, then the
- * ->hash_block_verified bitmap isn't allocated, and we use PG_checked
- * to directly indicate whether the page's block has been verified.
- *
- * Using PG_checked also guarantees that we re-verify hash pages that
- * get evicted and re-instantiated from the backing storage, as new
- * pages always start out with PG_checked cleared.
- */
- if (!vi->hash_block_verified)
- return PageChecked(hpage);
-
- /*
- * When the Merkle tree block size and page size differ, we use a bitmap
- * to indicate whether each hash block has been verified.
- *
- * However, we still need to ensure that hash pages that get evicted and
- * re-instantiated from the backing storage are re-verified. To do
- * this, we use PG_checked again, but now it doesn't really mean
- * "checked". Instead, now it just serves as an indicator for whether
- * the hash page is newly instantiated or not.
- *
- * The first thread that sees PG_checked=0 must clear the corresponding
- * bitmap bits, then set PG_checked=1. This requires a spinlock. To
- * avoid having to take this spinlock in the common case of
- * PG_checked=1, we start with an opportunistic lockless read.
- */
- if (PageChecked(hpage)) {
- /*
- * A read memory barrier is needed here to give ACQUIRE
- * semantics to the above PageChecked() test.
- */
- smp_rmb();
+ if (block_cached)
return test_bit(hblock_idx, vi->hash_block_verified);
- }
- spin_lock(&vi->hash_page_init_lock);
- if (PageChecked(hpage)) {
- verified = test_bit(hblock_idx, vi->hash_block_verified);
- } else {
- blocks_per_page = vi->tree_params.blocks_per_page;
- hblock_idx = round_down(hblock_idx, blocks_per_page);
- for (i = 0; i < blocks_per_page; i++)
- clear_bit(hblock_idx + i, vi->hash_block_verified);
- /*
- * A write memory barrier is needed here to give RELEASE
- * semantics to the below SetPageChecked() operation.
- */
- smp_wmb();
- SetPageChecked(hpage);
- verified = false;
- }
- spin_unlock(&vi->hash_page_init_lock);
- return verified;
+
+ clear_bit(hblock_idx, vi->hash_block_verified);
+ return false;
}

/*
@@ -179,7 +128,7 @@ verify_data_block(struct inode *inode, struct fsverity_info *vi,
goto error;
}
haddr = kmap_local_page(hpage) + hblock_offset_in_page;
- if (is_hash_block_verified(vi, hpage, hblock_idx)) {
+ if (is_hash_block_verified(vi, hblock_idx, PageChecked(hpage))) {
memcpy(_want_hash, haddr + hoffset, hsize);
want_hash = _want_hash;
kunmap_local(haddr);
--
2.40.1