Re: [PATCH v5 0/2] dm-integrity: Implement asynch digest support

[Harald Freudenberger <freude@linux.ibm.com>]

On 2025-07-25 19:38, Eric Biggers wrote:
> On Fri, Jul 25, 2025 at 10:14:30AM +0200, Harald Freudenberger wrote:
>> On 2025-07-24 16:40, Mikulas Patocka wrote:
>> > On Tue, 22 Jul 2025, Harald Freudenberger wrote:
>> >
>> > > Support for ahashes in dm-integrity.
>> > >
>> > > Changelog:
>> > >
>> > > v1: First implementation. Tested with crc32, sha256, hmac-sha256 and
>> > >     the s390 specific implementations for hmac-sha256 and protected
>> > >     key phmac-sha256. Also ran with some instrumented code (in the
>> > > digest
>> > >     implementation) to verify that in fact now the code runs
>> > > asynchronous.
>> > > v2: Support shash and ahash. Based on Mikulas' idea about implementing
>> > >     ahash support similar to dm-verity this version now adds support
>> > >     for ahash but does not replace the shash support. For more details
>> > >     see the text of the patch header.
>> > > v3: The line to store the digestsize into the new internal variable
>> > >     did not make it into the patch set which was sent out. So now
>> > >     this important code piece is also there. Also rebuilded, sparse
>> > >     checked and tested to make sure the patches are ok.
>> > > v4: Thanks to Mikulas a total new implementation of the ahash support
>> > >     for the dm-integrity layer :-)
>> > > v5: Slight rework around the allocation and comparing of ahash and
>> > >     shash algorithm.
>> > >     V5 has been tested with the new introduced ahash phmac which is a
>> > >     protected key ("hardware key") version of a hmac for s390. As of
>> > > now
>> > >     phmac is only available in Herbert Xu's cryptodev-2.6 kernel tree
>> > >     but will be merged into mainline with the next merge window for
>> > >     the 6.17 development kernel.
>> > >
>> > > Mikulas Patocka (2):
>> > >   dm-integrity: use internal variable for digestsize
>> > >   dm-integrity: introduce ahash support for the internal hash
>> > >
>> > >  drivers/md/dm-integrity.c | 370
>> > > +++++++++++++++++++++++++++-----------
>> > >  1 file changed, 265 insertions(+), 105 deletions(-)
>> > >
>> > >
>> > > base-commit: 89be9a83ccf1f88522317ce02f854f30d6115c41
>> > > --
>> > > 2.43.0
>> > >
>> >
>> > Hi
>> >
>> > Eric Biggers recently removed ahash support from dm-verity - see this
>> > commit:
>> > https://kernel.googlesource.com/pub/scm/linux/kernel/git/device-mapper/linux-dm/+/f43309c6743257244f11f14d31c297ee6a410ded
>> >
>> > Should I revert Eric's patch? - would you need dm-verity with
>> > asynchronous
>> > hashes on zseries too?
>> >
>> > Is this patch series needed for performance (does it perform better than
>> > the in-cpu instructions)? Or is it need because of better security (the
>> > keys are hidden in the hardware)?
>> >
>> > Mikulas
>> 
>> I've seen this. Well as of now we don't need dm-verity. However, I'll 
>> check
>> our plans and let you know within the next days.
>> 
>> Thanks
> 
> Isn't your use case the "s390 specific protected key hash phmac"
> (https://lore.kernel.org/linux-crypto/20250617134440.48000-1-freude@linux.ibm.com/)?
> dm-verity uses an unkeyed hash, so that isn't applicable there.
> 

Yes, I've also found this out. For our purpose it is enough to have
dm-integrity with phmac support - dm-verity is fine with the 
(synchronous)
s390 sha implementations and so no need for asynchronous support.

> BTW, did you consider a lib/crypto/ API for phmac?  I suspect it could
> be much simpler than the asynchronous hash based version.

This is an option for the future. However, as of now I did not 
investigate
in how to exactly implement this as a lib. Maybe a work item for the 
next
months...

> 
> - Eric

Thanks for your feedback and hints
Harald Freudenberger