Re: [PATCH] hw/sd/sdcard: Avoid confusing address calculation in rpmb_calc_hmac

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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 <jan.kiszka@siemens.com>
>>>
>>>   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 <zhaoguohan@kylinos.cn>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>
>>> 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é <philmd@linaro.org>

and queued!