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