On Sat, Aug 03, 2024 at 04:24:26PM +0200, Nam Cao wrote: > On Fri, Aug 02, 2024 at 08:01:38PM -0700, syzbot wrote: > > usb 1-1: BOGUS urb xfer, pipe 3 != type 1 > > WARNING: CPU: 0 PID: 2583 at drivers/usb/core/urb.c:503 usb_submit_urb+0xe4b/0x1730 drivers/usb/core/urb.c:503 > ... > > Call Trace: > > <TASK> > > r8712_usb_write_mem+0x2e4/0x3f0 drivers/staging/rtl8712/usb_ops_linux.c:170 > > rtl8712_dl_fw+0x7ab/0xfe0 drivers/staging/rtl8712/hal_init.c:203 > > rtl8712_hal_init drivers/staging/rtl8712/hal_init.c:330 [inline] > > rtl871x_hal_init+0xb3/0x190 drivers/staging/rtl8712/hal_init.c:394 > > netdev_open+0xea/0x800 drivers/staging/rtl8712/os_intfs.c:397 > > #syz test: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git linux-6.6.y > > diff --git a/drivers/staging/rtl8712/hal_init.c b/drivers/staging/rtl8712/hal_init.c > index 1148075f0cd6..80d8c462fafa 100644 > --- a/drivers/staging/rtl8712/hal_init.c > +++ b/drivers/staging/rtl8712/hal_init.c > @@ -152,6 +152,10 @@ static u8 chk_fwhdr(struct fw_hdr *pfwhdr, u32 ulfilelength) > return _SUCCESS; > } > > +static const int pipetypes[4] = { > + PIPE_CONTROL, PIPE_ISOCHRONOUS, PIPE_BULK, PIPE_INTERRUPT > +}; > + > static u8 rtl8712_dl_fw(struct _adapter *adapter) > { > sint i; > @@ -167,6 +171,29 @@ static u8 rtl8712_dl_fw(struct _adapter *adapter) > u32 txdscp_sz = sizeof(struct tx_desc); > u8 ret = _FAIL; > > + struct intf_hdl *hdl = &adapter->pio_queue->intf; > + struct intf_priv *pintfpriv = hdl->pintfpriv; > + struct dvobj_priv *pdvobj = (struct dvobj_priv *)pintfpriv->intf_dev; > + struct usb_device *pusbd = pdvobj->pusbdev; > + > + for (int i = 0; i < 16; ++i) { > + struct usb_host_endpoint *ep = pusbd->ep_in[i]; > + if (!ep) > + continue; > + int xfertype = usb_endpoint_type(&ep->desc); > + int type = pipetypes[xfertype]; > + pr_err("ep_in[%d] type=%d\n", i, type); > + } > + > + for (int i = 0; i < 16; ++i) { > + struct usb_host_endpoint *ep = pusbd->ep_out[i]; > + if (!ep) > + continue; > + int xfertype = usb_endpoint_type(&ep->desc); > + int type = pipetypes[xfertype]; > + pr_err("ep_out[%d] type=%d\n", i, type); > + } > + > ulfilelength = rtl871x_open_fw(adapter, &mappedfw); > if (mappedfw && (ulfilelength > 0)) { > update_fwhdr(&fwhdr, mappedfw); > @@ -200,6 +227,7 @@ static u8 rtl8712_dl_fw(struct _adapter *adapter) > txdesc->txdw0 |= cpu_to_le32(dump_imem_sz & > 0x0000ffff); > memcpy(payload, ptr, dump_imem_sz); > + pr_err("%s:%d\n", __func__, __LINE__); > r8712_write_mem(adapter, RTL8712_DMA_VOQ, > dump_imem_sz + TXDESC_SIZE, > (u8 *)txdesc); > @@ -229,6 +257,7 @@ static u8 rtl8712_dl_fw(struct _adapter *adapter) > txdesc->txdw0 |= cpu_to_le32(dump_emem_sz & > 0x0000ffff); > memcpy(payload, ptr, dump_emem_sz); > + pr_err("%s:%d\n", __func__, __LINE__); > r8712_write_mem(adapter, RTL8712_DMA_VOQ, > dump_emem_sz + TXDESC_SIZE, > (u8 *)txdesc); > @@ -282,6 +311,7 @@ static u8 rtl8712_dl_fw(struct _adapter *adapter) > txdesc->txdw0 |= cpu_to_le32(fwhdr.fw_priv_sz & 0x0000ffff); > txdesc->txdw0 |= cpu_to_le32(BIT(28)); > memcpy(payload, &fwhdr.fwpriv, fwhdr.fw_priv_sz); > + pr_err("%s:%d\n", __func__, __LINE__); > r8712_write_mem(adapter, RTL8712_DMA_VOQ, > fwhdr.fw_priv_sz + TXDESC_SIZE, (u8 *)txdesc); You don't have to run all these tests to figure out what the problem is; I can tell you. The bug is connected to ffaddr2piphd() in usb_ops_linux.c. That routine creates a bunch of Bulk pipe values with various endpoint numbers, based on the addr argument passed by its caller. But the driver doesn't check to make sure that these endpoints actually exist in the device or that they are actually Bulk endpoints. That's why the problem occurs. In this syzbot test the endpoint in question is really Interrupt, not Bulk. That's why the warning message appears about the pipe's type not matching the endpoint's type. Alan Stern
