| Message ID | 0c219fd9-6b11-9c20-3dec-7dc5beaff379@omp.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] usb: storage: sddr55: fix sloppy typing in sddr55_{read|write}_data() | expand |
On Sat, Jan 20, 2024 at 11:15:11PM +0300, Sergey Shtylyov wrote: > In sddr55_{read|write}_data(), the address variables are needlessly typed > as *unsigned long* -- which is 32-bit type on the 32-bit arches and 64-bit > type on the 64-bit arches; those variables' value should fit into just 3 > command bytes and consists of 10-bit block # (or at least the max block # > seems to be 1023) and 4-/5-bit page # within a block, so 32-bit *unsigned* > *int* type should be more than enough... > > Found by Linux Verification Center (linuxtesting.org) with the Svace static > analysis tool. > > Signed-off-by: Karina Yankevich <k.yankevich@omp.ru> > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> > > --- Reviewed-by: Alan Stern <stern@rowland.harvard.edu> > This patch is against the 'usb-next' branch of Greg KH's usb.git repo... > > Changes in version 3: > - rewrote the patch subject and description. > > Changes in version 2: > - compeltely redid the patch. > > drivers/usb/storage/sddr55.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > Index: usb/drivers/usb/storage/sddr55.c > =================================================================== > --- usb.orig/drivers/usb/storage/sddr55.c > +++ usb/drivers/usb/storage/sddr55.c > @@ -196,7 +196,7 @@ static int sddr55_read_data(struct us_da > unsigned char *buffer; > > unsigned int pba; > - unsigned long address; > + unsigned int address; > > unsigned short pages; > unsigned int len, offset; > @@ -316,7 +316,7 @@ static int sddr55_write_data(struct us_d > > unsigned int pba; > unsigned int new_pba; > - unsigned long address; > + unsigned int address; > > unsigned short pages; > int i; >
On 1/20/24 11:15 PM, Sergey Shtylyov wrote: Oops, I forgot: From: Karina Yankevich <k.yankevich@omp.ru> Ignore this version, I'll repost... > In sddr55_{read|write}_data(), the address variables are needlessly typed > as *unsigned long* -- which is 32-bit type on the 32-bit arches and 64-bit > type on the 64-bit arches; those variables' value should fit into just 3 > command bytes and consists of 10-bit block # (or at least the max block # > seems to be 1023) and 4-/5-bit page # within a block, so 32-bit *unsigned* > *int* type should be more than enough... > > Found by Linux Verification Center (linuxtesting.org) with the Svace static > analysis tool. > > Signed-off-by: Karina Yankevich <k.yankevich@omp.ru> > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> [...] MBR, Sergey
Index: usb/drivers/usb/storage/sddr55.c =================================================================== --- usb.orig/drivers/usb/storage/sddr55.c +++ usb/drivers/usb/storage/sddr55.c @@ -196,7 +196,7 @@ static int sddr55_read_data(struct us_da unsigned char *buffer; unsigned int pba; - unsigned long address; + unsigned int address; unsigned short pages; unsigned int len, offset; @@ -316,7 +316,7 @@ static int sddr55_write_data(struct us_d unsigned int pba; unsigned int new_pba; - unsigned long address; + unsigned int address; unsigned short pages; int i;
In sddr55_{read|write}_data(), the address variables are needlessly typed as *unsigned long* -- which is 32-bit type on the 32-bit arches and 64-bit type on the 64-bit arches; those variables' value should fit into just 3 command bytes and consists of 10-bit block # (or at least the max block # seems to be 1023) and 4-/5-bit page # within a block, so 32-bit *unsigned* *int* type should be more than enough... Found by Linux Verification Center (linuxtesting.org) with the Svace static analysis tool. Signed-off-by: Karina Yankevich <k.yankevich@omp.ru> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> --- This patch is against the 'usb-next' branch of Greg KH's usb.git repo... Changes in version 3: - rewrote the patch subject and description. Changes in version 2: - compeltely redid the patch. drivers/usb/storage/sddr55.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)