Hi Connor,
+cc: Daniel
On 23/06/2021 0:15, Connor Kuehl wrote:
> On 6/21/21 2:05 PM, Dov Murik wrote:
>> +static void fill_sev_hash_table_entry(SevHashTableEntry *e, const uint8_t
>> *guid,
>> + const uint8_t *hash, size_t hash_len)
>> +{
>> + memcpy(e->guid, guid, sizeof(e->guid));
>> + e->len = sizeof(*e);
>> + memcpy(e->hash, hash, hash_len);
>
> Should this memcpy be constrained to MIN(sizeof(e->hash), hash_len)? Or
> perhaps an assert statement since I see below that this function's
> caller sets this to HASH_SIZE which is currently == sizeof(e->hash).
>
> Actually, the assert statement would be easier to debug if the input
> to this function is ever unexpected, especially since it avoids an
> outcome where the input is silently truncated; which is a pitfall that
> that the memcpy clamping would fall into.
I agree. I'll change it to:
assert(hash_len == sizeof(e->hash));
memcpy(e->hash, hash, sizeof(e->hash));
This way also the memcpy length is known at compile-time.
Related: I wondered if I could replace HASH_SIZE in:
/* hard code sha256 digest size */
#define HASH_SIZE 32
typedef struct QEMU_PACKED SevHashTableEntry {
QemuUUID guid;
uint16_t len;
uint8_t hash[HASH_SIZE];
} SevHashTableEntry;
with some SHA256-related constant from crypto/hash.h, but I only found
the qcrypto_hash_digest_len(QCRYPTO_HASH_ALG_SHA256) function which
doesn't work for setting sizes of arrays at compile-time.
Daniel: do you know what would be the proper way?
Thanks,
-Dov
