| Message ID | 20240523092608.874986-1-shichaorai@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCHv2] Check whether divisor is non-zero before division | expand |
On Fri, May 24, 2024 at 10:13:45AM +0800, shichao lai wrote: > On Fri, May 24, 2024 at 12:30 AM Alan Stern <stern@rowland.harvard.edu> wrote: > > > > Good work! So the problem is that the driver believes the status[0] & > > 0x08 test. > > > > The way to fix this is to add an "initialized" flag to the alauda_info > > structure. Then alauda_check_media() should call alauda_init_media() if > > the 0x08 bit is set in status[0] _or_ if info->initialized is 0. And of > > course, alauda_check_media() should then set info->initialized to 1 if > > the alauda_init_media() call succeeds. > > > > Would you like to write and test a patch that does this? > > > > Alan Stern > > I tried to do this. And the workflow can enter alauda_init_media(), > but there are still many conditions to satisfy in alauda_init_media(). > Unfortunately alauda_init_media() stop and return here before > initializing uzonesize: > > if (data[0] != 0x14) { > usb_stor_dbg(us, "Media not ready after ack\n"); > return USB_STOR_TRANSPORT_ERROR; > } That's an error return. > The data[0] is status[0] showed before, and it was 0x0036. > I am not familiar with the status code of alauda. > How can I deal with this condition? > Is it ok to pass this condition when info->initialized == false, even > if the data[0] != 0x14? If alauda_init_media() returns an error, leave info->initialized unchanged. alauda_check_media() will return an error also, so the bad division won't take place. Alan Stern
On Sat, May 25, 2024 at 10:24 AM Alan Stern <stern@rowland.harvard.edu> wrote: > If alauda_init_media() returns an error, leave info->initialized > unchanged. alauda_check_media() will return an error also, so the bad > division won't take place. > > Alan Stern Thanks! You also remind me that the return value from alauda_init_media() is never used! By this way, the workflow now seems to work correctly. It tries to initialize multiple times, and finally disconnects due to no response. Now if possible, I will post a [PATCH v4] for this bug soon. I want to know whether it is possible to add some tags like Suggested-by or Reviewed-by for Dear Alan Stern, gregkh and oneukum as thanks for your discussions. e.g. ==== kernel log [ 47.266129][ T4125] alauda_check_media: before init_media, status[0]: 0000000000000036 [ 47.266555][ T4125] alauda_check_media: enter init_media [ 47.467314][ T9] usb 1-1: USB disconnect, device number 2 root@syzkaller:~# [ 47.485304][ T4125] alauda_get_media_status: data=54, rc=4 [ 47.485640][ T4125] alauda_init_media: exit in 391 [ 47.486104][ T41] sd 2:0:0:0: [sdb] Read Capacity(10) failed: Result: hostbyte=DID_ERROR driverbyte=DRIVER_OK [ 47.486591][ T41] sd 2:0:0:0: [sdb] Sense not available. [ 47.486889][ T41] sd 2:0:0:0: [sdb] 0 512-byte logical blocks: (0 B/0 B) [ 47.487212][ T41] sd 2:0:0:0: [sdb] 0-byte physical blocks [ 47.487515][ T41] sd 2:0:0:0: [sdb] Write Protect is off [ 47.487813][ T41] sd 2:0:0:0: [sdb] Asking for cache data failed [ 47.488104][ T41] sd 2:0:0:0: [sdb] Assuming drive cache: write through [ 47.491396][ T41] sd 2:0:0:0: [sdb] Attached SCSI removable disk [ 48.105309][ T1198] not responding...
diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c index 115f05a6201a..a6e60ef5cb0d 100644 --- a/drivers/usb/storage/alauda.c +++ b/drivers/usb/storage/alauda.c @@ -818,6 +818,8 @@ static int alauda_write_lba(struct us_data *us, u16 lba, unsigned int blocksize = MEDIA_INFO(us).blocksize; unsigned int lba_offset = lba % uzonesize; unsigned int new_pba_offset; + if (!uzonesize) + return USB_STOR_TRANSPORT_ERROR; unsigned int zone = lba / uzonesize; alauda_ensure_map_for_zone(us, zone); @@ -923,6 +925,8 @@ static int alauda_read_data(struct us_data *us, unsigned long address, unsigned int uzonesize = MEDIA_INFO(us).uzonesize; struct scatterlist *sg; int result; + if (!uzonesize) + return USB_STOR_TRANSPORT_ERROR; /* * Since we only read in one block at a time, we have to create
Since uzonesize may be zero, so judgements for non-zero are nessesary in both place. Previous check is moved out of loop, and one more check is added in alauda_write_lba. 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 | 4 ++++ 1 file changed, 4 insertions(+)