Re: [PATCH v5 00/16] hw/misc/aspeed_hace: Fix SG Accumulative Hash Calculations

["Cédric Le Goater" <clg@redhat.com>]

On 10/8/24 09:57, Cédric Le Goater wrote:
> Hello,
> 
> This is a resping of Alejandro's series fixing SG Accumulative Hash
> Calculations. See [1] for more details.

arf.

[1] https://lore.kernel.org/all/20240807195122.2827364-1-alejandro.zeise@seagate.com/

C.

> 
> The goal of this patch series is to fix accumulative hashing support
> in the Aspeed HACE module. The issue that stemmed this patch was a
> failure to boot an OpenBMC image using the "ast2600-evb" machine. The
> U-boot 2019.04 loader failed to verify image hashes.
> 
> These incorrect image hashes given by the HACE to the U-boot guest are
> due to an oversight in the HACE module. Previously when operating in
> scatter-gather accumulative mode, the HACE would cache the address
> provided by the guest which contained the source data. However, there
> was no deep copy, so when HACE generated the digest upon the reception
> of the final accumulative chunk the digest was incorrect, as the
> addresses provided had their regions overwritten by that time.
> 
> This fix consists of two main steps:
> * Add an accumulative hashing function to the qcrypto library
> * Modify the HACE module to use the accumulative hashing functions
> 
> All the crypto library backends (nettle, gnutls, etc.) support
> accumulative hashing, so it was trivial to create wrappers for those
> functions.
> 
> Changes in v5 (clg):
> 
>   - Changed documentation "non-zero on error" -> "-1 on error"
>   - Dropped qcrypto_hash_supports() in qcrypto_glib_hash_new()
>   - Removed superfluous cast (GChecksum *) in qcrypto_glib_hash_free()
>   - Reworked qcrypto_glib_hash_finalize()
>   - Dropped qcrypto_hash_supports() in qcrypto_gcrypt_hash_new()
>   - Reworked qcrypto_gcrypt_hash_finalize()
>   - Handled gcry_md_open() errors in qcrypto_gcrypt_hash_new()
>   - Dropped qcrypto_hash_supports() in qcrypto_gnutls_hash_new()
>   - Reworked qcrypto_gnutls_hash_finalize()
>   - Handled gnutls_hash_init() errors in qcrypto_gnutls_hash_new()
>   - Replaced gnutls_hash_deinit() by gnutls_hash_output() in
>     qcrypto_gnutls_hash_finalize()
>   - Freed resources with gnutls_hash_deinit() in qcrypto_gnutls_hash_free()
>   - Dropped qcrypto_hash_supports() in qcrypto_nettle_hash_new()
>   - Split iov changes from original patch
>   - Handled qcrypto_afalg_hash_ctx_new() errors in qcrypto_afalg_hash_new()
>   - Freed alg_name in qcrypto_afalg_hash_new()
>   - Reworked qcrypto_afalg_recv_from_kernel()
>   - Split iov changes from original patch
>   - Reworked qcrypto_hash_bytesv() error handling
>   - Used hash->driver int qcrypto_hash_new(), qcrypto_hash_free() qcrypto_hash_updatev()
>   - Introduced qcrypto_hash_supports() check in  qcrypto_hash_new()
>   - Introduced g_autofree variables in qcrypto_hash_finalize_digest()
>     and qcrypto_hash_finalize_base64()
>   - Re-arrranged code in qcrypto_hash_digestv() and qcrypto_hash_digest()
>   - Improved test_hash_accumulate() with g_autofree variables
>   - Fixed spelling in commit log
>   - Checkpatch fixes
> 
> Changes in V4:
> * Restructured patches so unit tests pass for each independently.
> * Freeing hash context is now a void function.
> * Added autoptr cleanup function definition for qcrypto_hash_free.
> * Separated qcrypto_hash_update into qcrypto_hash_update and
>    qcrypto_hash_updatev.
> * Changed public hash functions to use afalg implementation correctly if support
>    is enabled.
> * Fixed accumulative hashing in afalg driver (pass MSG_MORE socket flag).
> 
> Changes in V3:
> * Reworked crypto hash API with comments from Daniel
>    * Creation/Deletion of contexts, updating, and finalizing
>    * Modified existing API functions to use the new 4 main core functions
>    * Added test for accumulative hashing
>    * Added afalg driver implementation
> * Fixed bug in HACE module where hash context fails to allocate,
>    causing the HACE internal state to be incorrect and segfault.
> 
> Changes in V2:
> * Fixed error checking bug in libgcrypt crypto backend of
>    accumulate_bytesv
> 
> Alejandro Zeise (16):
>    crypto: accumulative hashing API
>    crypto/hash-glib: Implement new hash API
>    crypto/hash-gcrypt: Implement new hash API
>    crypto/hash-gnutls: Implement new hash API
>    crypto/hash-nettle: Implement new hash API
>    util/iov: Introduce iov_send_recv_with_flags()
>    crypto/hash-afalg: Implement new hash API
>    crypto/hash: Implement and use new hash API
>    tests/unit/test-crypto-hash: accumulative hashing
>    crypto/hash-glib: Remove old hash API functions
>    crypto/hash-gcrypt: Remove old hash API functions
>    crypto/hash-gnutls: Remove old hash API functions
>    crypto/hash-nettle: Remove old hash API functions
>    crypto/hash-afalg: Remove old hash API functions
>    crypto/hashpriv: Remove old hash API function
>    hw/misc/aspeed_hace: Fix SG Accumulative hashing
> 
>   crypto/hashpriv.h             |  13 ++-
>   include/crypto/hash.h         | 119 ++++++++++++++++++++++++
>   include/hw/misc/aspeed_hace.h |   4 +
>   include/qemu/iov.h            |  27 ++++++
>   crypto/hash-afalg.c           | 167 ++++++++++++++++++++++++----------
>   crypto/hash-gcrypt.c          | 112 +++++++++++++----------
>   crypto/hash-glib.c            |  92 +++++++++++--------
>   crypto/hash-gnutls.c          |  97 +++++++++++++-------
>   crypto/hash-nettle.c          |  81 ++++++++++-------
>   crypto/hash.c                 | 161 ++++++++++++++++++++++++++------
>   hw/misc/aspeed_hace.c         |  96 ++++++++++---------
>   tests/unit/test-crypto-hash.c |  46 ++++++++++
>   util/iov.c                    |  25 +++--
>   13 files changed, 756 insertions(+), 284 deletions(-)
>