Re: [PATCH] dm-verity: hash blocks with shash import+finup when possible

[Eric Biggers <ebiggers@kernel.org>]

On Thu, Nov 02, 2023 at 12:43:31PM +0800, Herbert Xu wrote:
> On Tue, Oct 31, 2023 at 10:48:56PM -0700, Eric Biggers wrote:
> >
> > Note that dm-verity also has to use an ugly and error-prone workaround to use
> > ahash at all, since its hash blocks can be cached in vmalloc memory; see
> > verity_hash_update().  shash handles vmalloc memory much more naturally, since
> > no translation from vmalloc address to page to linear address is needed.
> 
> So why not drop ahash and always use shash? Does anybody care about
> offload for dm-verity?

I'd love to do that; it's what I did in fsverity.  Someone did intentionally
convert dm-verity from shash to ahash in 2017, though; see commit d1ac3ff008fb.
So we'd be reverting that.  Maybe there are people who'd still care.  Maybe not.
I haven't yet gotten any complaints about switching fsverity to shash in v6.5
(and I only used ahash originally because of the precedent of dm-verity).

> Alternatively, we could incorporate this into ahash itself.  Then
> you could have an optimised code path that does not do SGs if the
> underlying algorithm is shash.
> 
> I really do not wish to see this ahash/shash paradigm proliferate.

Do you have in mind making struct ahash_request specify the data by either
scatterlist or by virtual address?  It might be possible.  It would be necessary
to wire up all possible combinations of (SG, virt) x (ahash_alg, shash_alg),
with the vmalloc_to_page() hack for the virt + ahash_alg case.

> OK.  But we do still have the module signature verification code
> path and I think that one still needs the can-sleep flag.

Well, struct shash_desc used to have that flag, but it never did anything.  The
few use cases like this might be more simply served by just having a helper
function crypto_shash_update_large() that passes the data in chunks to
crypto_shash_update().

- Eric