On Sat, Mar 23, 2024 at 10:55:51PM +0300, Sergey Shtylyov wrote:
> When isd200_scsi_to_ata() emulates the SCSI READ/WRITE (10) commands,
> the LBA is a 32-bit CDB field and the transfer length is a 16-bit CDB
> field, so using *unsigned long* (which is 32-bit type on the 32-bit
> arches and 64-bit type on the 64-bit arches) to declare the lba and
> blockCount variables doesn't make much sense. Also, when it emulates
> the READ CAPACITY command, the returned LBA is a 32-bit parameter data
> field and the ATA device CHS mode capacity fits into 32 bits as well,
> so using *unsigned long* to declare the capacity variable doesn't make
> much sense as well. Let's use the u16/u32 types for those variables...
>
> Found by Linux Verification Center (linuxtesting.org) with the SVACE
> static analysis tool.
>
> Signed-off-by: Sergey Shtylyov <s.shtylyov@xxxxxx>
Reviewed-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> ---
> This patch is against the 'usb-next' branch of Greg KH's usb.git repo...
>
> Changes in version 2:
> - fixed up the lba and blockCount variable declarations;
> - removed the typecasts from the blockCount variable calculation;
> - undid the reordering of the capacity variable declaration;
> - completely rewrote the patch description.
>
> drivers/usb/storage/isd200.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> Index: usb/drivers/usb/storage/isd200.c
> ===================================================================
> --- usb.orig/drivers/usb/storage/isd200.c
> +++ usb/drivers/usb/storage/isd200.c
> @@ -1232,8 +1232,8 @@ static int isd200_scsi_to_ata(struct scs
> int sendToTransport = 1;
> unsigned char sectnum, head;
> unsigned short cylinder;
> - unsigned long lba;
> - unsigned long blockCount;
> + u32 lba;
> + u16 blockCount;
> unsigned char senseData[8] = { 0, 0, 0, 0, 0, 0, 0, 0 };
>
> memset(ataCdb, 0, sizeof(union ata_cdb));
> @@ -1291,7 +1291,7 @@ static int isd200_scsi_to_ata(struct scs
>
> case READ_CAPACITY:
> {
> - unsigned long capacity;
> + u32 capacity;
> struct read_capacity_data readCapacityData;
>
> usb_stor_dbg(us, " ATA OUT - SCSIOP_READ_CAPACITY\n");
> @@ -1316,7 +1316,7 @@ static int isd200_scsi_to_ata(struct scs
> usb_stor_dbg(us, " ATA OUT - SCSIOP_READ\n");
>
> lba = be32_to_cpu(*(__be32 *)&srb->cmnd[2]);
> - blockCount = (unsigned long)srb->cmnd[7]<<8 | (unsigned long)srb->cmnd[8];
> + blockCount = srb->cmnd[7] << 8 | srb->cmnd[8];
>
> if (ata_id_has_lba(id)) {
> sectnum = (unsigned char)(lba);
> @@ -1348,7 +1348,7 @@ static int isd200_scsi_to_ata(struct scs
> usb_stor_dbg(us, " ATA OUT - SCSIOP_WRITE\n");
>
> lba = be32_to_cpu(*(__be32 *)&srb->cmnd[2]);
> - blockCount = (unsigned long)srb->cmnd[7]<<8 | (unsigned long)srb->cmnd[8];
> + blockCount = srb->cmnd[7] << 8 | srb->cmnd[8];
>
> if (ata_id_has_lba(id)) {
> sectnum = (unsigned char)(lba);
