Thread (3 messages) 3 messages, 3 authors, 2011-10-29

Re: [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate a null pointer dereference

From: Arend van Spriel <hidden>
Date: 2011-10-29 09:29:15
Also in: kernel-janitors, lkml, netdev

On 10/29/2011 04:27 AM, Julian Calaby wrote:
quoted hunk ↗ jump to hunk
On 29/10/11 10:58, Julia Lawall wrote:
quoted
From: Julia Lawall <redacted>

Delete di->name from the error reporting code, as it is meaningless if di
is NULL.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@r@
expression E, E1;
identifier f;
statement S1,S2,S3;
@@

if (E == NULL)
{
  ... when != if (E == NULL || ...) S1 else S2
      when != E = E1
*E->f
  ... when any
  return ...;
}
else S3
// </smpl>

Signed-off-by: Julia Lawall <redacted>

---
 drivers/net/wireless/brcm80211/brcmsmac/dma.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/brcm80211/brcmsmac/dma.c b/drivers/net/wireless/brcm80211/brcmsmac/dma.c
index b56a302..1d66f53 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/dma.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/dma.c
@@ -361,7 +361,7 @@ static uint _dma_ctrlflags(struct dma_info *di, uint mask, uint flags)
 	uint dmactrlflags = di->dma.dmactrlflags;
If di is null, we've already failed as it's dereferenced here.
quoted
 	if (di == NULL) {
-		DMA_ERROR(("%s: _dma_ctrlflags: NULL dma handle\n", di->name));
+		DMA_ERROR(("_dma_ctrlflags: NULL dma handle\n"));
 		return 0;
 	}
So, a better patch would be something like this:

(apologies if this doesn't apply - I've pretty much built it manually)

---

Though it's unlikely, di may be null, so we can't dereference
di->dma.dmactrlflags until we've checked it.

Move this de-reference after the check, and adjust the error
message to not require de-referencing di.

This is based upon Julia's original patch:
[ref]

Reported-by: Julia Lawall <redacted>
Signed-off-by: Julian Calaby <redacted>
CC: Julia Lawall <redacted>
diff --git a/drivers/net/wireless/brcm80211/brcmsmac/dma.c b/drivers/net/wireless/brcm80211/brcmsmac/dma.c
index b56a302..6ebec8f 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/dma.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/dma.c
@@ -358,13 +358,14 @@ static uint nrxdactive(struct dma_info *di, uint h, uint t
 
 static uint _dma_ctrlflags(struct dma_info *di, uint mask, uint flags)
 {
-       uint dmactrlflags = di->dma.dmactrlflags;
+       uint dmactrlflags;
 
        if (di == NULL) {
-               DMA_ERROR(("%s: _dma_ctrlflags: NULL dma handle\n", di->name));
+               DMA_ERROR(("_dma_ctrlflags: NULL dma handle\n"));
                return 0;
        }
 
+       dmactrlflags = di->dma.dmactrlflags;
        dmactrlflags &= ~mask;
        dmactrlflags |= flags;
 


Hi Julian, Julia

That change looks good, but it does not apply on top of our pending
patches. Probably need to resend those after the merge window. I will
create a applying patch and send it to John (and resend later if
needed). Thanks for finding this one.

Gr. AvS
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help