linux-kernel - Re: [PATCH v2] btrfs: switch to library APIs for checksums

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAEg-Je_4-vcyTg+aYA3nTsQ9ekBBZ1h89u9Qk_ZGQ_mGS_5Y4A@mail.gmail.com>
Date: Fri, 5 Dec 2025 08:05:59 -0500
From: Neal Gompa <neal@...pa.dev>
To: Eric Biggers <ebiggers@...nel.org>
Cc: linux-btrfs@...r.kernel.org, David Sterba <dsterba@...e.com>, Chris Mason <clm@...com>, 
	linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Ard Biesheuvel <ardb@...nel.org>
Subject: Re: [PATCH v2] btrfs: switch to library APIs for checksums

On Fri, Dec 5, 2025 at 2:21 AM Eric Biggers <ebiggers@...nel.org> wrote:
>
> Make btrfs use the library APIs instead of crypto_shash, for all
> checksum computations.  This has many benefits:
>
> - Allows future checksum types, e.g. XXH3 or CRC64, to be more easily
>   supported.  Only a library API will be needed, not crypto_shash too.
>
> - Eliminates the overhead of the generic crypto layer, including an
>   indirect call for every function call and other API overhead.  A
>   microbenchmark of btrfs_check_read_bio() with crc32c checksums shows a
>   speedup from 658 cycles to 608 cycles per 4096-byte block.
>
> - Decreases the stack usage of btrfs by reducing the size of checksum
>   contexts from 384 bytes to 240 bytes, and by eliminating the need for
>   some functions to declare a checksum context at all.
>
> - Increases reliability.  The library functions always succeed and
>   return void.  In contrast, crypto_shash can fail and return errors.
>   Also, the library functions are guaranteed to be available when btrfs
>   is loaded; there's no longer any need to use module softdeps to try to
>   work around the crypto modules sometimes not being loaded.
>
> - Fixes a bug where blake2b checksums didn't work on kernels booted with
>   fips=1.  Since btrfs checksums are for integrity only, it's fine for
>   them to use non-FIPS-approved algorithms.
>
> Note that with having to handle 4 algorithms instead of just 1-2, this
> commit does result in a slightly positive diffstat.  That being said,
> this wouldn't have been the case if btrfs had actually checked for
> errors from crypto_shash, which technically it should have been doing.
>
> Reviewed-by: Ard Biesheuvel <ardb@...nel.org>
> Signed-off-by: Eric Biggers <ebiggers@...nel.org>
> ---
>
> v2: rebased onto latest mainline, now that both the crypto library and
>     btrfs pull requests for 6.19 have been merged
>
>  fs/btrfs/Kconfig       |  8 ++--
>  fs/btrfs/compression.c |  1 -
>  fs/btrfs/disk-io.c     | 68 ++++++++----------------------
>  fs/btrfs/file-item.c   |  4 --
>  fs/btrfs/fs.c          | 96 ++++++++++++++++++++++++++++++++++++------
>  fs/btrfs/fs.h          | 23 +++++++---
>  fs/btrfs/inode.c       | 10 ++---
>  fs/btrfs/scrub.c       | 16 +++----
>  fs/btrfs/super.c       |  4 --
>  fs/btrfs/sysfs.c       |  6 +--
>  10 files changed, 133 insertions(+), 103 deletions(-)
>

This patch looks reasonable to me and seems to work as expected.

Reviewed-by: Neal Gompa <neal@...pa.dev>


-- 
真実はいつも一つ！/ Always, there's only one truth!