Re: [PATCH 08/10] staging: rtl8723au: Sanitize USB read/write functions

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

 



Jes.Sorensen@xxxxxxxxxx writes:
> From: Jes Sorensen <Jes.Sorensen@xxxxxxxxxx>
>
> The original Realtek provided functions suffered badly from clutter to
> accommodate broken operating systems. Lets try this lean and clean
> version instead.
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@xxxxxxxxxx>
> ---
>  drivers/staging/rtl8723au/hal/usb_ops_linux.c     | 289 ++++++----------------
>  drivers/staging/rtl8723au/include/usb_ops_linux.h |  14 +-
>  2 files changed, 76 insertions(+), 227 deletions(-)

Greg,

It looks like the kbuild robot didn't like this causing DMA on the
stack, so please drop patch 08/09 from this set until I have fixed this.

Sorry for the spam .... my Monday is spilling into the week it seems.

Cheers,
Jes

>
> diff --git a/drivers/staging/rtl8723au/hal/usb_ops_linux.c b/drivers/staging/rtl8723au/hal/usb_ops_linux.c
> index 780658c..0ce37a4 100644
> --- a/drivers/staging/rtl8723au/hal/usb_ops_linux.c
> +++ b/drivers/staging/rtl8723au/hal/usb_ops_linux.c
> @@ -22,270 +22,119 @@
>  #include <rtl8723a_hal.h>
>  #include <rtl8723a_recv.h>
>  
> -static int usbctrl_vendorreq(struct rtw_adapter *padapter, u8 request,
> -			     u16 value, u16 index, void *pdata, u16 len,
> -			     u8 requesttype)
> +u8 rtl8723au_read8(struct rtw_adapter *padapter, u16 addr)
>  {
>  	struct dvobj_priv *pdvobjpriv = adapter_to_dvobj(padapter);
>  	struct usb_device *udev = pdvobjpriv->pusbdev;
> -	unsigned int pipe;
> -	int status = 0;
> -	u8 reqtype;
> -	u8 *pIo_buf;
> -	int vendorreq_times = 0;
> -
> -	if (padapter->bSurpriseRemoved) {
> -		RT_TRACE(_module_hci_ops_os_c_, _drv_err_,
> -			 ("usbctrl_vendorreq:(padapter->bSurpriseRemoved)!!!"));
> -		status = -EPERM;
> -		goto exit;
> -	}
> -
> -	if (len > MAX_VENDOR_REQ_CMD_SIZE) {
> -		DBG_8723A("[%s] Buffer len error , vendor request failed\n",
> -			  __func__);
> -		status = -EINVAL;
> -		goto exit;
> -	}
> -
> -	mutex_lock(&pdvobjpriv->usb_vendor_req_mutex);
> -
> -	/*  Acquire IO memory for vendorreq */
> -	pIo_buf = pdvobjpriv->usb_vendor_req_buf;
> -
> -	if (pIo_buf == NULL) {
> -		DBG_8723A("[%s] pIo_buf == NULL \n", __func__);
> -		status = -ENOMEM;
> -		goto release_mutex;
> -	}
> -
> -	while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
> -		memset(pIo_buf, 0, len);
> -
> -		if (requesttype == 0x01) {
> -			pipe = usb_rcvctrlpipe(udev, 0);/* read_in */
> -			reqtype =  REALTEK_USB_VENQT_READ;
> -		} else {
> -			pipe = usb_sndctrlpipe(udev, 0);/* write_out */
> -			reqtype =  REALTEK_USB_VENQT_WRITE;
> -			memcpy(pIo_buf, pdata, len);
> -		}
> -
> -		status = usb_control_msg(udev, pipe, request, reqtype,
> -					 value, index, pIo_buf, len,
> -					 RTW_USB_CONTROL_MSG_TIMEOUT);
> -
> -		if (status == len) {   /*  Success this control transfer. */
> -			rtw_reset_continual_urb_error(pdvobjpriv);
> -			if (requesttype == 0x01) {
> -				/* For Control read transfer, we have to copy
> -				 * the read data from pIo_buf to pdata.
> -				 */
> -				memcpy(pdata, pIo_buf,  len);
> -			}
> -		} else { /*  error cases */
> -			DBG_8723A("reg 0x%x, usb %s %u fail, status:%d value ="
> -				  " 0x%x, vendorreq_times:%d\n",
> -				  value, (requesttype == 0x01) ?
> -				  "read" : "write",
> -				  len, status, *(u32 *)pdata, vendorreq_times);
> -
> -			if (status < 0) {
> -				if (status == -ESHUTDOWN || status == -ENODEV)
> -					padapter->bSurpriseRemoved = true;
> -			} else { /*  status != len && status >= 0 */
> -				if (status > 0) {
> -					if (requesttype == 0x01) {
> -						/*
> -						 * For Control read transfer,
> -						 * we have to copy the read
> -						 * data from pIo_buf to pdata.
> -						 */
> -						memcpy(pdata, pIo_buf,  len);
> -					}
> -				}
> -			}
> -
> -			if (rtw_inc_and_chk_continual_urb_error(pdvobjpriv)) {
> -				padapter->bSurpriseRemoved = true;
> -				break;
> -			}
> -		}
> -
> -		/*  firmware download is checksumed, don't retry */
> -		if ((value >= FW_8723A_START_ADDRESS &&
> -		     value <= FW_8723A_END_ADDRESS) || status == len)
> -			break;
> -	}
> -
> -release_mutex:
> -	mutex_unlock(&pdvobjpriv->usb_vendor_req_mutex);
> -exit:
> -	return status;
> -}
> -
> -u8 rtl8723au_read8(struct rtw_adapter *padapter, u32 addr)
> -{
> -	u8 request;
> -	u8 requesttype;
> -	u16 wvalue;
> -	u16 index;
> -	u16 len;
> -	u8 data = 0;
> -
> -	request = 0x05;
> -	requesttype = 0x01;/* read_in */
> -	index = 0;/* n/a */
> -
> -	wvalue = (u16)(addr&0x0000ffff);
> -	len = 1;
> +	int len;
> +	u8 data;
>  
> -	usbctrl_vendorreq(padapter, request, wvalue, index, &data,
> -			  len, requesttype);
> +	len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> +			      REALTEK_USB_VENQT_CMD_REQ, REALTEK_USB_VENQT_READ,
> +			      addr, 0, &data, sizeof(data),
> +			      RTW_USB_CONTROL_MSG_TIMEOUT);
>  
>  	return data;
>  }
>  
> -u16 rtl8723au_read16(struct rtw_adapter *padapter, u32 addr)
> +u16 rtl8723au_read16(struct rtw_adapter *padapter, u16 addr)
>  {
> -	u8 request;
> -	u8 requesttype;
> -	u16 wvalue;
> -	u16 index;
> -	u16 len;
> +	struct dvobj_priv *pdvobjpriv = adapter_to_dvobj(padapter);
> +	struct usb_device *udev = pdvobjpriv->pusbdev;
> +	int len;
>  	__le16 data;
>  
> -	request = 0x05;
> -	requesttype = 0x01;/* read_in */
> -	index = 0;/* n/a */
> -
> -	wvalue = (u16)(addr&0x0000ffff);
> -	len = 2;
> -
> -	usbctrl_vendorreq(padapter, request, wvalue, index, &data,
> -			  len, requesttype);
> +	len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> +			      REALTEK_USB_VENQT_CMD_REQ, REALTEK_USB_VENQT_READ,
> +			      addr, 0, &data, sizeof(data),
> +			      RTW_USB_CONTROL_MSG_TIMEOUT);
>  
>  	return le16_to_cpu(data);
>  }
>  
> -u32 rtl8723au_read32(struct rtw_adapter *padapter, u32 addr)
> +u32 rtl8723au_read32(struct rtw_adapter *padapter, u16 addr)
>  {
> -	u8 request;
> -	u8 requesttype;
> -	u16 wvalue;
> -	u16 index;
> -	u16 len;
> +	struct dvobj_priv *pdvobjpriv = adapter_to_dvobj(padapter);
> +	struct usb_device *udev = pdvobjpriv->pusbdev;
> +	int len;
>  	__le32 data;
>  
> -	request = 0x05;
> -	requesttype = 0x01;/* read_in */
> -	index = 0;/* n/a */
> -
> -	wvalue = (u16)(addr&0x0000ffff);
> -	len = 4;
> -
> -	usbctrl_vendorreq(padapter, request, wvalue, index, &data,
> -			  len, requesttype);
> +	len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> +			      REALTEK_USB_VENQT_CMD_REQ, REALTEK_USB_VENQT_READ,
> +			      addr, 0, &data, sizeof(data),
> +			      RTW_USB_CONTROL_MSG_TIMEOUT);
>  
>  	return le32_to_cpu(data);
>  }
>  
> -int rtl8723au_write8(struct rtw_adapter *padapter, u32 addr, u8 val)
> +int rtl8723au_write8(struct rtw_adapter *padapter, u16 addr, u8 val)
>  {
> -	u8 request;
> -	u8 requesttype;
> -	u16 wvalue;
> -	u16 index;
> -	u16 len;
> -	u8 data;
> +	struct dvobj_priv *pdvobjpriv = adapter_to_dvobj(padapter);
> +	struct usb_device *udev = pdvobjpriv->pusbdev;
>  	int ret;
> +	u8 data = val;
>  
> -	request = 0x05;
> -	requesttype = 0x00;/* write_out */
> -	index = 0;/* n/a */
> -
> -	wvalue = (u16)(addr&0x0000ffff);
> -	len = 1;
> -
> -	data = val;
> -
> -	ret = usbctrl_vendorreq(padapter, request, wvalue, index, &data,
> -				len, requesttype);
> +	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
> +			      REALTEK_USB_VENQT_CMD_REQ,
> +			      REALTEK_USB_VENQT_WRITE,
> +			      addr, 0, &data, sizeof(data),
> +			      RTW_USB_CONTROL_MSG_TIMEOUT);
>  
> -	return ret;
> +	if (ret != sizeof(data))
> +		return _FAIL;
> +	return _SUCCESS;
>  }
>  
> -int rtl8723au_write16(struct rtw_adapter *padapter, u32 addr, u16 val)
> +int rtl8723au_write16(struct rtw_adapter *padapter, u16 addr, u16 val)
>  {
> -	u8 request;
> -	u8 requesttype;
> -	u16 wvalue;
> -	u16 index;
> -	u16 len;
> -	__le16 data;
> +	struct dvobj_priv *pdvobjpriv = adapter_to_dvobj(padapter);
> +	struct usb_device *udev = pdvobjpriv->pusbdev;
>  	int ret;
> +	__le16 data = cpu_to_le16(val);
>  
> -	request = 0x05;
> -	requesttype = 0x00;/* write_out */
> -	index = 0;/* n/a */
> -
> -	wvalue = (u16)(addr&0x0000ffff);
> -	len = 2;
> +	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
> +			      REALTEK_USB_VENQT_CMD_REQ,
> +			      REALTEK_USB_VENQT_WRITE,
> +			      addr, 0, &data, sizeof(data),
> +			      RTW_USB_CONTROL_MSG_TIMEOUT);
>  
> -	data = cpu_to_le16(val);
> -
> -	ret = usbctrl_vendorreq(padapter, request, wvalue, index, &data,
> -				len, requesttype);
> -	return ret;
> +	if (ret != sizeof(data))
> +		return _FAIL;
> +	return _SUCCESS;
>  }
>  
> -int rtl8723au_write32(struct rtw_adapter *padapter, u32 addr, u32 val)
> +int rtl8723au_write32(struct rtw_adapter *padapter, u16 addr, u32 val)
>  {
> -	u8 request;
> -	u8 requesttype;
> -	u16 wvalue;
> -	u16 index;
> -	u16 len;
> -	__le32 data;
> +	struct dvobj_priv *pdvobjpriv = adapter_to_dvobj(padapter);
> +	struct usb_device *udev = pdvobjpriv->pusbdev;
>  	int ret;
> +	__le32 data = cpu_to_le32(val);
>  
> -	request = 0x05;
> -	requesttype = 0x00;/* write_out */
> -	index = 0;/* n/a */
> +	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
> +			      REALTEK_USB_VENQT_CMD_REQ,
> +			      REALTEK_USB_VENQT_WRITE,
> +			      addr, 0, &data, sizeof(data),
> +			      RTW_USB_CONTROL_MSG_TIMEOUT);
>  
> -	wvalue = (u16)(addr&0x0000ffff);
> -	len = 4;
> -	data = cpu_to_le32(val);
> -
> -	ret = usbctrl_vendorreq(padapter, request, wvalue, index, &data,
> -				len, requesttype);
> -
> -	return ret;
> +	if (ret != sizeof(data))
> +		return _FAIL;
> +	return _SUCCESS;
>  }
>  
> -int rtl8723au_writeN(struct rtw_adapter *padapter,
> -		     u32 addr, u32 length, u8 *pdata)
> +int rtl8723au_writeN(struct rtw_adapter *padapter, u16 addr, u16 len, u8 *buf)
>  {
> -	u8 request;
> -	u8 requesttype;
> -	u16 wvalue;
> -	u16 index;
> -	u16 len;
> -	u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
> +	struct dvobj_priv *pdvobjpriv = adapter_to_dvobj(padapter);
> +	struct usb_device *udev = pdvobjpriv->pusbdev;
>  	int ret;
>  
> -	request = 0x05;
> -	requesttype = 0x00;/* write_out */
> -	index = 0;/* n/a */
> -
> -	wvalue = (u16)(addr&0x0000ffff);
> -	len = length;
> -	memcpy(buf, pdata, len);
> +	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
> +			      REALTEK_USB_VENQT_CMD_REQ,
> +			      REALTEK_USB_VENQT_WRITE,
> +			      addr, 0, buf, len, RTW_USB_CONTROL_MSG_TIMEOUT);
>  
> -	ret = usbctrl_vendorreq(padapter, request, wvalue, index, buf,
> -				len, requesttype);
> -
> -	return ret;
> +	if (ret != len)
> +		return _FAIL;
> +	return _SUCCESS;
>  }
>  
>  /*
> diff --git a/drivers/staging/rtl8723au/include/usb_ops_linux.h b/drivers/staging/rtl8723au/include/usb_ops_linux.h
> index e540a4b..bf68bbb 100644
> --- a/drivers/staging/rtl8723au/include/usb_ops_linux.h
> +++ b/drivers/staging/rtl8723au/include/usb_ops_linux.h
> @@ -29,13 +29,13 @@ int rtl8723au_write_port(struct rtw_adapter *padapter, u32 addr, u32 cnt,
>  void rtl8723au_write_port_cancel(struct rtw_adapter *padapter);
>  int rtl8723au_read_interrupt(struct rtw_adapter *adapter, u32 addr);
>  
> -u8 rtl8723au_read8(struct rtw_adapter *padapter, u32 addr);
> -u16 rtl8723au_read16(struct rtw_adapter *padapter, u32 addr);
> -u32 rtl8723au_read32(struct rtw_adapter *padapter, u32 addr);
> -int rtl8723au_write8(struct rtw_adapter *padapter, u32 addr, u8 val);
> -int rtl8723au_write16(struct rtw_adapter *padapter, u32 addr, u16 val);
> -int rtl8723au_write32(struct rtw_adapter *padapter, u32 addr, u32 val);
> +u8 rtl8723au_read8(struct rtw_adapter *padapter, u16 addr);
> +u16 rtl8723au_read16(struct rtw_adapter *padapter, u16 addr);
> +u32 rtl8723au_read32(struct rtw_adapter *padapter, u16 addr);
> +int rtl8723au_write8(struct rtw_adapter *padapter, u16 addr, u8 val);
> +int rtl8723au_write16(struct rtw_adapter *padapter, u16 addr, u16 val);
> +int rtl8723au_write32(struct rtw_adapter *padapter, u16 addr, u32 val);
>  int rtl8723au_writeN(struct rtw_adapter *padapter,
> -		     u32 addr, u32 length, u8 *pdata);
> +		     u16 addr, u16 length, u8 *pdata);
>  
>  #endif
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [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