On Thu, 6 Nov 2025 at 07:29, <[email protected]> wrote:
>
> From: GuoHan Zhao <[email protected]>
>
> Coverity reported a potential out-of-bounds read in rpmb_calc_hmac():
>
> CID 1642869: Out-of-bounds read (OVERRUN)
> Overrunning array of 256 bytes at byte offset 256 by dereferencing
> pointer &frame->data[256].
>
> The issue arises from using &frame->data[RPMB_DATA_LEN] as the source
> pointer for memcpy(). Although computing a one-past-the-end pointer is
> legal, dereferencing it (as memcpy() does) is undefined behavior in C.
>
> Signed-off-by: GuoHan Zhao <[email protected]>
> ---
> 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 9c86c016cc9d..bc2e9863a534 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,
> + (const uint8_t *)frame + RPMB_DATA_LEN,
> RPMB_HASH_LEN - RPMB_DATA_LEN);
> offset = lduw_be_p(&frame->address) * RPMB_DATA_LEN +
> sd_part_offset(sd);
> do {
What is this code even trying to do ? We define a RPMBDataFrame
which is a packed struct, but now we're randomly memcpying
a lump of data out of the middle of it ??
The start of the struct is
uint8_t stuff_bytes[RPMB_STUFF_LEN]; // offset 0
uint8_t key_mac[RPMB_KEY_MAC_LEN]; // offset 196
uint8_t data[RPMB_DATA_LEN]; // offset 228
uint8_t nonce[RPMB_NONCE_LEN]; // offset 484
so frame + RPMB_DATA_LEN (256) starts 28 bytes into the data
array; and then we're copying 28 bytes of data?
The existing code (frame->data[RPMB_DATA_LEN]) doesn't make
sense either, as that's a weird way to write frame->nonce,
and the RPMB_NONCE_LEN doesn't have the same length as what
we're copying either.
Can somebody who understands this explain what this code
is intended to be doing ?
thanks
-- PMM
