* [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, linux-wireless,
netdev, linux-kernel
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).