Re: [syzbot] [staging?] [usb?] WARNING in r8712_usb_write_mem/usb_submit_urb (2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux