fsverity: use shash API instead of ahash API
Subsystems (4)
Eric Biggers
maintainerRFC PATCH 1/1 fsverity: use shash API instead of ahash API
From: Eric Biggers <[email protected]>
The "ahash" API, like the other scatterlist-based crypto APIs such as "skcipher", comes with some well-known limitations. First, it can't easily be used with vmalloc addresses. Second, the request struct can't be allocated on the stack. This adds complexity and a possible failure point that needs to be worked around, e.g. using a mempool.
The only benefit of ahash over "shash" is that ahash is needed to access traditional memory-to-memory crypto accelerators, i.e. drivers/crypto/. However, this style of crypto acceleration has largely fallen out of favor and been superseded by CPU-based acceleration or inline crypto engines. Also, ahash needs to be used asynchronously to take full advantage of such hardware, but fs/verity/ has never done this.
On all systems that aren't actually using one of these ahash-only crypto accelerators, ahash just adds unnecessary overhead as it sits between the user and the underlying shash algorithms.
Also, XFS is planned to cache fsverity Merkle tree blocks in the existing XFS buffer cache. As a result, it will be possible for a single Merkle tree block to be split across discontiguous pages
(https://lore.kernel.org/r/[email protected]).
This data will need to be hashed. It is easiest to work with a vmapped address in this case. However, ahash is incompatible with this.
Therefore, let's convert fs/verity/ from ahash to shash. This simplifies the code, and it should also slightly improve performance for everyone who wasn't actually using one of these ahash-only crypto accelerators, i.e. almost everyone (or maybe even everyone)!
--- fs/verity/enable.c | 19 ++--- fs/verity/fsverity_private.h | 13 +--- fs/verity/hash_algs.c | 131 ++++++----------------------------- fs/verity/verify.c | 108 +++++++++++------------------ 4 files changed, 71 insertions(+), 200 deletions(-)
fs/verity/enable.c +6 -13
@@ -7,6 +7,7 @@#include "fsverity_private.h" +#include <crypto/hash.h> #include <linux/mount.h> #include <linux/sched/signal.h> #include <linux/uaccess.h>
@@ -19,7 +20,7 @@ struct block_buffer {/* Hash a block, writing the result to the next level's pending block buffer. */ static int hash_one_block (struct inode *inode,const struct merkle_tree_params *params, -struct ahash_request *req,struct block_buffer *cur) +struct block_buffer *cur) {struct block_buffer *next = cur +1 ;int err;
@@ -27,8 +28,7 @@ static int hash_one_block(struct inode *inode,/* Zero-pad the block if it's shorter than the block size. */ memset (&cur->data[cur->filled],0 , params->block_size - cur->filled); - err =fsverity_hash_block (params, inode, req,virt_to_page (cur->data), -offset_in_page (cur->data), + err =fsverity_hash_block (params, inode, cur->data, &next->data[next->filled]);if (err)return err;
@@ -67,7 +67,6 @@ static int build_merkle_tree(struct file *filp,struct inode *inode =file_inode (filp);const u64 data_size = inode->i_size;const int num_levels = params->num_levels; -struct ahash_request *req;struct block_buffer _buffers[1 +FS_VERITY_MAX_LEVELS +1 ] = {};struct block_buffer *buffers = &_buffers[1 ];unsigned long level_offset[FS_VERITY_MAX_LEVELS ];
@@ -81,9 +80,6 @@ static int build_merkle_tree(struct file *filp,return 0 ; } -/* This allocation never fails, since it's mempool-backed. */ - req =fsverity_alloc_hash_request (params->hash_alg,GFP_KERNEL ); -/* * Allocate the block buffers. Buffer "-1" is for data blocks. * Buffers 0 <= level < num_levels are for the actual tree levels.
@@ -120,7 +116,7 @@ static int build_merkle_tree(struct file *filp,fsverity_err (inode,"Short read of file data" ); goto out; } - err =hash_one_block (inode, params, req, &buffers[-1 ]); + err =hash_one_block (inode, params, &buffers[-1 ]);if (err) goto out;for (level =0 ; level < num_levels; level++) {
@@ -131,8 +127,7 @@ static int build_merkle_tree(struct file *filp, }/* Next block at @level is full */ - err =hash_one_block (inode, params, req, - &buffers[level]); + err =hash_one_block (inode, params, &buffers[level]);if (err) goto out; err = write_merkle_tree_block(inode,
@@ -152,8 +147,7 @@ static int build_merkle_tree(struct file *filp,/* Finish all nonempty pending tree blocks. */ for (level =0 ; level < num_levels; level++) {if (buffers[level].filled !=0 ) { - err =hash_one_block (inode, params, req, - &buffers[level]); + err =hash_one_block (inode, params, &buffers[level]);if (err) goto out; err = write_merkle_tree_block(inode,
@@ -173,7 +167,6 @@ static int build_merkle_tree(struct file *filp, out:for (level =-1 ; level < num_levels; level++)kfree (buffers[level].data); -fsverity_free_hash_request (params->hash_alg, req);return err; }
fs/verity/fsverity_private.h +2 -11
@@ -11,9 +11,6 @@#define pr_fmt (fmt) "fs-verity: " fmt#include <linux/fsverity.h> -#include <linux/mempool.h> - -struct ahash_request ;/* * Implementation limit: maximum depth of the Merkle tree. For now 8 is plenty;
@@ -23,11 +20,10 @@ struct ahash_request;/* A hash algorithm supported by fs-verity */ struct fsverity_hash_alg { -struct crypto_ahash *tfm;/* hash tfm, allocated on demand */ +struct crypto_shash *tfm;/* hash tfm, allocated on demand */ const char *name;/* crypto API name, e.g. sha256 */ unsigned int digest_size;/* digest size in bytes, e.g. 32 for SHA-256 */ unsigned int block_size;/* block size in bytes, e.g. 64 for SHA-256 */ -mempool_t req_pool;/* mempool with a preallocated hash request */ /* * The HASH_ALGO_* constant for this algorithm. This is different from * FS_VERITY_HASH_ALG_*, which uses a different numbering scheme.
@@ -85,15 +81,10 @@ extern struct fsverity_hash_alg fsverity_hash_algs[];struct fsverity_hash_alg *fsverity_get_hash_alg (const struct inode *inode,unsigned int num); -struct ahash_request *fsverity_alloc_hash_request (struct fsverity_hash_alg *alg, -gfp_t gfp_flags); -void fsverity_free_hash_request (struct fsverity_hash_alg *alg, -struct ahash_request *req);const u8 *fsverity_prepare_hash_state (struct fsverity_hash_alg *alg,const u8 *salt,size_t salt_size);int fsverity_hash_block (const struct merkle_tree_params *params, -const struct inode *inode,struct ahash_request *req, -struct page *page,unsigned int offset,u8 *out); +const struct inode *inode,const void *data,u8 *out);int fsverity_hash_buffer (struct fsverity_hash_alg *alg,const void *data,size_t size,u8 *out);void __init fsverity_check_hash_algs (void );
fs/verity/hash_algs.c +21 -110
237 lines not on this page · raw message
fs/verity/verify.c +42 -66
228 lines not on this page · raw message
base-commit: 99ddf2254febae9eab7fb0bcc02c5322243f5c49
Signature · 1 line
2.40.0
Quoted 4 lines · Eric Biggers
Therefore, let's convert
fs/verity/from ahash to shash. This simplifies the code, and it should also slightly improve performance for everyone who wasn't actually using one of these ahash-only crypto accelerators, i.e. almost everyone (or maybe even everyone)!
This looks like a very nice cleanup to me. So unless someone really uses async crypto offload heavily for fsverity and can come up with a convincing argument for that, I'm all for the change.