On Tue, Oct 08, 2024 at 09:57:23AM +0200, Cédric Le Goater wrote:
> From: Alejandro Zeise <[email protected]>
>
> Make the Aspeed HACE module use the new qcrypto accumulative hashing functions
> when in scatter-gather accumulative mode. A hash context will maintain a
> "running-hash" as each scatter-gather chunk is received.
>
> Previously each scatter-gather "chunk" was cached
> so the hash could be computed once the final chunk was received.
> However, the cache was a shallow copy, so once the guest overwrote the
> memory provided to HACE the final hash would not be correct.
>
> Possibly related to: https://gitlab.com/qemu-project/qemu/-/issues/1121
> Buglink: https://github.com/openbmc/qemu/issues/36
>
> Signed-off-by: Alejandro Zeise <[email protected]>
> [ clg: - Checkpatch fixes ]
> Signed-off-by: Cédric Le Goater <[email protected]>
> ---
> include/hw/misc/aspeed_hace.h | 4 ++
> hw/misc/aspeed_hace.c | 96 +++++++++++++++++++----------------
> 2 files changed, 56 insertions(+), 44 deletions(-)
>
> static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
> bool acc_mode)
> {
> struct iovec iov[ASPEED_HACE_MAX_SG];
> + uint32_t total_msg_len;
> + uint32_t pad_offset;
> g_autofree uint8_t *digest_buf = NULL;
> size_t digest_len = 0;
> - int niov = 0;
> + bool sg_acc_mode_final_request = false;
> int i;
> void *haddr;
>
> + if (acc_mode && s->hash_ctx == NULL) {
Error local_err = NULL;
> + s->hash_ctx = qcrypto_hash_new(algo, NULL);
&local_err;
> + if (s->hash_ctx == NULL) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: qcrypto failed to create hash context\n",
> + __func__);
Add error_get_pretty() output to the message so we get useful
information reported, and then error_free,.
> + return;
> + }
> + }
> +
> if (sg_mode) {
> uint32_t len = 0;
>
> - if (niov) {
> - i = niov;
> - }
> + if (acc_mode) {
> + if (qcrypto_hash_updatev(s->hash_ctx, iov, i, NULL) < 0) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: qcrypto hash update failed\n", __func__);
> + return;
> + }
> +
> + if (sg_acc_mode_final_request) {
> + if (qcrypto_hash_finalize_bytes(s->hash_ctx, &digest_buf,
> + &digest_len, NULL)) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: qcrypto failed to finalize hash\n",
> + __func__);
> + }
>
> - if (qcrypto_hash_bytesv(algo, iov, i, &digest_buf, &digest_len, NULL) <
> 0) {
> + qcrypto_hash_free(s->hash_ctx);
> +
> + s->hash_ctx = NULL;
> + s->iov_count = 0;
> + s->total_req_len = 0;
> + }
> + } else if (qcrypto_hash_bytesv(algo, iov, i, &digest_buf,
> + &digest_len, NULL) < 0) {
> qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto failed\n", __func__);
> return;
> }
Same comment about passing an Error object to all these methods
and logging the useful error message.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
