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>

diff --git a/drivers/staging/rtl8723au/hal/usb_ops_linux.c b/drivers/staging/rtl8723au/hal/usb_ops_linux.c
index 780658c80160dfb98d725c11f208aad2ad5c1cda..c1b04c13c3924635ded0b6b3a34254e3749925e7 100644 (file)
--- a/drivers/staging/rtl8723au/hal/usb_ops_linux.c
+++ b/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;
 }
 
 /*

diff --git a/drivers/staging/rtl8723au/include/drv_types.h b/drivers/staging/rtl8723au/include/drv_types.h
index c06de681c74fb2c83c4980bb9822287d6156f75e..df55e5bbb6f2a48811ae8d3ff002e7b52c0f7641 100644 (file)
--- a/drivers/staging/rtl8723au/include/drv_types.h
+++ b/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;

diff --git a/drivers/staging/rtl8723au/include/usb_ops_linux.h b/drivers/staging/rtl8723au/include/usb_ops_linux.h
index e540a4bad0877fb9b25c7c4f9c6c5b0dccb7313d..bf68bbb41f9c253c90b0fb97ca2088d35d5efd69 100644 (file)
--- 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

diff --git a/drivers/staging/rtl8723au/os_dep/usb_intf.c b/drivers/staging/rtl8723au/os_dep/usb_intf.c
index e0d55144627e49f70055adf09ac7296dff005d86..ec9021601b3ed9c694fa361f1c164305d9ff2563 100644 (file)
--- a/drivers/staging/rtl8723au/os_dep/usb_intf.c
+++ b/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)