Skip to content

Commit 8fcd94a

Browse files
committed
fsverity: use shash API instead of ahash API
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/20230405233753.GU3223426@dread.disaster.area). 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)! Link: https://lore.kernel.org/r/20230516052306.99600-1-ebiggers@kernel.org Reviewed-by: Christoph Hellwig <hch@lst.de> Acked-by: Ard Biesheuvel <ardb@kernel.org> Signed-off-by: Eric Biggers <ebiggers@google.com>
1 parent f1fcbaa commit 8fcd94a

File tree

4 files changed

+71
-201
lines changed

4 files changed

+71
-201
lines changed

fs/verity/enable.c

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include "fsverity_private.h"
99

10+
#include <crypto/hash.h>
1011
#include <linux/mount.h>
1112
#include <linux/sched/signal.h>
1213
#include <linux/uaccess.h>
@@ -20,7 +21,7 @@ struct block_buffer {
2021
/* Hash a block, writing the result to the next level's pending block buffer. */
2122
static int hash_one_block(struct inode *inode,
2223
const struct merkle_tree_params *params,
23-
struct ahash_request *req, struct block_buffer *cur)
24+
struct block_buffer *cur)
2425
{
2526
struct block_buffer *next = cur + 1;
2627
int err;
@@ -36,8 +37,7 @@ static int hash_one_block(struct inode *inode,
3637
/* Zero-pad the block if it's shorter than the block size. */
3738
memset(&cur->data[cur->filled], 0, params->block_size - cur->filled);
3839

39-
err = fsverity_hash_block(params, inode, req, virt_to_page(cur->data),
40-
offset_in_page(cur->data),
40+
err = fsverity_hash_block(params, inode, cur->data,
4141
&next->data[next->filled]);
4242
if (err)
4343
return err;
@@ -76,7 +76,6 @@ static int build_merkle_tree(struct file *filp,
7676
struct inode *inode = file_inode(filp);
7777
const u64 data_size = inode->i_size;
7878
const int num_levels = params->num_levels;
79-
struct ahash_request *req;
8079
struct block_buffer _buffers[1 + FS_VERITY_MAX_LEVELS + 1] = {};
8180
struct block_buffer *buffers = &_buffers[1];
8281
unsigned long level_offset[FS_VERITY_MAX_LEVELS];
@@ -90,9 +89,6 @@ static int build_merkle_tree(struct file *filp,
9089
return 0;
9190
}
9291

93-
/* This allocation never fails, since it's mempool-backed. */
94-
req = fsverity_alloc_hash_request(params->hash_alg, GFP_KERNEL);
95-
9692
/*
9793
* Allocate the block buffers. Buffer "-1" is for data blocks.
9894
* Buffers 0 <= level < num_levels are for the actual tree levels.
@@ -130,7 +126,7 @@ static int build_merkle_tree(struct file *filp,
130126
fsverity_err(inode, "Short read of file data");
131127
goto out;
132128
}
133-
err = hash_one_block(inode, params, req, &buffers[-1]);
129+
err = hash_one_block(inode, params, &buffers[-1]);
134130
if (err)
135131
goto out;
136132
for (level = 0; level < num_levels; level++) {
@@ -141,8 +137,7 @@ static int build_merkle_tree(struct file *filp,
141137
}
142138
/* Next block at @level is full */
143139

144-
err = hash_one_block(inode, params, req,
145-
&buffers[level]);
140+
err = hash_one_block(inode, params, &buffers[level]);
146141
if (err)
147142
goto out;
148143
err = write_merkle_tree_block(inode,
@@ -162,8 +157,7 @@ static int build_merkle_tree(struct file *filp,
162157
/* Finish all nonempty pending tree blocks. */
163158
for (level = 0; level < num_levels; level++) {
164159
if (buffers[level].filled != 0) {
165-
err = hash_one_block(inode, params, req,
166-
&buffers[level]);
160+
err = hash_one_block(inode, params, &buffers[level]);
167161
if (err)
168162
goto out;
169163
err = write_merkle_tree_block(inode,
@@ -183,7 +177,6 @@ static int build_merkle_tree(struct file *filp,
183177
out:
184178
for (level = -1; level < num_levels; level++)
185179
kfree(buffers[level].data);
186-
fsverity_free_hash_request(params->hash_alg, req);
187180
return err;
188181
}
189182

fs/verity/fsverity_private.h

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,6 @@
1111
#define pr_fmt(fmt) "fs-verity: " fmt
1212

1313
#include <linux/fsverity.h>
14-
#include <linux/mempool.h>
15-
16-
struct ahash_request;
1714

1815
/*
1916
* Implementation limit: maximum depth of the Merkle tree. For now 8 is plenty;
@@ -23,11 +20,10 @@ struct ahash_request;
2320

2421
/* A hash algorithm supported by fs-verity */
2522
struct fsverity_hash_alg {
26-
struct crypto_ahash *tfm; /* hash tfm, allocated on demand */
23+
struct crypto_shash *tfm; /* hash tfm, allocated on demand */
2724
const char *name; /* crypto API name, e.g. sha256 */
2825
unsigned int digest_size; /* digest size in bytes, e.g. 32 for SHA-256 */
2926
unsigned int block_size; /* block size in bytes, e.g. 64 for SHA-256 */
30-
mempool_t req_pool; /* mempool with a preallocated hash request */
3127
/*
3228
* The HASH_ALGO_* constant for this algorithm. This is different from
3329
* FS_VERITY_HASH_ALG_*, which uses a different numbering scheme.
@@ -85,15 +81,10 @@ extern struct fsverity_hash_alg fsverity_hash_algs[];
8581

8682
struct fsverity_hash_alg *fsverity_get_hash_alg(const struct inode *inode,
8783
unsigned int num);
88-
struct ahash_request *fsverity_alloc_hash_request(struct fsverity_hash_alg *alg,
89-
gfp_t gfp_flags);
90-
void fsverity_free_hash_request(struct fsverity_hash_alg *alg,
91-
struct ahash_request *req);
9284
const u8 *fsverity_prepare_hash_state(struct fsverity_hash_alg *alg,
9385
const u8 *salt, size_t salt_size);
9486
int fsverity_hash_block(const struct merkle_tree_params *params,
95-
const struct inode *inode, struct ahash_request *req,
96-
struct page *page, unsigned int offset, u8 *out);
87+
const struct inode *inode, const void *data, u8 *out);
9788
int fsverity_hash_buffer(struct fsverity_hash_alg *alg,
9889
const void *data, size_t size, u8 *out);
9990
void __init fsverity_check_hash_algs(void);

fs/verity/hash_algs.c

Lines changed: 21 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
#include "fsverity_private.h"
99

1010
#include <crypto/hash.h>
11-
#include <linux/scatterlist.h>
1211

1312
/* The hash algorithms supported by fs-verity */
1413
struct fsverity_hash_alg fsverity_hash_algs[] = {
@@ -44,7 +43,7 @@ struct fsverity_hash_alg *fsverity_get_hash_alg(const struct inode *inode,
4443
unsigned int num)
4544
{
4645
struct fsverity_hash_alg *alg;
47-
struct crypto_ahash *tfm;
46+
struct crypto_shash *tfm;
4847
int err;
4948

5049
if (num >= ARRAY_SIZE(fsverity_hash_algs) ||
@@ -63,11 +62,7 @@ struct fsverity_hash_alg *fsverity_get_hash_alg(const struct inode *inode,
6362
if (alg->tfm != NULL)
6463
goto out_unlock;
6564

66-
/*
67-
* Using the shash API would make things a bit simpler, but the ahash
68-
* API is preferable as it allows the use of crypto accelerators.
69-
*/
70-
tfm = crypto_alloc_ahash(alg->name, 0, 0);
65+
tfm = crypto_alloc_shash(alg->name, 0, 0);
7166
if (IS_ERR(tfm)) {
7267
if (PTR_ERR(tfm) == -ENOENT) {
7368
fsverity_warn(inode,
@@ -84,68 +79,26 @@ struct fsverity_hash_alg *fsverity_get_hash_alg(const struct inode *inode,
8479
}
8580

8681
err = -EINVAL;
87-
if (WARN_ON_ONCE(alg->digest_size != crypto_ahash_digestsize(tfm)))
82+
if (WARN_ON_ONCE(alg->digest_size != crypto_shash_digestsize(tfm)))
8883
goto err_free_tfm;
89-
if (WARN_ON_ONCE(alg->block_size != crypto_ahash_blocksize(tfm)))
90-
goto err_free_tfm;
91-
92-
err = mempool_init_kmalloc_pool(&alg->req_pool, 1,
93-
sizeof(struct ahash_request) +
94-
crypto_ahash_reqsize(tfm));
95-
if (err)
84+
if (WARN_ON_ONCE(alg->block_size != crypto_shash_blocksize(tfm)))
9685
goto err_free_tfm;
9786

9887
pr_info("%s using implementation \"%s\"\n",
99-
alg->name, crypto_ahash_driver_name(tfm));
88+
alg->name, crypto_shash_driver_name(tfm));
10089

10190
/* pairs with smp_load_acquire() above */
10291
smp_store_release(&alg->tfm, tfm);
10392
goto out_unlock;
10493

10594
err_free_tfm:
106-
crypto_free_ahash(tfm);
95+
crypto_free_shash(tfm);
10796
alg = ERR_PTR(err);
10897
out_unlock:
10998
mutex_unlock(&fsverity_hash_alg_init_mutex);
11099
return alg;
111100
}
112101

113-
/**
114-
* fsverity_alloc_hash_request() - allocate a hash request object
115-
* @alg: the hash algorithm for which to allocate the request
116-
* @gfp_flags: memory allocation flags
117-
*
118-
* This is mempool-backed, so this never fails if __GFP_DIRECT_RECLAIM is set in
119-
* @gfp_flags. However, in that case this might need to wait for all
120-
* previously-allocated requests to be freed. So to avoid deadlocks, callers
121-
* must never need multiple requests at a time to make forward progress.
122-
*
123-
* Return: the request object on success; NULL on failure (but see above)
124-
*/
125-
struct ahash_request *fsverity_alloc_hash_request(struct fsverity_hash_alg *alg,
126-
gfp_t gfp_flags)
127-
{
128-
struct ahash_request *req = mempool_alloc(&alg->req_pool, gfp_flags);
129-
130-
if (req)
131-
ahash_request_set_tfm(req, alg->tfm);
132-
return req;
133-
}
134-
135-
/**
136-
* fsverity_free_hash_request() - free a hash request object
137-
* @alg: the hash algorithm
138-
* @req: the hash request object to free
139-
*/
140-
void fsverity_free_hash_request(struct fsverity_hash_alg *alg,
141-
struct ahash_request *req)
142-
{
143-
if (req) {
144-
ahash_request_zero(req);
145-
mempool_free(req, &alg->req_pool);
146-
}
147-
}
148-
149102
/**
150103
* fsverity_prepare_hash_state() - precompute the initial hash state
151104
* @alg: hash algorithm
@@ -159,23 +112,20 @@ const u8 *fsverity_prepare_hash_state(struct fsverity_hash_alg *alg,
159112
const u8 *salt, size_t salt_size)
160113
{
161114
u8 *hashstate = NULL;
162-
struct ahash_request *req = NULL;
115+
SHASH_DESC_ON_STACK(desc, alg->tfm);
163116
u8 *padded_salt = NULL;
164117
size_t padded_salt_size;
165-
struct scatterlist sg;
166-
DECLARE_CRYPTO_WAIT(wait);
167118
int err;
168119

120+
desc->tfm = alg->tfm;
121+
169122
if (salt_size == 0)
170123
return NULL;
171124

172-
hashstate = kmalloc(crypto_ahash_statesize(alg->tfm), GFP_KERNEL);
125+
hashstate = kmalloc(crypto_shash_statesize(alg->tfm), GFP_KERNEL);
173126
if (!hashstate)
174127
return ERR_PTR(-ENOMEM);
175128

176-
/* This allocation never fails, since it's mempool-backed. */
177-
req = fsverity_alloc_hash_request(alg, GFP_KERNEL);
178-
179129
/*
180130
* Zero-pad the salt to the next multiple of the input size of the hash
181131
* algorithm's compression function, e.g. 64 bytes for SHA-256 or 128
@@ -190,26 +140,18 @@ const u8 *fsverity_prepare_hash_state(struct fsverity_hash_alg *alg,
190140
goto err_free;
191141
}
192142
memcpy(padded_salt, salt, salt_size);
193-
194-
sg_init_one(&sg, padded_salt, padded_salt_size);
195-
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP |
196-
CRYPTO_TFM_REQ_MAY_BACKLOG,
197-
crypto_req_done, &wait);
198-
ahash_request_set_crypt(req, &sg, NULL, padded_salt_size);
199-
200-
err = crypto_wait_req(crypto_ahash_init(req), &wait);
143+
err = crypto_shash_init(desc);
201144
if (err)
202145
goto err_free;
203146

204-
err = crypto_wait_req(crypto_ahash_update(req), &wait);
147+
err = crypto_shash_update(desc, padded_salt, padded_salt_size);
205148
if (err)
206149
goto err_free;
207150

208-
err = crypto_ahash_export(req, hashstate);
151+
err = crypto_shash_export(desc, hashstate);
209152
if (err)
210153
goto err_free;
211154
out:
212-
fsverity_free_hash_request(alg, req);
213155
kfree(padded_salt);
214156
return hashstate;
215157

@@ -223,9 +165,7 @@ const u8 *fsverity_prepare_hash_state(struct fsverity_hash_alg *alg,
223165
* fsverity_hash_block() - hash a single data or hash block
224166
* @params: the Merkle tree's parameters
225167
* @inode: inode for which the hashing is being done
226-
* @req: preallocated hash request
227-
* @page: the page containing the block to hash
228-
* @offset: the offset of the block within @page
168+
* @data: virtual address of a buffer containing the block to hash
229169
* @out: output digest, size 'params->digest_size' bytes
230170
*
231171
* Hash a single data or hash block. The hash is salted if a salt is specified
@@ -234,33 +174,24 @@ const u8 *fsverity_prepare_hash_state(struct fsverity_hash_alg *alg,
234174
* Return: 0 on success, -errno on failure
235175
*/
236176
int fsverity_hash_block(const struct merkle_tree_params *params,
237-
const struct inode *inode, struct ahash_request *req,
238-
struct page *page, unsigned int offset, u8 *out)
177+
const struct inode *inode, const void *data, u8 *out)
239178
{
240-
struct scatterlist sg;
241-
DECLARE_CRYPTO_WAIT(wait);
179+
SHASH_DESC_ON_STACK(desc, params->hash_alg->tfm);
242180
int err;
243181

244-
sg_init_table(&sg, 1);
245-
sg_set_page(&sg, page, params->block_size, offset);
246-
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP |
247-
CRYPTO_TFM_REQ_MAY_BACKLOG,
248-
crypto_req_done, &wait);
249-
ahash_request_set_crypt(req, &sg, out, params->block_size);
182+
desc->tfm = params->hash_alg->tfm;
250183

251184
if (params->hashstate) {
252-
err = crypto_ahash_import(req, params->hashstate);
185+
err = crypto_shash_import(desc, params->hashstate);
253186
if (err) {
254187
fsverity_err(inode,
255188
"Error %d importing hash state", err);
256189
return err;
257190
}
258-
err = crypto_ahash_finup(req);
191+
err = crypto_shash_finup(desc, data, params->block_size, out);
259192
} else {
260-
err = crypto_ahash_digest(req);
193+
err = crypto_shash_digest(desc, data, params->block_size, out);
261194
}
262-
263-
err = crypto_wait_req(err, &wait);
264195
if (err)
265196
fsverity_err(inode, "Error %d computing block hash", err);
266197
return err;
@@ -273,32 +204,12 @@ int fsverity_hash_block(const struct merkle_tree_params *params,
273204
* @size: size of data to hash, in bytes
274205
* @out: output digest, size 'alg->digest_size' bytes
275206
*
276-
* Hash some data which is located in physically contiguous memory (i.e. memory
277-
* allocated by kmalloc(), not by vmalloc()). No salt is used.
278-
*
279207
* Return: 0 on success, -errno on failure
280208
*/
281209
int fsverity_hash_buffer(struct fsverity_hash_alg *alg,
282210
const void *data, size_t size, u8 *out)
283211
{
284-
struct ahash_request *req;
285-
struct scatterlist sg;
286-
DECLARE_CRYPTO_WAIT(wait);
287-
int err;
288-
289-
/* This allocation never fails, since it's mempool-backed. */
290-
req = fsverity_alloc_hash_request(alg, GFP_KERNEL);
291-
292-
sg_init_one(&sg, data, size);
293-
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP |
294-
CRYPTO_TFM_REQ_MAY_BACKLOG,
295-
crypto_req_done, &wait);
296-
ahash_request_set_crypt(req, &sg, out, size);
297-
298-
err = crypto_wait_req(crypto_ahash_digest(req), &wait);
299-
300-
fsverity_free_hash_request(alg, req);
301-
return err;
212+
return crypto_shash_tfm_digest(alg->tfm, data, size, out);
302213
}
303214

304215
void __init fsverity_check_hash_algs(void)

0 commit comments

Comments
 (0)