Re: [Qemu-devel] [PATCH] support >2TB SCSI disks

[Rik van Riel]

Paul Brook wrote:
> >      case 0:
> > -        lba = buf[3] | (buf[2] << 8) | ((buf[1] & 0x1f) << 16);
> > +        lba = (uint64_t) buf[3] | ((uint64_t) buf[2] << 8) |
> > +              (((uint64_t) buf[1] & 0x1f) << 16);
>
> This is not required, though I guess it's harmless.

I thought I'd keep them all consistent :)

> >      case 4:
> > ...
> >          len = buf[13] | (buf[12] << 8) | (buf[11] << 16) | (buf[10] << 24);
> > ...
> > +    case 0x88:
> >          r->sector_count = len * s->cluster_size;
>
> Implementing these commands introduces several overflows. There are several 
> places (including SCSIRequest->sector_count and the return value from 
> scsi_send_comand) that assume the transfer length fits in a signed (32-bit) 
> int.

True, a SCSI transfer of more than 2GB would cause an overflow.

> > +            /* Returned value is the address of the last sector.  */
> > +            if (nb_sectors) {
> > +                nb_sectors--;
>
> By my reading both this and the current Read Capacity(10)  are incorrect.
> They need to divide by s->cluster_size.

Good point.  Want me to send in a separate patch that does that?

--
All rights reversed.