On 17/11/25 07:09, Jan Kiszka wrote:
On 16.11.25 18:43, Philippe Mathieu-Daudé wrote:
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")
Feel free to add it. But not that it is not really a bug fix IMHO. It is
a code clarification, output is identical.
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.
This is not correct: 284 - 256 != 16
We hash 284 bytes, that is everything from data field to the end of
RPMBDataFrame.
Right. This is why I took so long time before starting to review your
previous series, I was looking for full-focused brain mode. Now than
I could focus again, this is now clear crystal. I'll just add a
"Hash everything from data field to the end of RPMBDataFrame" comment.
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
and queued!
