Re: [PATCH-for-9.1 2/2] hw/sd/sdcard: Assert @data_offset is in range

[Peter Maydell]

On Mon, 8 Apr 2024 at 15:18, Philippe Mathieu-Daudé <phi...@linaro.org> wrote:
>
> Prevent out-of-bound access with assertions.
>
> Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> ---
>  hw/sd/sd.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 16d8d52a78..c081211582 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1875,6 +1875,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
>                              sd->current_cmd, value);
>      switch (sd->current_cmd) {
>      case 24:  /* CMD24:  WRITE_SINGLE_BLOCK */
> +        assert(sd->data_offset < sizeof(sd->data));
>          sd->data[sd->data_offset ++] = value;

Abstract out functions

static void append_sd_data_byte(SDState *sd, uint8_t value)
{
    assert(sd->data_offset < sizeof(sd->data));
    sd->data[sd->data_offset++] = value;
}

static void read_sd_data_byte(SDState *sd, uint8_t value)
{
    assert(sd->data_offset < sizeof(sd->sd_data));
    return sd->data[sd->data_offset++];
}

(etc for read_sd_status_byte() etc) ?

(sadly I don't think there's a verb that is the equivalent
of "prepend/append" but for removing elements.)


>      case 22:  /* ACMD22: SEND_NUM_WR_BLOCKS */
> +        assert(sd->data_offset < sizeof(sd->sd_status));
>          ret = sd->data[sd->data_offset ++];

Checking against the size of a different array from
the one we're reading from.

thanks
-- PMM