That is what I envisioned. It is just an off-by-one on the bounds check. Marcus Glocker <[email protected]> wrote:
> On Tue, Mar 23, 2021 at 08:29:06AM -0600, Theo de Raadt wrote: > > > Marcus Glocker <[email protected]> wrote: > > > > > Index: sys/dev/sdmmc/sdmmc_scsi.c > > > =================================================================== > > > RCS file: /cvs/src/sys/dev/sdmmc/sdmmc_scsi.c,v > > > retrieving revision 1.59 > > > diff -u -p -u -p -r1.59 sdmmc_scsi.c > > > --- sys/dev/sdmmc/sdmmc_scsi.c 15 Oct 2020 13:22:13 -0000 1.59 > > > +++ sys/dev/sdmmc/sdmmc_scsi.c 23 Mar 2021 07:32:52 -0000 > > > @@ -365,9 +365,8 @@ sdmmc_scsi_cmd(struct scsi_xfer *xs) > > > /* A read or write operation. */ > > > sdmmc_scsi_decode_rw(xs, &blockno, &blockcnt); > > > > > > - if (blockno >= tgt->card->csd.capacity || > > > - blockno + blockcnt > tgt->card->csd.capacity) { > > > - DPRINTF(("%s: out of bounds %u-%u >= %u\n", DEVNAME(sc), > > > + if (blockno + blockcnt >= tgt->card->csd.capacity) { > > > + DPRINTF(("%s: out of bounds %u+%u >= %u\n", DEVNAME(sc), > > > blockno, blockcnt, tgt->card->csd.capacity)); > > > xs->error = XS_DRIVER_STUFFUP; > > > scsi_done(xs); > > > > blockno and blockcnt are both 32-bit, so this feels dangerously close > > to integer wrap, and I believe the first condition cannot be deleted. > > OK, then lets keep the diff minimal invasive. I'm ignoring the DPRINTF > part for now. > > > Index: sys/dev/sdmmc/sdmmc_scsi.c > =================================================================== > RCS file: /cvs/src/sys/dev/sdmmc/sdmmc_scsi.c,v > retrieving revision 1.59 > diff -u -p -u -p -r1.59 sdmmc_scsi.c > --- sys/dev/sdmmc/sdmmc_scsi.c 15 Oct 2020 13:22:13 -0000 1.59 > +++ sys/dev/sdmmc/sdmmc_scsi.c 23 Mar 2021 14:45:53 -0000 > @@ -366,7 +366,7 @@ sdmmc_scsi_cmd(struct scsi_xfer *xs) > sdmmc_scsi_decode_rw(xs, &blockno, &blockcnt); > > if (blockno >= tgt->card->csd.capacity || > - blockno + blockcnt > tgt->card->csd.capacity) { > + blockno + blockcnt >= tgt->card->csd.capacity) { > DPRINTF(("%s: out of bounds %u-%u >= %u\n", DEVNAME(sc), > blockno, blockcnt, tgt->card->csd.capacity)); > xs->error = XS_DRIVER_STUFFUP; >
