All of lore.kernel.org
 help / color / mirror / Atom feed
From: Julian Calaby <julian.calaby@gmail.com>
To: Julia Lawall <julia@diku.dk>
Cc: "John W. Linville" <linville@tuxdriver.com>,
	kernel-janitors@vger.kernel.org, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate
Date: Sat, 29 Oct 2011 02:27:33 +0000	[thread overview]
Message-ID: <4EAB6495.40707@gmail.com> (raw)
In-Reply-To: <1319846297-2985-2-git-send-email-julia@diku.dk>

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

WARNING: multiple messages have this Message-ID (diff)
From: Julian Calaby <julian.calaby@gmail.com>
To: Julia Lawall <julia@diku.dk>
Cc: "John W. Linville" <linville@tuxdriver.com>,
	kernel-janitors@vger.kernel.org, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/5] drivers/net/wireless/brcm80211/brcmsmac/dma.c: eliminate a null pointer dereference
Date: Sat, 29 Oct 2011 13:27:33 +1100	[thread overview]
Message-ID: <4EAB6495.40707@gmail.com> (raw)
In-Reply-To: <1319846297-2985-2-git-send-email-julia@diku.dk>

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

  reply	other threads:[~2011-10-29  2:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 ` Julian Calaby [this message]
2011-10-29  2:27   ` 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EAB6495.40707@gmail.com \
    --to=julian.calaby@gmail.com \
    --cc=julia@diku.dk \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.