Re: [Qemu-devel] [PATCH 2/7] sd: fix card size checking on R/W accesses

[andrzej zaborowski]

Hi Vincent,

On 26 July 2011 01:19, Vincent Palatin <address@hidden> wrote:
> We need to check that we are not crossing the boundaries of the card for
> the current access not for the next one which might not happen.
>
> Signed-off-by: Vincent Palatin <address@hidden>
> ---
>  hw/sd.c |   22 ++++++++++++----------
>  1 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/hw/sd.c b/hw/sd.c
> index f48d589..de477fe 100644
> --- a/hw/sd.c
> +++ b/hw/sd.c
> @@ -1451,11 +1451,6 @@ void sd_write_data(SDState *sd, uint8_t value)
>         sd->data[sd->data_offset ++] = value;
>         if (sd->data_offset >= sd->blk_len) {
>             /* TODO: Check CRC before committing */
> -            sd->state = sd_programming_state;
> -            BLK_WRITE_BLOCK(sd->data_start, sd->data_offset);
> -            sd->blk_written ++;
> -            sd->data_start += sd->blk_len;
> -            sd->data_offset = 0;
>             if (sd->data_start + sd->blk_len > sd->size) {
>                 sd->card_status |= ADDRESS_ERROR;
>                 break;
> @@ -1464,6 +1459,11 @@ void sd_write_data(SDState *sd, uint8_t value)
>                 sd->card_status |= WP_VIOLATION;
>                 break;
>             }
> +            sd->state = sd_programming_state;
> +            BLK_WRITE_BLOCK(sd->data_start, sd->data_offset);
> +            sd->blk_written ++;
> +            sd->data_start += sd->blk_len;
> +            sd->data_offset = 0;
>             sd->csd[14] |= 0x40;
>
>             /* Bzzzzzzztt .... Operation complete.  */
> @@ -1606,17 +1606,19 @@ uint8_t sd_read_data(SDState *sd)
>         break;
>
>     case 18:   /* CMD18:  READ_MULTIPLE_BLOCK */
> -        if (sd->data_offset == 0)
> +        if (sd->data_offset == 0) {
> +            if (sd->data_start + io_len > sd->size) {
> +                sd->card_status |= ADDRESS_ERROR;
> +                ret = 0;
> +                break;
> +            }
>             BLK_READ_BLOCK(sd->data_start, io_len);
> +        }
>         ret = sd->data[sd->data_offset ++];
>
>         if (sd->data_offset >= io_len) {
>             sd->data_start += io_len;
>             sd->data_offset = 0;
> -            if (sd->data_start + io_len > sd->size) {
> -                sd->card_status |= ADDRESS_ERROR;
> -                break;
> -            }
>         }
>         break;

I have to look more closely at the spec but it looks like
ADDRESS_ERROR might not even be the right error code for when we
read/write outside of the memory, because the spec says it's for
incorrect alignment.  Also the first chunk of this patch is
unnecessary now because I committed a patch from Peter Maydell that
did the same thing.

I pushed your patch 01 in the series, thanks.

Cheers