linux-kernel - Re: [PATCH v2] dm-verity: remove support for asynchronous hashes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1ee8338a-b19a-409a-bbe6-2068893b8abc@gmail.com>
Date: Thu, 10 Jul 2025 10:28:55 +0200
From: Milan Broz <gmazyland@...il.com>
To: Eric Biggers <ebiggers@...nel.org>, dm-devel@...ts.linux.dev,
 Alasdair Kergon <agk@...hat.com>, Mike Snitzer <snitzer@...nel.org>,
 Mikulas Patocka <mpatocka@...hat.com>
Cc: linux-kernel@...r.kernel.org, Gilad Ben-Yossef <gilad@...yossef.com>,
 Ard Biesheuvel <ardb@...nel.org>, "Jason A . Donenfeld" <Jason@...c4.com>,
 Sami Tolvanen <samitolvanen@...gle.com>
Subject: Re: [PATCH v2] dm-verity: remove support for asynchronous hashes

On 7/9/25 9:09 PM, Eric Biggers wrote:
> The support for asynchronous hashes in dm-verity has outlived its
> usefulness.  It adds significant code complexity and opportunity for
> bugs.  I don't know of anyone using it in practice.  (The original
> submitter of the code possibly was, but that was 8 years ago.)  Data I
> recently collected for en/decryption shows that using off-CPU crypto
> "accelerators" is consistently much slower than the CPU
> (https://lore.kernel.org/r/20250704070322.20692-1-ebiggers@kernel.org/),
> even on CPUs that lack dedicated cryptographic instructions.  Similar
> results are likely to be seen for hashing.
> 
> I already removed support for asynchronous hashes from fsverity two
> years ago, and no one ever complained.
> 
> Moreover, neither dm-verity, fsverity, nor fscrypt has ever actually
> used the asynchronous crypto algorithms in a truly asynchronous manner.
> The lack of interest in such optimizations provides further evidence
> that it's only the CPU-based crypto that actually matters.
> 
> Historically, it's also been common for people to forget to enable the
> optimized SHA-256 code, which could contribute to an off-CPU crypto
> engine being perceived as more useful than it really is.  In 6.16 I
> fixed that: the optimized SHA-256 code is now enabled by default.
> 
> Therefore, let's drop the support for asynchronous hashes in dm-verity.
> 
> Tested with verity-compat-test.

Hi,

I shortly tested it with veritysetup too, also on 32bit.
And I like this patch (I wish we can remove the async thing from the dmcrypt too...)

Just one nitpick - could you please increase minor version of dm-verity target,
so we have information in debug log that it is a patched version?

Thanks,
Milan

> 
> Acked-by: Ard Biesheuvel <ardb@...nel.org>
> Signed-off-by: Eric Biggers <ebiggers@...nel.org>
> ---
> 
> Changed in v2:
>    - Removed the now-unused 'may_sleep' parameter from verity_hash()
>    - Fixed a typo in commit message
>    - Added comment in verity_setup_salt_and_hashstate()
>    - Keep SHASH_DESC_ON_STACK in existing place in
>      verity_setup_salt_and_hashstate(), to reduce the diff slightly
>    - Added Ard's Acked-by
> 
>   drivers/md/dm-verity-fec.c    |   4 +-
>   drivers/md/dm-verity-target.c | 183 ++++++----------------------------
>   drivers/md/dm-verity.h        |  22 ++--
>   3 files changed, 37 insertions(+), 172 deletions(-)