Re: [wireless-next:master 207/237] drivers/net/wireless/brcm80211/brcmsmac/dma.c:352:20-24: ERROR: di is NULL but dereferenced.

[Seth Forshee <seth.forshee@canonical.com>]

On Thu, Nov 29, 2012 at 09:13:37AM +0800, Fengguang Wu wrote:
> Hi Seth,
> 
> FYI, there are coccinelle warnings in
> 
> tree:   git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-next.git master
> head:   0751f8654602e4255f0b9c17784d8100d5896010
> commit: 90123e045cac4ce8ec13e266f030c618fa674554 [207/237] brcmsmac: Add brcms_dbg_dma() debug macro
> 
> + drivers/net/wireless/brcm80211/brcmsmac/dma.c:352:20-24: ERROR: di is NULL but dereferenced.
> 
> vim +352 drivers/net/wireless/brcm80211/brcmsmac/dma.c
> 
> 5b435de0 Arend van Spriel 2011-10-05  336  
> 5b435de0 Arend van Spriel 2011-10-05  337  static uint ntxdactive(struct dma_info *di, uint h, uint t)
> 5b435de0 Arend van Spriel 2011-10-05  338  {
> 5b435de0 Arend van Spriel 2011-10-05  339  	return txd(di, t-h);
> 5b435de0 Arend van Spriel 2011-10-05  340  }
> 5b435de0 Arend van Spriel 2011-10-05  341  
> 5b435de0 Arend van Spriel 2011-10-05  342  static uint nrxdactive(struct dma_info *di, uint h, uint t)
> 5b435de0 Arend van Spriel 2011-10-05  343  {
> 5b435de0 Arend van Spriel 2011-10-05  344  	return rxd(di, t-h);
> 5b435de0 Arend van Spriel 2011-10-05  345  }
> 5b435de0 Arend van Spriel 2011-10-05  346  
> 5b435de0 Arend van Spriel 2011-10-05  347  static uint _dma_ctrlflags(struct dma_info *di, uint mask, uint flags)
> 5b435de0 Arend van Spriel 2011-10-05  348  {
> ae8e4672 Arend van Spriel 2011-10-29  349  	uint dmactrlflags;
> 5b435de0 Arend van Spriel 2011-10-05  350  
> 5b435de0 Arend van Spriel 2011-10-05  351  	if (di == NULL) {
> 90123e04 Seth Forshee     2012-11-15 @352  		brcms_dbg_dma(di->core, "NULL dma handle\n");
> 5b435de0 Arend van Spriel 2011-10-05  353  		return 0;
> 5b435de0 Arend van Spriel 2011-10-05  354  	}

Hi Fengguang,

Yep, that's obviously wrong. Thanks for the bug report.

John, here's a fix. There's no way to have a debug message if di is
NULL, so I've just removed it. Obviously I've never hitting that
condition anyway.

Seth


>From b0d7b62345e5b32b4022278f238296f5bdf06e8a Mon Sep 17 00:00:00 2001
From: Seth Forshee <seth.forshee@canonical.com>
Date: Thu, 29 Nov 2012 07:36:00 -0600
Subject: [PATCH] brcmsmac: Fix possible NULL pointer dereference in
 _dma_ctrlflags()

There's a debug message to warn if this function is passed a NULL
pointer, but in order to print the message we have to dereference the
pointer. Obviously this isn't a good idea, so remove the message.

Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/net/wireless/brcm80211/brcmsmac/dma.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmsmac/dma.c b/drivers/net/wireless/brcm80211/brcmsmac/dma.c
index 511e457..1860c57 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/dma.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/dma.c
@@ -349,10 +349,8 @@ static uint _dma_ctrlflags(struct dma_info *di, uint mask, uint flags)
 {
 	uint dmactrlflags;
 
-	if (di == NULL) {
-		brcms_dbg_dma(di->core, "NULL dma handle\n");
+	if (di == NULL)
 		return 0;
-	}
 
 	dmactrlflags = di->dma.dmactrlflags;
 	dmactrlflags &= ~mask;
-- 
1.7.9.5