[v2 PATCH 00/13] Architecture-optimized SHA-256 library API

Herbert Xu posted 13 patches 7 months, 3 weeks ago
There is a newer version of this series
arch/arm/configs/exynos_defconfig             |   1 -
arch/arm/configs/milbeaut_m10v_defconfig      |   1 -
arch/arm/configs/multi_v7_defconfig           |   1 -
arch/arm/configs/omap2plus_defconfig          |   1 -
arch/arm/configs/pxa_defconfig                |   1 -
arch/arm/crypto/Kconfig                       |  21 -
arch/arm/crypto/Makefile                      |   8 +-
arch/arm/crypto/sha2-ce-glue.c                |  87 ----
arch/arm/crypto/sha256_glue.c                 | 107 -----
arch/arm/crypto/sha256_glue.h                 |   9 -
arch/arm/crypto/sha256_neon_glue.c            |  75 ---
arch/arm/lib/crypto/.gitignore                |   1 +
arch/arm/lib/crypto/Kconfig                   |   7 +
arch/arm/lib/crypto/Makefile                  |   8 +-
arch/arm/{ => lib}/crypto/sha256-armv4.pl     |  20 +-
.../sha2-ce-core.S => lib/crypto/sha256-ce.S} |  10 +-
arch/arm/lib/crypto/sha256.c                  |  64 +++
arch/arm64/configs/defconfig                  |   1 -
arch/arm64/crypto/Kconfig                     |  19 -
arch/arm64/crypto/Makefile                    |  13 +-
arch/arm64/crypto/sha2-ce-glue.c              | 138 ------
arch/arm64/crypto/sha256-glue.c               | 171 -------
arch/arm64/crypto/sha512-glue.c               |   6 +-
arch/arm64/lib/crypto/.gitignore              |   1 +
arch/arm64/lib/crypto/Kconfig                 |   6 +
arch/arm64/lib/crypto/Makefile                |   9 +-
.../crypto/sha2-armv8.pl}                     |   2 +-
.../sha2-ce-core.S => lib/crypto/sha256-ce.S} |  36 +-
arch/arm64/lib/crypto/sha256.c                |  75 +++
arch/mips/cavium-octeon/Kconfig               |   6 +
.../mips/cavium-octeon/crypto/octeon-sha256.c | 139 ++----
arch/mips/configs/cavium_octeon_defconfig     |   1 -
arch/mips/crypto/Kconfig                      |  10 -
arch/powerpc/crypto/Kconfig                   |  11 -
arch/powerpc/crypto/Makefile                  |   2 -
arch/powerpc/crypto/sha256-spe-glue.c         | 128 ------
arch/powerpc/lib/crypto/Kconfig               |   6 +
arch/powerpc/lib/crypto/Makefile              |   3 +
.../powerpc/{ => lib}/crypto/sha256-spe-asm.S |   0
arch/powerpc/lib/crypto/sha256.c              |  70 +++
arch/riscv/crypto/Kconfig                     |  11 -
arch/riscv/crypto/Makefile                    |   3 -
arch/riscv/crypto/sha256-riscv64-glue.c       | 125 -----
arch/riscv/lib/crypto/Kconfig                 |   8 +
arch/riscv/lib/crypto/Makefile                |   3 +
.../sha256-riscv64-zvknha_or_zvknhb-zvkb.S    |   4 +-
arch/riscv/lib/crypto/sha256.c                |  67 +++
arch/s390/configs/debug_defconfig             |   1 -
arch/s390/configs/defconfig                   |   1 -
arch/s390/crypto/Kconfig                      |  10 -
arch/s390/crypto/Makefile                     |   1 -
arch/s390/crypto/sha256_s390.c                | 144 ------
arch/s390/lib/crypto/Kconfig                  |   6 +
arch/s390/lib/crypto/Makefile                 |   2 +
arch/s390/lib/crypto/sha256.c                 |  47 ++
arch/sparc/crypto/Kconfig                     |  10 -
arch/sparc/crypto/Makefile                    |   2 -
arch/sparc/crypto/aes_asm.S                   |   3 +-
arch/sparc/crypto/aes_glue.c                  |   3 +-
arch/sparc/crypto/camellia_asm.S              |   3 +-
arch/sparc/crypto/camellia_glue.c             |   3 +-
arch/sparc/crypto/des_asm.S                   |   3 +-
arch/sparc/crypto/des_glue.c                  |   3 +-
arch/sparc/crypto/md5_asm.S                   |   3 +-
arch/sparc/crypto/md5_glue.c                  |   3 +-
arch/sparc/crypto/sha1_asm.S                  |   3 +-
arch/sparc/crypto/sha1_glue.c                 |   3 +-
arch/sparc/crypto/sha256_glue.c               | 129 ------
arch/sparc/crypto/sha512_asm.S                |   3 +-
arch/sparc/crypto/sha512_glue.c               |   3 +-
arch/sparc/{crypto => include/asm}/opcodes.h  |   6 +-
arch/sparc/lib/Makefile                       |   1 +
arch/sparc/lib/crc32c_asm.S                   |   3 +-
arch/sparc/lib/crypto/Kconfig                 |   8 +
arch/sparc/lib/crypto/Makefile                |   4 +
arch/sparc/lib/crypto/sha256.c                |  64 +++
arch/sparc/{ => lib}/crypto/sha256_asm.S      |   5 +-
arch/x86/crypto/Kconfig                       |  14 -
arch/x86/crypto/Makefile                      |   3 -
arch/x86/crypto/sha256_ssse3_glue.c           | 432 ------------------
arch/x86/lib/crypto/Kconfig                   |   8 +
arch/x86/lib/crypto/Makefile                  |   3 +
arch/x86/{ => lib}/crypto/sha256-avx-asm.S    |  12 +-
arch/x86/{ => lib}/crypto/sha256-avx2-asm.S   |  12 +-
.../crypto/sha256-ni-asm.S}                   |  36 +-
arch/x86/{ => lib}/crypto/sha256-ssse3-asm.S  |  14 +-
arch/x86/lib/crypto/sha256.c                  |  80 ++++
arch/x86/purgatory/Makefile                   |   3 -
arch/x86/purgatory/sha256.c                   |  15 +
crypto/Kconfig                                |   1 +
crypto/Makefile                               |   3 +-
crypto/sha256.c                               | 198 ++++++++
crypto/sha256_generic.c                       | 102 -----
include/crypto/internal/sha2.h                |  75 +++
include/crypto/sha2.h                         |  23 +-
include/crypto/sha256_base.h                  | 148 ------
lib/crypto/Kconfig                            |  30 ++
lib/crypto/Makefile                           |   1 +
lib/crypto/sha256-generic.c                   | 139 ++++++
lib/crypto/sha256.c                           | 150 ++----
100 files changed, 1165 insertions(+), 2313 deletions(-)
delete mode 100644 arch/arm/crypto/sha2-ce-glue.c
delete mode 100644 arch/arm/crypto/sha256_glue.c
delete mode 100644 arch/arm/crypto/sha256_glue.h
delete mode 100644 arch/arm/crypto/sha256_neon_glue.c
rename arch/arm/{ => lib}/crypto/sha256-armv4.pl (97%)
rename arch/arm/{crypto/sha2-ce-core.S => lib/crypto/sha256-ce.S} (91%)
create mode 100644 arch/arm/lib/crypto/sha256.c
delete mode 100644 arch/arm64/crypto/sha2-ce-glue.c
delete mode 100644 arch/arm64/crypto/sha256-glue.c
rename arch/arm64/{crypto/sha512-armv8.pl => lib/crypto/sha2-armv8.pl} (99%)
rename arch/arm64/{crypto/sha2-ce-core.S => lib/crypto/sha256-ce.S} (80%)
create mode 100644 arch/arm64/lib/crypto/sha256.c
delete mode 100644 arch/powerpc/crypto/sha256-spe-glue.c
rename arch/powerpc/{ => lib}/crypto/sha256-spe-asm.S (100%)
create mode 100644 arch/powerpc/lib/crypto/sha256.c
delete mode 100644 arch/riscv/crypto/sha256-riscv64-glue.c
rename arch/riscv/{ => lib}/crypto/sha256-riscv64-zvknha_or_zvknhb-zvkb.S (98%)
create mode 100644 arch/riscv/lib/crypto/sha256.c
delete mode 100644 arch/s390/crypto/sha256_s390.c
create mode 100644 arch/s390/lib/crypto/sha256.c
delete mode 100644 arch/sparc/crypto/sha256_glue.c
rename arch/sparc/{crypto => include/asm}/opcodes.h (96%)
create mode 100644 arch/sparc/lib/crypto/Kconfig
create mode 100644 arch/sparc/lib/crypto/Makefile
create mode 100644 arch/sparc/lib/crypto/sha256.c
rename arch/sparc/{ => lib}/crypto/sha256_asm.S (95%)
delete mode 100644 arch/x86/crypto/sha256_ssse3_glue.c
rename arch/x86/{ => lib}/crypto/sha256-avx-asm.S (98%)
rename arch/x86/{ => lib}/crypto/sha256-avx2-asm.S (98%)
rename arch/x86/{crypto/sha256_ni_asm.S => lib/crypto/sha256-ni-asm.S} (85%)
rename arch/x86/{ => lib}/crypto/sha256-ssse3-asm.S (98%)
create mode 100644 arch/x86/lib/crypto/sha256.c
create mode 100644 arch/x86/purgatory/sha256.c
create mode 100644 crypto/sha256.c
delete mode 100644 crypto/sha256_generic.c
create mode 100644 include/crypto/internal/sha2.h
delete mode 100644 include/crypto/sha256_base.h
create mode 100644 lib/crypto/sha256-generic.c
[v2 PATCH 00/13] Architecture-optimized SHA-256 library API
Posted by Herbert Xu 7 months, 3 weeks ago
Changes in v2:
- Rebase on top of lib partial block helper series.
- Restore the block-only shash implementation of sha256.
- Move the SIMD hardirq test out of the block functions so that
  it is only done for the lib/crypto interface.
- Split the lib/crypto sha256 module to break cycle in allmod build.

This is based on

	https://patchwork.kernel.org/project/linux-crypto/list/?series=957415

Original description:

Following the example of several other algorithms (e.g. CRC32, ChaCha,
Poly1305, BLAKE2s), this series refactors the kernel's existing
architecture-optimized SHA-256 code to be available via the library API,
instead of just via the crypto_shash API as it was before.  It also
reimplements the SHA-256 crypto_shash API on top of the library API.

This makes it possible to use the SHA-256 library in
performance-critical cases.  The new design is also much simpler, with a
negative diffstat of over 1200 lines.  Finally, this also fixes the
longstanding issue where the arch-optimized SHA-256 was disabled by
default, so people often forgot to enable it.

For now the SHA-256 library is well-covered by the crypto_shash
self-tests, but I plan to add a test for the library directly later.
I've fully tested this series on arm, arm64, riscv, and x86.  On mips,
powerpc, s390, and sparc I've only been able to partially test it, since
QEMU does not support the SHA-256 instructions on those platforms.  If
anyone with access to a mips, powerpc, s390, or sparc system that has
SHA-256 instructions can verify that the crypto self-tests still pass,
that would be appreciated.  But I don't expect any issues, especially
since the new code is more straightforward than the old code.

Eric Biggers (13):
  crypto: sha256 - support arch-optimized lib and expose through shash
  crypto: arm/sha256 - implement library instead of shash
  crypto: arm64/sha256 - remove obsolete chunking logic
  crypto: arm64/sha256 - implement library instead of shash
  crypto: mips/sha256 - implement library instead of shash
  crypto: powerpc/sha256 - implement library instead of shash
  crypto: riscv/sha256 - implement library instead of shash
  crypto: s390/sha256 - implement library instead of shash
  crypto: sparc - move opcodes.h into asm directory
  crypto: sparc/sha256 - implement library instead of shash
  crypto: x86/sha256 - implement library instead of shash
  crypto: sha256 - remove sha256_base.h
  crypto: lib/sha256 - improve function prototypes

 arch/arm/configs/exynos_defconfig             |   1 -
 arch/arm/configs/milbeaut_m10v_defconfig      |   1 -
 arch/arm/configs/multi_v7_defconfig           |   1 -
 arch/arm/configs/omap2plus_defconfig          |   1 -
 arch/arm/configs/pxa_defconfig                |   1 -
 arch/arm/crypto/Kconfig                       |  21 -
 arch/arm/crypto/Makefile                      |   8 +-
 arch/arm/crypto/sha2-ce-glue.c                |  87 ----
 arch/arm/crypto/sha256_glue.c                 | 107 -----
 arch/arm/crypto/sha256_glue.h                 |   9 -
 arch/arm/crypto/sha256_neon_glue.c            |  75 ---
 arch/arm/lib/crypto/.gitignore                |   1 +
 arch/arm/lib/crypto/Kconfig                   |   7 +
 arch/arm/lib/crypto/Makefile                  |   8 +-
 arch/arm/{ => lib}/crypto/sha256-armv4.pl     |  20 +-
 .../sha2-ce-core.S => lib/crypto/sha256-ce.S} |  10 +-
 arch/arm/lib/crypto/sha256.c                  |  64 +++
 arch/arm64/configs/defconfig                  |   1 -
 arch/arm64/crypto/Kconfig                     |  19 -
 arch/arm64/crypto/Makefile                    |  13 +-
 arch/arm64/crypto/sha2-ce-glue.c              | 138 ------
 arch/arm64/crypto/sha256-glue.c               | 171 -------
 arch/arm64/crypto/sha512-glue.c               |   6 +-
 arch/arm64/lib/crypto/.gitignore              |   1 +
 arch/arm64/lib/crypto/Kconfig                 |   6 +
 arch/arm64/lib/crypto/Makefile                |   9 +-
 .../crypto/sha2-armv8.pl}                     |   2 +-
 .../sha2-ce-core.S => lib/crypto/sha256-ce.S} |  36 +-
 arch/arm64/lib/crypto/sha256.c                |  75 +++
 arch/mips/cavium-octeon/Kconfig               |   6 +
 .../mips/cavium-octeon/crypto/octeon-sha256.c | 139 ++----
 arch/mips/configs/cavium_octeon_defconfig     |   1 -
 arch/mips/crypto/Kconfig                      |  10 -
 arch/powerpc/crypto/Kconfig                   |  11 -
 arch/powerpc/crypto/Makefile                  |   2 -
 arch/powerpc/crypto/sha256-spe-glue.c         | 128 ------
 arch/powerpc/lib/crypto/Kconfig               |   6 +
 arch/powerpc/lib/crypto/Makefile              |   3 +
 .../powerpc/{ => lib}/crypto/sha256-spe-asm.S |   0
 arch/powerpc/lib/crypto/sha256.c              |  70 +++
 arch/riscv/crypto/Kconfig                     |  11 -
 arch/riscv/crypto/Makefile                    |   3 -
 arch/riscv/crypto/sha256-riscv64-glue.c       | 125 -----
 arch/riscv/lib/crypto/Kconfig                 |   8 +
 arch/riscv/lib/crypto/Makefile                |   3 +
 .../sha256-riscv64-zvknha_or_zvknhb-zvkb.S    |   4 +-
 arch/riscv/lib/crypto/sha256.c                |  67 +++
 arch/s390/configs/debug_defconfig             |   1 -
 arch/s390/configs/defconfig                   |   1 -
 arch/s390/crypto/Kconfig                      |  10 -
 arch/s390/crypto/Makefile                     |   1 -
 arch/s390/crypto/sha256_s390.c                | 144 ------
 arch/s390/lib/crypto/Kconfig                  |   6 +
 arch/s390/lib/crypto/Makefile                 |   2 +
 arch/s390/lib/crypto/sha256.c                 |  47 ++
 arch/sparc/crypto/Kconfig                     |  10 -
 arch/sparc/crypto/Makefile                    |   2 -
 arch/sparc/crypto/aes_asm.S                   |   3 +-
 arch/sparc/crypto/aes_glue.c                  |   3 +-
 arch/sparc/crypto/camellia_asm.S              |   3 +-
 arch/sparc/crypto/camellia_glue.c             |   3 +-
 arch/sparc/crypto/des_asm.S                   |   3 +-
 arch/sparc/crypto/des_glue.c                  |   3 +-
 arch/sparc/crypto/md5_asm.S                   |   3 +-
 arch/sparc/crypto/md5_glue.c                  |   3 +-
 arch/sparc/crypto/sha1_asm.S                  |   3 +-
 arch/sparc/crypto/sha1_glue.c                 |   3 +-
 arch/sparc/crypto/sha256_glue.c               | 129 ------
 arch/sparc/crypto/sha512_asm.S                |   3 +-
 arch/sparc/crypto/sha512_glue.c               |   3 +-
 arch/sparc/{crypto => include/asm}/opcodes.h  |   6 +-
 arch/sparc/lib/Makefile                       |   1 +
 arch/sparc/lib/crc32c_asm.S                   |   3 +-
 arch/sparc/lib/crypto/Kconfig                 |   8 +
 arch/sparc/lib/crypto/Makefile                |   4 +
 arch/sparc/lib/crypto/sha256.c                |  64 +++
 arch/sparc/{ => lib}/crypto/sha256_asm.S      |   5 +-
 arch/x86/crypto/Kconfig                       |  14 -
 arch/x86/crypto/Makefile                      |   3 -
 arch/x86/crypto/sha256_ssse3_glue.c           | 432 ------------------
 arch/x86/lib/crypto/Kconfig                   |   8 +
 arch/x86/lib/crypto/Makefile                  |   3 +
 arch/x86/{ => lib}/crypto/sha256-avx-asm.S    |  12 +-
 arch/x86/{ => lib}/crypto/sha256-avx2-asm.S   |  12 +-
 .../crypto/sha256-ni-asm.S}                   |  36 +-
 arch/x86/{ => lib}/crypto/sha256-ssse3-asm.S  |  14 +-
 arch/x86/lib/crypto/sha256.c                  |  80 ++++
 arch/x86/purgatory/Makefile                   |   3 -
 arch/x86/purgatory/sha256.c                   |  15 +
 crypto/Kconfig                                |   1 +
 crypto/Makefile                               |   3 +-
 crypto/sha256.c                               | 198 ++++++++
 crypto/sha256_generic.c                       | 102 -----
 include/crypto/internal/sha2.h                |  75 +++
 include/crypto/sha2.h                         |  23 +-
 include/crypto/sha256_base.h                  | 148 ------
 lib/crypto/Kconfig                            |  30 ++
 lib/crypto/Makefile                           |   1 +
 lib/crypto/sha256-generic.c                   | 139 ++++++
 lib/crypto/sha256.c                           | 150 ++----
 100 files changed, 1165 insertions(+), 2313 deletions(-)
 delete mode 100644 arch/arm/crypto/sha2-ce-glue.c
 delete mode 100644 arch/arm/crypto/sha256_glue.c
 delete mode 100644 arch/arm/crypto/sha256_glue.h
 delete mode 100644 arch/arm/crypto/sha256_neon_glue.c
 rename arch/arm/{ => lib}/crypto/sha256-armv4.pl (97%)
 rename arch/arm/{crypto/sha2-ce-core.S => lib/crypto/sha256-ce.S} (91%)
 create mode 100644 arch/arm/lib/crypto/sha256.c
 delete mode 100644 arch/arm64/crypto/sha2-ce-glue.c
 delete mode 100644 arch/arm64/crypto/sha256-glue.c
 rename arch/arm64/{crypto/sha512-armv8.pl => lib/crypto/sha2-armv8.pl} (99%)
 rename arch/arm64/{crypto/sha2-ce-core.S => lib/crypto/sha256-ce.S} (80%)
 create mode 100644 arch/arm64/lib/crypto/sha256.c
 delete mode 100644 arch/powerpc/crypto/sha256-spe-glue.c
 rename arch/powerpc/{ => lib}/crypto/sha256-spe-asm.S (100%)
 create mode 100644 arch/powerpc/lib/crypto/sha256.c
 delete mode 100644 arch/riscv/crypto/sha256-riscv64-glue.c
 rename arch/riscv/{ => lib}/crypto/sha256-riscv64-zvknha_or_zvknhb-zvkb.S (98%)
 create mode 100644 arch/riscv/lib/crypto/sha256.c
 delete mode 100644 arch/s390/crypto/sha256_s390.c
 create mode 100644 arch/s390/lib/crypto/sha256.c
 delete mode 100644 arch/sparc/crypto/sha256_glue.c
 rename arch/sparc/{crypto => include/asm}/opcodes.h (96%)
 create mode 100644 arch/sparc/lib/crypto/Kconfig
 create mode 100644 arch/sparc/lib/crypto/Makefile
 create mode 100644 arch/sparc/lib/crypto/sha256.c
 rename arch/sparc/{ => lib}/crypto/sha256_asm.S (95%)
 delete mode 100644 arch/x86/crypto/sha256_ssse3_glue.c
 rename arch/x86/{ => lib}/crypto/sha256-avx-asm.S (98%)
 rename arch/x86/{ => lib}/crypto/sha256-avx2-asm.S (98%)
 rename arch/x86/{crypto/sha256_ni_asm.S => lib/crypto/sha256-ni-asm.S} (85%)
 rename arch/x86/{ => lib}/crypto/sha256-ssse3-asm.S (98%)
 create mode 100644 arch/x86/lib/crypto/sha256.c
 create mode 100644 arch/x86/purgatory/sha256.c
 create mode 100644 crypto/sha256.c
 delete mode 100644 crypto/sha256_generic.c
 create mode 100644 include/crypto/internal/sha2.h
 delete mode 100644 include/crypto/sha256_base.h
 create mode 100644 lib/crypto/sha256-generic.c

-- 
2.39.5
Re: [v2 PATCH 00/13] Architecture-optimized SHA-256 library API
Posted by Eric Biggers 7 months, 3 weeks ago
On Sun, Apr 27, 2025 at 02:30:41PM +0800, Herbert Xu wrote:
> Changes in v2:
> - Rebase on top of lib partial block helper series.
> - Restore the block-only shash implementation of sha256.
> - Move the SIMD hardirq test out of the block functions so that
>   it is only done for the lib/crypto interface.
> - Split the lib/crypto sha256 module to break cycle in allmod build.
> 
> This is based on
> 
> 	https://patchwork.kernel.org/project/linux-crypto/list/?series=957415

Well, barely a day and you've already ruined my patch series.  Now instead of a
clean design where the crypto_shash API is built on top of the normal library
API (sha256_update() etc.), there's now a special low-level API
"sha256_choose_blocks()" just for shash that it's built on top of instead, for
no good reason.  You're also still pushing your broken BLOCK_HASH_UPDATE_BLOCKS
macro that doesn't work with size_t, and putting my name on your broken code
that uses it.

And yes, sorry about the allmodconfig build error.  It just means that the
generic code needs to be split into its own module, like how curve25519 works.
I'll post a new version with that fixed and your gratuitous changes undone.

- Eric
Re: [v2 PATCH 00/13] Architecture-optimized SHA-256 library API
Posted by Herbert Xu 7 months, 3 weeks ago
On Sun, Apr 27, 2025 at 05:35:14AM -0700, Eric Biggers wrote:
>
> Well, barely a day and you've already ruined my patch series.  Now instead of a
> clean design where the crypto_shash API is built on top of the normal library
> API (sha256_update() etc.), there's now a special low-level API
> "sha256_choose_blocks()" just for shash that it's built on top of instead, for
> no good reason.  You're also still pushing your broken BLOCK_HASH_UPDATE_BLOCKS
> macro that doesn't work with size_t, and putting my name on your broken code
> that uses it.

Your design is unacceptable because you're forcing the partial block
handling on shash where it's not needed, just as you're forcing the
hardirq support on everything.

I'll take your point about size_t and update the BLOCK_HASH helper.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [v2 PATCH 00/13] Architecture-optimized SHA-256 library API
Posted by Eric Biggers 7 months, 3 weeks ago
On Sun, Apr 27, 2025 at 08:41:38PM +0800, Herbert Xu wrote:
> On Sun, Apr 27, 2025 at 05:35:14AM -0700, Eric Biggers wrote:
> >
> > Well, barely a day and you've already ruined my patch series.  Now instead of a
> > clean design where the crypto_shash API is built on top of the normal library
> > API (sha256_update() etc.), there's now a special low-level API
> > "sha256_choose_blocks()" just for shash that it's built on top of instead, for
> > no good reason.  You're also still pushing your broken BLOCK_HASH_UPDATE_BLOCKS
> > macro that doesn't work with size_t, and putting my name on your broken code
> > that uses it.
> 
> Your design is unacceptable because you're forcing the partial block
> handling on shash where it's not needed,

Excuse me?  It's the other way around.  In my version the partial block handling
is only in the library, not shash.  In your version you've forced it into the
shash layer, even though the library does it already.  I understand that you've
added support for partial block handling to crypto/shash.c and you want to feel
like your work is useful, but in this case it's not, since the libray has to
handle arbitrary-length inputs anyway.

> just as you're forcing the hardirq support on everything.

If you want crypto_shash to warn on hardirq usage you should just put a
WARN_ON(in_hardirq()) in crypto_shash_*(), which will actually achieve that.
Not add a shash-specific non-hardirq-safe low-level API to the library that can
silently corrupt random tasks' SIMD registers on production systems.

- Eric
Re: [v2 PATCH 00/13] Architecture-optimized SHA-256 library API
Posted by Eric Biggers 7 months, 3 weeks ago
On Sun, Apr 27, 2025 at 05:56:43AM -0700, Eric Biggers wrote:
> On Sun, Apr 27, 2025 at 08:41:38PM +0800, Herbert Xu wrote:
> > On Sun, Apr 27, 2025 at 05:35:14AM -0700, Eric Biggers wrote:
> > >
> > > Well, barely a day and you've already ruined my patch series.  Now instead of a
> > > clean design where the crypto_shash API is built on top of the normal library
> > > API (sha256_update() etc.), there's now a special low-level API
> > > "sha256_choose_blocks()" just for shash that it's built on top of instead, for
> > > no good reason.  You're also still pushing your broken BLOCK_HASH_UPDATE_BLOCKS
> > > macro that doesn't work with size_t, and putting my name on your broken code
> > > that uses it.
> > 
> > Your design is unacceptable because you're forcing the partial block
> > handling on shash where it's not needed,
> 
> Excuse me?  It's the other way around.  In my version the partial block handling
> is only in the library, not shash.  In your version you've forced it into the
> shash layer, even though the library does it already.  I understand that you've
> added support for partial block handling to crypto/shash.c and you want to feel
> like your work is useful, but in this case it's not, since the libray has to
> handle arbitrary-length inputs anyway.
> 
> > just as you're forcing the hardirq support on everything.
> 
> If you want crypto_shash to warn on hardirq usage you should just put a
> WARN_ON(in_hardirq()) in crypto_shash_*(), which will actually achieve that.
> Not add a shash-specific non-hardirq-safe low-level API to the library that can
> silently corrupt random tasks' SIMD registers on production systems.

By the way, as I mentioned in my cover letter:

    For now the SHA-256 library is well-covered by the crypto_shash
    self-tests, but I plan to add a test for the library directly later.

But due to your gratuitous changes where crypto_shash is no longer built on top
of the normal SHA-256 library API, that's no longer the case.

So while I do still plan to add a SHA-256 library test anyway, I don't see the
reason for not also making crypto_shash just do the right thing.

- Eric
Re: [v2 PATCH 00/13] Architecture-optimized SHA-256 library API
Posted by Herbert Xu 7 months, 3 weeks ago
On Sun, Apr 27, 2025 at 06:11:38AM -0700, Eric Biggers wrote:
>
> By the way, as I mentioned in my cover letter:
> 
>     For now the SHA-256 library is well-covered by the crypto_shash
>     self-tests, but I plan to add a test for the library directly later.
> 
> But due to your gratuitous changes where crypto_shash is no longer built on top
> of the normal SHA-256 library API, that's no longer the case.
> 
> So while I do still plan to add a SHA-256 library test anyway, I don't see the
> reason for not also making crypto_shash just do the right thing.

OK at least this objection makes sense.

I'll add an additional sha256 shash implementation that wraps
around the lib/sha256 interface so we don't lose that testing
coverage.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt