Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Date:   Wed, 1 Nov 2023 22:40:08 -0700
From:   Eric Biggers <ebiggers@kernel.org>
To:     Herbert Xu <herbert@gondor.apana.org.au>
Cc:     agk@redhat.com, snitzer@kernel.org, dm-devel@lists.linux.dev,
        linux-crypto@vger.kernel.org, gilad@benyossef.com,
        samitolvanen@google.com
Subject: Re: [PATCH] dm-verity: hash blocks with shash import+finup when
 possible
Message-ID: <20231102054008.GG1498@sol.localdomain>
References: <20231030023351.6041-1-ebiggers@kernel.org>
 <ZUHZQ3tJH0WSV9dX@gondor.apana.org.au>
 <20231101054856.GA140941@sol.localdomain>
 <ZUMo87EMeYxiCZLX@gondor.apana.org.au>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZUMo87EMeYxiCZLX@gondor.apana.org.au>
Precedence: bulk

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