Messages in this thread Patch in this message | | | From | "Fabio M. De Francesco" <> | Subject | Re: [syzbot] WARNING in usb_tx_block/usb_submit_urb | Date | Thu, 16 Feb 2023 07:54:08 +0100 |
| |
On mercoledì 15 febbraio 2023 12:05:15 CET Hillf Danton wrote: > On Tue, 14 Feb 2023 23:00:47 -0800 > > > syzbot found the following issue on: > > > > HEAD commit: f87b564686ee dt-bindings: usb: amlogic,meson-g12a-usb- ctrl.. > > git tree: > > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1670f2b3480000 > Kill urb in flight after submitting it. > > #syz test https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git > f87b564686ee > > --- x/drivers/net/wireless/marvell/libertas/if_usb.c > +++ y/drivers/net/wireless/marvell/libertas/if_usb.c > @@ -763,9 +763,7 @@ static int if_usb_issue_boot_command(str > memset(bootcmd->pad, 0, sizeof(bootcmd->pad)); > > /* Issue command */ > - usb_tx_block(cardp, cardp->ep_out_buf, sizeof(*bootcmd)); > - > - return 0; > + return usb_tx_block(cardp, cardp->ep_out_buf, sizeof(*bootcmd)); > } > > > @@ -853,10 +851,12 @@ restart: > }
> > cardp->bootcmdresp = 0; > + ret = if_usb_issue_boot_command(cardp, BOOT_CMD_FW_BY_USB); > + if (ret) > + goto done;
I think that you are changing the logic here (please read below)...
> do { > int j = 0; > i++; > - if_usb_issue_boot_command(cardp, BOOT_CMD_FW_BY_USB);
Don't we need to call if_usb_issue_boot_command() in a loop in order to retry the command?
> /* wait for command response */ > do { > j++; > @@ -864,6 +864,8 @@ restart: > } while (cardp->bootcmdresp == 0 && j < 10); > } while (cardp->bootcmdresp == 0 && i < 5); > > + usb_kill_urb(cardp->tx_urb); > +
I'm not an expert in the USB core, anyway calling usb_kill_urb() looks good to me, but I think we should call it after each call of if_usb_issue_boot_command() in the above outer loop.
> if (cardp->bootcmdresp == BOOT_CMD_RESP_NOT_SUPPORTED) { > /* Return to normal operation */ > ret = -EOPNOTSUPP; > --
Can the following work?
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git f87b564686ee
diff --git a/drivers/net/wireless/marvell/libertas/if_usb.c b/drivers/net/ wireless/marvell/libertas/if_usb.c index 20436a289d5c..626357d0c7b0 100644 --- a/drivers/net/wireless/marvell/libertas/if_usb.c +++ b/drivers/net/wireless/marvell/libertas/if_usb.c @@ -859,6 +859,7 @@ static void if_usb_prog_firmware(struct lbs_private *priv, int ret, j++; msleep_interruptible(100); } while (cardp->bootcmdresp == 0 && j < 10); + usb_kill_urb(cardp->tx_urb): } while (cardp->bootcmdresp == 0 && i < 5); if (cardp->bootcmdresp == BOOT_CMD_RESP_NOT_SUPPORTED) { -- Thanks,
Fabio
| |