On Tue, Sep 19, 2017 at 1:23 AM, Michael Olbrich
<m.olbr...@pengutronix.de> wrote:
> On Mon, Sep 18, 2017 at 02:28:26PM -0700, Alistair Francis wrote:
>> On Sat, Sep 16, 2017 at 3:35 AM, Michael Olbrich
>> <m.olbr...@pengutronix.de> wrote:
>> > The current code checks if the next block exceeds the size of the card.
>> > This generates an error while reading the last block of the card.
>> > Do the out-of-bounds check when starting to read a new block to fix this.
>> >
>> > This issue became visible with increased error checking in Linux 4.13.
>> >
>> > Signed-off-by: Michael Olbrich <m.olbr...@pengutronix.de>
>>
>> Thanks for the patch!
>>
>> > ---
>> >
>> > Changes in v2:
>> > - fixed warning
>> >
>> > I'm not quite sure if 0x00 is the correct return value, but it's used
>> > elsewhere in the same function when an error occurs, so it seems
>> > reasonable.
>>
>> Returning 0 looks fine to me.
>>
>> >
>> > hw/sd/sd.c | 12 ++++++------
>> > 1 file changed, 6 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> > index ba47bff4db80..35347a5bbcde 100644
>> > --- a/hw/sd/sd.c
>> > +++ b/hw/sd/sd.c
>> > @@ -1797,8 +1797,13 @@ 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;
>> > + return 0x00;
>> > + }
>>
>> Why move it inside the if (sd->data_offset == 0) and not just below
>> the ret = sd->data[sd->data_offset ++] ?
>>
>> > BLK_READ_BLOCK(sd->data_start, io_len);
>
> Mostly because of the line above. This copies the full block from the
> backend storage to sd->data, so we need to make sure that the data is
> actually available to fill sd->data, not if it's ok to access a certain
> byte within sd->data.
Doesn't this mean that the check is only done for the first block
then? When data_offset is 0.
Thanks,
Alistair
>
> Michael
>
>> > + }
>> > ret = sd->data[sd->data_offset ++];
>> >
>> > if (sd->data_offset >= io_len) {
>> > @@ -1812,11 +1817,6 @@ uint8_t sd_read_data(SDState *sd)
>> > break;
>> > }
>> > }
>> > -
>> > - if (sd->data_start + io_len > sd->size) {
>> > - sd->card_status |= ADDRESS_ERROR;
>> > - break;
>> > - }
>> > }
>> > break;
>> >
>> > --
>> > 2.14.1
>> >
>> >
>>
>
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
