From 2786faa33bfc8d61b4fa45dd2e31664de796c837 Mon Sep 17 00:00:00 2001 From: Jes Sorensen Date: Tue, 1 Jul 2014 12:07:00 +0200 Subject: [PATCH] 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 Signed-off-by: Greg Kroah-Hartman --- 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 780658c80160..c1b04c13c392 100644 --- a/drivers/staging/rtl8723au/hal/usb_ops_linux.c +++ b/drivers/staging/rtl8723au/hal/usb_ops_linux.c @@ -22,270 +22,149 @@ #include #include -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 c06de681c74f..df55e5bbb6f2 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 e540a4bad087..bf68bbb41f9c 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 e0d55144627e..ec9021601b3e 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) -- 2.11.0