staging: rtl8723au: Sanitize USB read/write functions

The original Realtek provided functions suffered badly from clutter to
accommodate broken operating systems. Lets try this lean and clean
version instead.

v2: Do not use the stack for data passed to usb_control_msg(). This
    requires reintroducing the mutex used in the old function. In
    addition, get rid of the no longer used 'usb_vendor_req_buf'.
    Note that rtl8723au_writeN() remains unlocked, so it can be used
    for bulk block transfers without having to retake the mutex for
    every write().
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

drivers/staging/rtl8723au/hal/usb_ops_linux.c

@@ -22,270 +22,149 @@
 #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;
-	}
+	int len;
+	u8 data;
 
 	mutex_lock(&pdvobjpriv->usb_vendor_req_mutex);
+	len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+			      REALTEK_USB_VENQT_CMD_REQ, REALTEK_USB_VENQT_READ,
+			      addr, 0, &pdvobjpriv->usb_buf.val8, sizeof(data),
+			      RTW_USB_CONTROL_MSG_TIMEOUT);
 
-	/*  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:
+	data = pdvobjpriv->usb_buf.val8;
 	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;
-
-	usbctrl_vendorreq(padapter, request, wvalue, index, &data,
-			  len, requesttype);
 
 	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;
-	__le16 data;
-
-	request = 0x05;
-	requesttype = 0x01;/* read_in */
-	index = 0;/* n/a */
+	struct dvobj_priv *pdvobjpriv = adapter_to_dvobj(padapter);
+	struct usb_device *udev = pdvobjpriv->pusbdev;
+	int len;
+	u16 data;
 
-	wvalue = (u16)(addr&0x0000ffff);
-	len = 2;
+	mutex_lock(&pdvobjpriv->usb_vendor_req_mutex);
+	len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+			      REALTEK_USB_VENQT_CMD_REQ, REALTEK_USB_VENQT_READ,
+			      addr, 0, &pdvobjpriv->usb_buf.val16, sizeof(data),
+			      RTW_USB_CONTROL_MSG_TIMEOUT);
 
-	usbctrl_vendorreq(padapter, request, wvalue, index, &data,
-			  len, requesttype);
+	data = le16_to_cpu(pdvobjpriv->usb_buf.val16);
+	mutex_unlock(&pdvobjpriv->usb_vendor_req_mutex);
 
-	return le16_to_cpu(data);
+	return 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;
-	__le32 data;
-
-	request = 0x05;
-	requesttype = 0x01;/* read_in */
-	index = 0;/* n/a */
+	struct dvobj_priv *pdvobjpriv = adapter_to_dvobj(padapter);
+	struct usb_device *udev = pdvobjpriv->pusbdev;
+	int len;
+	u32 data;
 
-	wvalue = (u16)(addr&0x0000ffff);
-	len = 4;
+	mutex_lock(&pdvobjpriv->usb_vendor_req_mutex);
+	len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+			      REALTEK_USB_VENQT_CMD_REQ, REALTEK_USB_VENQT_READ,
+			      addr, 0, &pdvobjpriv->usb_buf.val32, sizeof(data),
+			      RTW_USB_CONTROL_MSG_TIMEOUT);
 
-	usbctrl_vendorreq(padapter, request, wvalue, index, &data,
-			  len, requesttype);
+	data = le32_to_cpu(pdvobjpriv->usb_buf.val32);
+	mutex_unlock(&pdvobjpriv->usb_vendor_req_mutex);
 
-	return le32_to_cpu(data);
+	return 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;
 
-	request = 0x05;
-	requesttype = 0x00;/* write_out */
-	index = 0;/* n/a */
-
-	wvalue = (u16)(addr&0x0000ffff);
-	len = 1;
+	mutex_lock(&pdvobjpriv->usb_vendor_req_mutex);
+	pdvobjpriv->usb_buf.val8 = val;
 
-	data = val;
+	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+			      REALTEK_USB_VENQT_CMD_REQ,
+			      REALTEK_USB_VENQT_WRITE,
+			      addr, 0, &pdvobjpriv->usb_buf.val8, sizeof(val),
+			      RTW_USB_CONTROL_MSG_TIMEOUT);
 
-	ret = usbctrl_vendorreq(padapter, request, wvalue, index, &data,
-				len, requesttype);
+	if (ret != sizeof(val))
+		ret = _FAIL;
+	else
+		ret = _SUCCESS;
 
+	mutex_unlock(&pdvobjpriv->usb_vendor_req_mutex);
 	return ret;
 }
 
-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;
 
-	request = 0x05;
-	requesttype = 0x00;/* write_out */
-	index = 0;/* n/a */
+	mutex_lock(&pdvobjpriv->usb_vendor_req_mutex);
+	pdvobjpriv->usb_buf.val16 = cpu_to_le16(val);
 
-	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, &pdvobjpriv->usb_buf.val16, sizeof(val),
+			      RTW_USB_CONTROL_MSG_TIMEOUT);
 
-	data = cpu_to_le16(val);
+	if (ret != sizeof(val))
+		ret = _FAIL;
+	else
+		ret = _SUCCESS;
 
-	ret = usbctrl_vendorreq(padapter, request, wvalue, index, &data,
-				len, requesttype);
+	mutex_unlock(&pdvobjpriv->usb_vendor_req_mutex);
 	return ret;
 }
 
-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;
 
-	request = 0x05;
-	requesttype = 0x00;/* write_out */
-	index = 0;/* n/a */
+	mutex_lock(&pdvobjpriv->usb_vendor_req_mutex);
+	pdvobjpriv->usb_buf.val32 = cpu_to_le32(val);
 
-	wvalue = (u16)(addr&0x0000ffff);
-	len = 4;
-	data = cpu_to_le32(val);
+	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+			      REALTEK_USB_VENQT_CMD_REQ,
+			      REALTEK_USB_VENQT_WRITE,
+			      addr, 0, &pdvobjpriv->usb_buf.val32, sizeof(val),
+			      RTW_USB_CONTROL_MSG_TIMEOUT);
 
-	ret = usbctrl_vendorreq(padapter, request, wvalue, index, &data,
-				len, requesttype);
+	if (ret != sizeof(val))
+		ret = _FAIL;
+	else
+		ret = _SUCCESS;
 
+	mutex_unlock(&pdvobjpriv->usb_vendor_req_mutex);
 	return ret;
 }
 
-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;
 }
 
 /*

drivers/staging/rtl8723au/include/drv_types.h

@@ -177,10 +177,13 @@ struct dvobj_priv {
 	u8	RtNumOutPipes;
 	int	ep_num[5]; /* endpoint number */
 
-	struct mutex  usb_vendor_req_mutex;
+	struct mutex usb_vendor_req_mutex;
 
-	u8 *usb_alloc_vendor_req_buf;
-	u8 *usb_vendor_req_buf;
+	union {
+		__le32 val32;
+		__le16 val16;
+		u8 val8;
+	} usb_buf;
 
 	struct usb_interface *pusbintf;
 	struct usb_device *pusbdev;

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

drivers/staging/rtl8723au/os_dep/usb_intf.c

@@ -101,31 +101,16 @@ static inline int RT_usb_endpoint_num(const struct usb_endpoint_descriptor *epd)
 
 static int rtw_init_intf_priv(struct dvobj_priv *dvobj)
 {
-	int rst = _SUCCESS;
-
 	mutex_init(&dvobj->usb_vendor_req_mutex);
-	dvobj->usb_alloc_vendor_req_buf = kzalloc(MAX_USB_IO_CTL_SIZE,
-						  GFP_KERNEL);
-	if (dvobj->usb_alloc_vendor_req_buf == NULL) {
-		DBG_8723A("alloc usb_vendor_req_buf failed...\n");
-		rst = _FAIL;
-		goto exit;
-	}
-	dvobj->usb_vendor_req_buf =
-		PTR_ALIGN(dvobj->usb_alloc_vendor_req_buf, ALIGNMENT_UNIT);
-exit:
-	return rst;
+
+	return _SUCCESS;
 }
 
 static int rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
 {
-	int rst = _SUCCESS;
-
-	kfree(dvobj->usb_alloc_vendor_req_buf);
-
 	mutex_destroy(&dvobj->usb_vendor_req_mutex);
 
-	return rst;
+	return _SUCCESS;
 }
 
 static struct dvobj_priv *usb_dvobj_init(struct usb_interface *usb_intf)