* [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate a null pointer dereference
@ 2011-10-28 23:58 Julia Lawall
2011-10-29 2:27 ` Julian Calaby
0 siblings, 1 reply; 3+ messages in thread
From: Julia Lawall @ 2011-10-28 23:58 UTC (permalink / raw)
To: John W. Linville; +Cc: kernel-janitors, linux-wireless, netdev, linux-kernel
From: Julia Lawall <julia@diku.dk>
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 <julia@diku.dk>
---
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 == NULL) {
- DMA_ERROR(("%s: _dma_ctrlflags: NULL dma handle\n", di->name));
+ DMA_ERROR(("_dma_ctrlflags: NULL dma handle\n"));
return 0;
}
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate a null pointer dereference 2011-10-28 23:58 [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate a null pointer dereference Julia Lawall @ 2011-10-29 2:27 ` Julian Calaby 2011-10-29 9:28 ` Arend van Spriel 0 siblings, 1 reply; 3+ messages in thread From: Julian Calaby @ 2011-10-29 2:27 UTC (permalink / raw) To: Julia Lawall Cc: John W. Linville, kernel-janitors, linux-wireless, netdev, linux-kernel On 29/10/11 10:58, Julia Lawall wrote: > From: Julia Lawall <julia@diku.dk> > > 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 <julia@diku.dk> > > --- > 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. > 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: <1319846297-2985-2-git-send-email-julia@diku.dk> Reported-by: Julia Lawall <julia@diku.dk> Signed-off-by: Julian Calaby <julian.calaby@gmail.com> CC: Julia Lawall <julia@diku.dk> 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; -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/ .Plan: http://sites.google.com/site/juliancalaby ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate a null pointer dereference 2011-10-29 2:27 ` Julian Calaby @ 2011-10-29 9:28 ` Arend van Spriel 0 siblings, 0 replies; 3+ messages in thread From: Arend van Spriel @ 2011-10-29 9:28 UTC (permalink / raw) To: Julian Calaby Cc: Julia Lawall, John W. Linville, kernel-janitors@vger.kernel.org, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org On 10/29/2011 04:27 AM, Julian Calaby wrote: > On 29/10/11 10:58, Julia Lawall wrote: >> From: Julia Lawall <julia@diku.dk> >> >> 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 <julia@diku.dk> >> >> --- >> 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. > >> 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: > <1319846297-2985-2-git-send-email-julia@diku.dk> > > Reported-by: Julia Lawall <julia@diku.dk> > Signed-off-by: Julian Calaby <julian.calaby@gmail.com> > CC: Julia Lawall <julia@diku.dk> > > 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-10-29 9:29 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-10-28 23:58 [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate a null pointer dereference Julia Lawall 2011-10-29 2:27 ` Julian Calaby 2011-10-29 9:28 ` Arend van Spriel
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).