* [PATCH 0/3] Some fixes got r8712u
@ 2016-06-04 1:17 Larry Finger
2016-06-04 1:17 ` [PATCH 1/3] staging: r8712u: Check pointer before use Larry Finger
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Larry Finger @ 2016-06-04 1:17 UTC (permalink / raw)
To: gregkh; +Cc: devel, netdev, Larry Finger
While testing the patches that removed the semaphores from this driver,
kmemleak reported some leaked skbs. There were also some false positives.
I also found code that dereferenced a pointer before it was checked. All
of these conditions are fixed.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Larry Finger (3):
staging: r8712u: Check pointer before use
staging: r8712u: Fix leak of skb
staging: r8712u: Handle some false positives from kmemleak
drivers/staging/rtl8712/recv_linux.c | 1 -
drivers/staging/rtl8712/rtl8712_recv.h | 1 -
drivers/staging/rtl8712/usb_ops_linux.c | 86 ++++++++++++++-------------------
drivers/staging/rtl8712/xmit_linux.c | 2 +
4 files changed, 39 insertions(+), 51 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/3] staging: r8712u: Check pointer before use
2016-06-04 1:17 [PATCH 0/3] Some fixes got r8712u Larry Finger
@ 2016-06-04 1:17 ` Larry Finger
2016-06-04 1:17 ` [PATCH 2/3] staging: r8712u: Fix leak of skb Larry Finger
2016-06-04 1:17 ` [PATCH 3/3] staging: r8712u: Handle some false positives from kmemleak Larry Finger
2 siblings, 0 replies; 4+ messages in thread
From: Larry Finger @ 2016-06-04 1:17 UTC (permalink / raw)
To: gregkh; +Cc: netdev, devel, Larry Finger
Routine r8712_usb_read_port() dereferences "precvbuf" before testing it
for NULL.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
drivers/staging/rtl8712/usb_ops_linux.c | 72 ++++++++++++++++-----------------
1 file changed, 34 insertions(+), 38 deletions(-)
diff --git a/drivers/staging/rtl8712/usb_ops_linux.c b/drivers/staging/rtl8712/usb_ops_linux.c
index 6f12345..a79c35e 100644
--- a/drivers/staging/rtl8712/usb_ops_linux.c
+++ b/drivers/staging/rtl8712/usb_ops_linux.c
@@ -270,51 +270,47 @@ u32 r8712_usb_read_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *rmem)
struct usb_device *pusbd = pdvobj->pusbdev;
if (adapter->bDriverStopped || adapter->bSurpriseRemoved ||
- adapter->pwrctrlpriv.pnp_bstop_trx)
+ adapter->pwrctrlpriv.pnp_bstop_trx || !precvbuf)
return _FAIL;
if (precvbuf->reuse || !precvbuf->pskb) {
precvbuf->pskb = skb_dequeue(&precvpriv->free_recv_skb_queue);
if (precvbuf->pskb != NULL)
precvbuf->reuse = true;
}
- if (precvbuf != NULL) {
- r8712_init_recvbuf(adapter, precvbuf);
- /* re-assign for linux based on skb */
- if (!precvbuf->reuse || !precvbuf->pskb) {
- precvbuf->pskb = netdev_alloc_skb(adapter->pnetdev,
- MAX_RECVBUF_SZ + RECVBUFF_ALIGN_SZ);
- if (!precvbuf->pskb)
- return _FAIL;
- tmpaddr = (addr_t)precvbuf->pskb->data;
- alignment = tmpaddr & (RECVBUFF_ALIGN_SZ - 1);
- skb_reserve(precvbuf->pskb,
- (RECVBUFF_ALIGN_SZ - alignment));
- precvbuf->phead = precvbuf->pskb->head;
- precvbuf->pdata = precvbuf->pskb->data;
- precvbuf->ptail = skb_tail_pointer(precvbuf->pskb);
- precvbuf->pend = skb_end_pointer(precvbuf->pskb);
- precvbuf->pbuf = precvbuf->pskb->data;
- } else { /* reuse skb */
- precvbuf->phead = precvbuf->pskb->head;
- precvbuf->pdata = precvbuf->pskb->data;
- precvbuf->ptail = skb_tail_pointer(precvbuf->pskb);
- precvbuf->pend = skb_end_pointer(precvbuf->pskb);
- precvbuf->pbuf = precvbuf->pskb->data;
- precvbuf->reuse = false;
- }
- purb = precvbuf->purb;
- /* translate DMA FIFO addr to pipehandle */
- pipe = ffaddr2pipehdl(pdvobj, addr);
- usb_fill_bulk_urb(purb, pusbd, pipe,
- precvbuf->pbuf, MAX_RECVBUF_SZ,
- r8712_usb_read_port_complete,
- precvbuf);
- err = usb_submit_urb(purb, GFP_ATOMIC);
- if ((err) && (err != (-EPERM)))
- ret = _FAIL;
- } else {
- ret = _FAIL;
+ r8712_init_recvbuf(adapter, precvbuf);
+ /* re-assign for linux based on skb */
+ if (!precvbuf->reuse || !precvbuf->pskb) {
+ precvbuf->pskb = netdev_alloc_skb(adapter->pnetdev,
+ MAX_RECVBUF_SZ + RECVBUFF_ALIGN_SZ);
+ if (!precvbuf->pskb)
+ return _FAIL;
+ tmpaddr = (addr_t)precvbuf->pskb->data;
+ alignment = tmpaddr & (RECVBUFF_ALIGN_SZ - 1);
+ skb_reserve(precvbuf->pskb,
+ (RECVBUFF_ALIGN_SZ - alignment));
+ precvbuf->phead = precvbuf->pskb->head;
+ precvbuf->pdata = precvbuf->pskb->data;
+ precvbuf->ptail = skb_tail_pointer(precvbuf->pskb);
+ precvbuf->pend = skb_end_pointer(precvbuf->pskb);
+ precvbuf->pbuf = precvbuf->pskb->data;
+ } else { /* reuse skb */
+ precvbuf->phead = precvbuf->pskb->head;
+ precvbuf->pdata = precvbuf->pskb->data;
+ precvbuf->ptail = skb_tail_pointer(precvbuf->pskb);
+ precvbuf->pend = skb_end_pointer(precvbuf->pskb);
+ precvbuf->pbuf = precvbuf->pskb->data;
+ precvbuf->reuse = false;
}
+ purb = precvbuf->purb;
+ /* translate DMA FIFO addr to pipehandle */
+ pipe = ffaddr2pipehdl(pdvobj, addr);
+ usb_fill_bulk_urb(purb, pusbd, pipe,
+ precvbuf->pbuf, MAX_RECVBUF_SZ,
+ r8712_usb_read_port_complete,
+ precvbuf);
+ err = usb_submit_urb(purb, GFP_ATOMIC);
+ if ((err) && (err != (-EPERM)))
+ ret = _FAIL;
return ret;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/3] staging: r8712u: Fix leak of skb
2016-06-04 1:17 [PATCH 0/3] Some fixes got r8712u Larry Finger
2016-06-04 1:17 ` [PATCH 1/3] staging: r8712u: Check pointer before use Larry Finger
@ 2016-06-04 1:17 ` Larry Finger
2016-06-04 1:17 ` [PATCH 3/3] staging: r8712u: Handle some false positives from kmemleak Larry Finger
2 siblings, 0 replies; 4+ messages in thread
From: Larry Finger @ 2016-06-04 1:17 UTC (permalink / raw)
To: gregkh; +Cc: devel, netdev, Larry Finger
There are two types of messages queued for RX. The major type, which does
I/O on the device, was being handled properly. The skbs that communicated
with the firmware were being leaked.
While rewriting the code that sets up the skb, it was possible to remove
the private variable indicating that the old skb could be reused.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
drivers/staging/rtl8712/recv_linux.c | 1 -
drivers/staging/rtl8712/rtl8712_recv.h | 1 -
drivers/staging/rtl8712/usb_ops_linux.c | 24 ++++++++----------------
3 files changed, 8 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/rtl8712/recv_linux.c b/drivers/staging/rtl8712/recv_linux.c
index 735a0ea..576c15d 100644
--- a/drivers/staging/rtl8712/recv_linux.c
+++ b/drivers/staging/rtl8712/recv_linux.c
@@ -60,7 +60,6 @@ int r8712_os_recvbuf_resource_alloc(struct _adapter *padapter,
if (!precvbuf->purb)
res = _FAIL;
precvbuf->pskb = NULL;
- precvbuf->reuse = false;
precvbuf->pallocated_buf = NULL;
precvbuf->pbuf = NULL;
precvbuf->pdata = NULL;
diff --git a/drivers/staging/rtl8712/rtl8712_recv.h b/drivers/staging/rtl8712/rtl8712_recv.h
index fd9e3fc..925ec74 100644
--- a/drivers/staging/rtl8712/rtl8712_recv.h
+++ b/drivers/staging/rtl8712/rtl8712_recv.h
@@ -103,7 +103,6 @@ struct recv_buf {
struct _adapter *adapter;
struct urb *purb;
_pkt *pskb;
- u8 reuse;
u8 irp_pending;
u32 transfer_len;
uint len;
diff --git a/drivers/staging/rtl8712/usb_ops_linux.c b/drivers/staging/rtl8712/usb_ops_linux.c
index a79c35e..9a54c3b 100644
--- a/drivers/staging/rtl8712/usb_ops_linux.c
+++ b/drivers/staging/rtl8712/usb_ops_linux.c
@@ -202,26 +202,23 @@ static void r8712_usb_read_port_complete(struct urb *purb)
if (purb->status == 0) { /* SUCCESS */
if ((purb->actual_length > (MAX_RECVBUF_SZ)) ||
(purb->actual_length < RXDESC_SIZE)) {
- precvbuf->reuse = true;
r8712_read_port(padapter, precvpriv->ff_hwaddr, 0,
(unsigned char *)precvbuf);
} else {
+ _pkt *pskb = precvbuf->pskb;
+
precvbuf->transfer_len = purb->actual_length;
pbuf = (uint *)precvbuf->pbuf;
isevt = le32_to_cpu(*(pbuf + 1)) & 0x1ff;
if ((isevt & 0x1ff) == 0x1ff) {
r8712_rxcmd_event_hdl(padapter, pbuf);
- precvbuf->reuse = true;
+ skb_queue_tail(&precvpriv->rx_skb_queue, pskb);
r8712_read_port(padapter, precvpriv->ff_hwaddr,
0, (unsigned char *)precvbuf);
} else {
- _pkt *pskb = precvbuf->pskb;
-
skb_put(pskb, purb->actual_length);
skb_queue_tail(&precvpriv->rx_skb_queue, pskb);
tasklet_hi_schedule(&precvpriv->recv_tasklet);
- precvbuf->pskb = NULL;
- precvbuf->reuse = false;
r8712_read_port(padapter, precvpriv->ff_hwaddr,
0, (unsigned char *)precvbuf);
}
@@ -241,7 +238,6 @@ static void r8712_usb_read_port_complete(struct urb *purb)
}
/* Fall through. */
case -EPROTO:
- precvbuf->reuse = true;
r8712_read_port(padapter, precvpriv->ff_hwaddr, 0,
(unsigned char *)precvbuf);
break;
@@ -272,14 +268,11 @@ u32 r8712_usb_read_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *rmem)
if (adapter->bDriverStopped || adapter->bSurpriseRemoved ||
adapter->pwrctrlpriv.pnp_bstop_trx || !precvbuf)
return _FAIL;
- if (precvbuf->reuse || !precvbuf->pskb) {
- precvbuf->pskb = skb_dequeue(&precvpriv->free_recv_skb_queue);
- if (precvbuf->pskb != NULL)
- precvbuf->reuse = true;
- }
r8712_init_recvbuf(adapter, precvbuf);
- /* re-assign for linux based on skb */
- if (!precvbuf->reuse || !precvbuf->pskb) {
+ /* Try to use skb from the free queue */
+ precvbuf->pskb = skb_dequeue(&precvpriv->free_recv_skb_queue);
+
+ if (!precvbuf->pskb) {
precvbuf->pskb = netdev_alloc_skb(adapter->pnetdev,
MAX_RECVBUF_SZ + RECVBUFF_ALIGN_SZ);
if (!precvbuf->pskb)
@@ -293,13 +286,12 @@ u32 r8712_usb_read_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *rmem)
precvbuf->ptail = skb_tail_pointer(precvbuf->pskb);
precvbuf->pend = skb_end_pointer(precvbuf->pskb);
precvbuf->pbuf = precvbuf->pskb->data;
- } else { /* reuse skb */
+ } else { /* skb is reused */
precvbuf->phead = precvbuf->pskb->head;
precvbuf->pdata = precvbuf->pskb->data;
precvbuf->ptail = skb_tail_pointer(precvbuf->pskb);
precvbuf->pend = skb_end_pointer(precvbuf->pskb);
precvbuf->pbuf = precvbuf->pskb->data;
- precvbuf->reuse = false;
}
purb = precvbuf->purb;
/* translate DMA FIFO addr to pipehandle */
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] staging: r8712u: Handle some false positives from kmemleak
2016-06-04 1:17 [PATCH 0/3] Some fixes got r8712u Larry Finger
2016-06-04 1:17 ` [PATCH 1/3] staging: r8712u: Check pointer before use Larry Finger
2016-06-04 1:17 ` [PATCH 2/3] staging: r8712u: Fix leak of skb Larry Finger
@ 2016-06-04 1:17 ` Larry Finger
2 siblings, 0 replies; 4+ messages in thread
From: Larry Finger @ 2016-06-04 1:17 UTC (permalink / raw)
To: gregkh; +Cc: netdev, devel, Larry Finger
When this driver preallocates some URBs, kmemleak is unable to find that
allocated memory when it scans. When the driver is unloaded, that memory
is reclaimed, therefore, the report is a false positive.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
drivers/staging/rtl8712/xmit_linux.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/rtl8712/xmit_linux.c b/drivers/staging/rtl8712/xmit_linux.c
index 695f9b9..7f52953 100644
--- a/drivers/staging/rtl8712/xmit_linux.c
+++ b/drivers/staging/rtl8712/xmit_linux.c
@@ -31,6 +31,7 @@
#include <linux/usb.h>
#include <linux/ip.h>
#include <linux/if_ether.h>
+#include <linux/kmemleak.h>
#include "osdep_service.h"
#include "drv_types.h"
@@ -132,6 +133,7 @@ int r8712_xmit_resource_alloc(struct _adapter *padapter,
netdev_err(padapter->pnetdev, "pxmitbuf->pxmit_urb[i] == NULL\n");
return _FAIL;
}
+ kmemleak_not_leak(pxmitbuf->pxmit_urb[i]);
}
return _SUCCESS;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-06-04 1:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-04 1:17 [PATCH 0/3] Some fixes got r8712u Larry Finger
2016-06-04 1:17 ` [PATCH 1/3] staging: r8712u: Check pointer before use Larry Finger
2016-06-04 1:17 ` [PATCH 2/3] staging: r8712u: Fix leak of skb Larry Finger
2016-06-04 1:17 ` [PATCH 3/3] staging: r8712u: Handle some false positives from kmemleak Larry Finger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).