All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate a null pointer dereference
@ 2011-10-28 23:58 ` Julia Lawall
  0 siblings, 0 replies; 6+ 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] 6+ messages in thread

* [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate a null pointer dereference
@ 2011-10-28 23:58 ` Julia Lawall
  0 siblings, 0 replies; 6+ 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] 6+ messages in thread

* Re: [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate
  2011-10-28 23:58 ` Julia Lawall
@ 2011-10-29  2:27   ` Julian Calaby
  -1 siblings, 0 replies; 6+ 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] 6+ 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
  0 siblings, 0 replies; 6+ 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] 6+ messages in thread

* Re: [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c:
  2011-10-29  2:27   ` [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate a null pointer dereference Julian Calaby
@ 2011-10-29  9:28     ` Arend van Spriel
  -1 siblings, 0 replies; 6+ 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] 6+ messages in thread

* Re: [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate a null pointer dereference
@ 2011-10-29  9:28     ` Arend van Spriel
  0 siblings, 0 replies; 6+ 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] 6+ messages in thread

end of thread, other threads:[~2011-10-29  9:29 UTC | newest]

Thread overview: 6+ 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-28 23:58 ` Julia Lawall
2011-10-29  2:27 ` [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate Julian Calaby
2011-10-29  2:27   ` [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate a null pointer dereference Julian Calaby
2011-10-29  9:28   ` [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: Arend van Spriel
2011-10-29  9:28     ` [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate a null pointer dereference Arend van Spriel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.