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 |