[PATCH v2 08/10] staging: rtl8723au: Sanitize USB read/write functions
Jes.Sorensen at redhat.com
Jes.Sorensen at redhat.com
Tue Jul 1 10:07:00 UTC 2014
From: Jes Sorensen <Jes.Sorensen at redhat.com>
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 at redhat.com>
---
drivers/staging/rtl8723au/hal/usb_ops_linux.c | 299 +++++++---------------
drivers/staging/rtl8723au/include/drv_types.h | 9 +-
drivers/staging/rtl8723au/include/usb_ops_linux.h | 14 +-
drivers/staging/rtl8723au/os_dep/usb_intf.c | 21 +-
4 files changed, 105 insertions(+), 238 deletions(-)
diff --git a/drivers/staging/rtl8723au/hal/usb_ops_linux.c b/drivers/staging/rtl8723au/hal/usb_ops_linux.c
index 780658c..c1b04c1 100644
--- 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 c06de68..df55e5b 100644
--- 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 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
diff --git a/drivers/staging/rtl8723au/os_dep/usb_intf.c b/drivers/staging/rtl8723au/os_dep/usb_intf.c
index e0d5514..ec90216 100644
--- 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)
--
1.9.3
More information about the devel
mailing list