| Message ID | 20240523072242.787164-1-shichaorai@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Check whether divisor is non-zero before division | expand |
On Thu, May 23, 2024 at 03:22:42PM +0800, Shichao Lai wrote: > Since uzonesize may be zero, so a judgement for non-zero is nessesary. > > Reported-by: xingwei lee <xrivendell7@gmail.com> > Reported-by: yue sun <samsun1006219@gmail.com> > Signed-off-by: Shichao Lai <shichaorai@gmail.com> > --- > drivers/usb/storage/alauda.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c > index 115f05a6201a..db075a8c03cb 100644 > --- a/drivers/usb/storage/alauda.c > +++ b/drivers/usb/storage/alauda.c > @@ -947,6 +947,8 @@ static int alauda_read_data(struct us_data *us, unsigned long address, > sg = NULL; > > while (sectors > 0) { > + if (!uzonesize) > + return USB_STOR_TRANSPORT_ERROR; > unsigned int zone = lba / uzonesize; /* integer division */ > unsigned int lba_offset = lba - (zone * uzonesize); > unsigned int pages; > -- > 2.34.1 > > Looks good, thanks! I'll queue this up after 6.10-rc1 is out. greg k-h
On 23.05.24 09:22, Shichao Lai wrote: Hi, > Since uzonesize may be zero, so a judgement for non-zero is nessesary. > > Reported-by: xingwei lee <xrivendell7@gmail.com> > Reported-by: yue sun <samsun1006219@gmail.com> > Signed-off-by: Shichao Lai <shichaorai@gmail.com> > --- > drivers/usb/storage/alauda.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c > index 115f05a6201a..db075a8c03cb 100644 > --- a/drivers/usb/storage/alauda.c > +++ b/drivers/usb/storage/alauda.c > @@ -947,6 +947,8 @@ static int alauda_read_data(struct us_data *us, unsigned long address, > sg = NULL; > > while (sectors > 0) { > + if (!uzonesize) > + return USB_STOR_TRANSPORT_ERROR; May I point out that uzonesize does not change in this function? There is no need to retest within the loop. > unsigned int zone = lba / uzonesize; /* integer division */ > unsigned int lba_offset = lba - (zone * uzonesize); > unsigned int pages; Secondly, alauda_write_lba() has the same issue. You also need to check in alauda_write_data(). Regards Oliver
On Thu, May 23, 2024 at 4:18 PM Oliver Neukum <oneukum@suse.com> wrote: > > On 23.05.24 09:22, Shichao Lai wrote: > > Hi, > > > Since uzonesize may be zero, so a judgement for non-zero is nessesary. > > > > Reported-by: xingwei lee <xrivendell7@gmail.com> > > Reported-by: yue sun <samsun1006219@gmail.com> > > Signed-off-by: Shichao Lai <shichaorai@gmail.com> > > --- > > drivers/usb/storage/alauda.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c > > index 115f05a6201a..db075a8c03cb 100644 > > --- a/drivers/usb/storage/alauda.c > > +++ b/drivers/usb/storage/alauda.c > > @@ -947,6 +947,8 @@ static int alauda_read_data(struct us_data *us, unsigned long address, > > sg = NULL; > > > > while (sectors > 0) { > > + if (!uzonesize) > > + return USB_STOR_TRANSPORT_ERROR; > > May I point out that uzonesize does not change in this function? > There is no need to retest within the loop. > > > unsigned int zone = lba / uzonesize; /* integer division */ > > unsigned int lba_offset = lba - (zone * uzonesize); > > unsigned int pages; > > Secondly, alauda_write_lba() has the same issue. > You also need to check in alauda_write_data(). > > Regards > Oliver Thanks for the helpful tip! I reviewed the code. Your suggestions can actually avoid repeated checks. And there is also such a problem in alauda_write_lba(). I am a beginner at making patches. May I commit a patch again which fixes both issues you mentioned?
On 23.05.24 11:12, shichao lai wrote: > Thanks for the helpful tip! > I reviewed the code. Your suggestions can actually avoid repeated checks. > And there is also such a problem in alauda_write_lba(). > I am a beginner at making patches. May I commit a patch again which > fixes both issues you mentioned? Hi, yes, you usually are supposed to change the "[PATCH]" in the subject line to "[PATCHv2]" and add a line in the log describing the difference between the first and second version. HTH Oliver
diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c index 115f05a6201a..db075a8c03cb 100644 --- a/drivers/usb/storage/alauda.c +++ b/drivers/usb/storage/alauda.c @@ -947,6 +947,8 @@ static int alauda_read_data(struct us_data *us, unsigned long address, sg = NULL; while (sectors > 0) { + if (!uzonesize) + return USB_STOR_TRANSPORT_ERROR; unsigned int zone = lba / uzonesize; /* integer division */ unsigned int lba_offset = lba - (zone * uzonesize); unsigned int pages;
Since uzonesize may be zero, so a judgement for non-zero is nessesary. Reported-by: xingwei lee <xrivendell7@gmail.com> Reported-by: yue sun <samsun1006219@gmail.com> Signed-off-by: Shichao Lai <shichaorai@gmail.com> --- drivers/usb/storage/alauda.c | 2 ++ 1 file changed, 2 insertions(+)