| Message ID | 1414569232-21357-5-git-send-email-hare@suse.de |
|---|---|
| State | New |
| Headers | show |
On 10/29/2014 08:53 AM, Hannes Reinecke wrote: > 6 byte CDBs do not have a dedicated area for LBAs, and even if > it certainly won't be at byte 0. > > Signed-off-by: Hannes Reinecke <hare@suse.de> > --- > hw/scsi/scsi-bus.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c > index 919a86c..64d0880 100644 > --- a/hw/scsi/scsi-bus.c > +++ b/hw/scsi/scsi-bus.c > @@ -1195,9 +1195,6 @@ static uint64_t scsi_cmd_lba(SCSICommand *cmd) > uint64_t lba; > > switch (buf[0] >> 5) { > - case 0: > - lba = ldl_be_p(&buf[0]) & 0x1fffff; These are bits 0-20 of the first big endian u32, which means the low five bits of byte 1, together with byte 2 and byte 3. The patch as is breaks (obsolete) commands such as READ(6) and WRITE(6). Why did you need it? Paolo > - break; > case 1: > case 2: > case 5: >
On 10/29/2014 10:16 AM, Paolo Bonzini wrote: > > > On 10/29/2014 08:53 AM, Hannes Reinecke wrote: >> 6 byte CDBs do not have a dedicated area for LBAs, and even if >> it certainly won't be at byte 0. >> >> Signed-off-by: Hannes Reinecke <hare@suse.de> >> --- >> hw/scsi/scsi-bus.c | 3 --- >> 1 file changed, 3 deletions(-) >> >> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c >> index 919a86c..64d0880 100644 >> --- a/hw/scsi/scsi-bus.c >> +++ b/hw/scsi/scsi-bus.c >> @@ -1195,9 +1195,6 @@ static uint64_t scsi_cmd_lba(SCSICommand *cmd) >> uint64_t lba; >> >> switch (buf[0] >> 5) { >> - case 0: >> - lba = ldl_be_p(&buf[0]) & 0x1fffff; > > These are bits 0-20 of the first big endian u32, which means the low > five bits of byte 1, together with byte 2 and byte 3. > > The patch as is breaks (obsolete) commands such as READ(6) and WRITE(6). > Why did you need it? > Because without this patch we end up with having a (basically random) value in cmd.lba, and we're ending up here: if (cmd.lba != -1) { trace_scsi_req_parsed_lba(d->id, d->lun, tag, buf[0], cmd.lba); } and causing a buffer overflow when printing out the cdb. Cheers, Hannes
On 10/29/2014 10:52 AM, Hannes Reinecke wrote: >> > Because without this patch we end up with having a (basically random) > value in cmd.lba, and we're ending up here: > > if (cmd.lba != -1) { > trace_scsi_req_parsed_lba(d->id, d->lun, tag, buf[0], cmd.lba); } Yeah, this is ugly but not fatal. > and causing a buffer overflow when printing out the cdb. Where exactly? This is the part I don't understand. Paolo
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index 919a86c..64d0880 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -1195,9 +1195,6 @@ static uint64_t scsi_cmd_lba(SCSICommand *cmd) uint64_t lba; switch (buf[0] >> 5) { - case 0: - lba = ldl_be_p(&buf[0]) & 0x1fffff; - break; case 1: case 2: case 5:
6 byte CDBs do not have a dedicated area for LBAs, and even if it certainly won't be at byte 0. Signed-off-by: Hannes Reinecke <hare@suse.de> --- hw/scsi/scsi-bus.c | 3 --- 1 file changed, 3 deletions(-)