Hi Jan,
On 14/11/25 22:27, Jan Kiszka wrote:
From: Jan Kiszka <[email protected]>
From the source frame, we initially need to copy out all fields after
data, thus starting from nonce on. Avoid expressing this indirectly by
pointing to the end of the data field - which also raised the attention
of Coverity (out-of-bound read /wrt data).
Resolves: CID 1642869
Fixes: 3acf956ea1a ("hw/sd/sdcard: Handle RPMB MAC field")
Reported-by: GuoHan Zhao <[email protected]>
Signed-off-by: Jan Kiszka <[email protected]>
---
Tested, not causing any regression. Please check again if Coverity is
happy as well. Thanks!
hw/sd/sd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 9c86c016cc..7fdb9195e0 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1161,7 +1161,8 @@ static bool rpmb_calc_hmac(SDState *sd, const
RPMBDataFrame *frame,
assert(RPMB_HASH_LEN <= sizeof(sd->data));
- memcpy((uint8_t *)buf + RPMB_DATA_LEN, &frame->data[RPMB_DATA_LEN],
+ memcpy((uint8_t *)buf + RPMB_DATA_LEN,
+ (uint8_t *)frame + offsetof(RPMBDataFrame, nonce),
RPMB_HASH_LEN - RPMB_DATA_LEN);
Having:
#define RPMB_HASH_LEN (RPMB_DATA_LEN + RPMB_NONCE_LEN)
then
RPMB_HASH_LEN - RPMB_DATA_LEN = RPMB_NONCE_LEN.
So this is equivalent to:
memcpy((uint8_t *)buf + RPMB_DATA_LEN,
(uint8_t *)frame + offsetof(RPMBDataFrame, nonce),
RPMB_NONCE_LEN);
How is this not equivalent to my previous "broken" suggestion?
memcpy((uint8_t *)buf + RPMB_DATA_LEN,
frame->nonce,
RPMB_NONCE_LEN);
(Yet again late here...)
offset = lduw_be_p(&frame->address) * RPMB_DATA_LEN +
sd_part_offset(sd);
do {
