linux-kernel - [PATCH] Fix NULL dereference in rtlwifi driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LNX.2.00.1101202309030.11854@swampdragon.chaosbits.net>
Date:	Thu, 20 Jan 2011 23:18:42 +0100 (CET)
From:	Jesper Juhl <jj@...osbits.net>
To:	Larry Finger <Larry.Finger@...inger.net>
cc:	netdev@...r.kernel.org, linux-wireless@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	"John W. Linville" <linville@...driver.com>,
	Chaoming Li <chaoming_li@...lsil.com.cn>
Subject: [PATCH] Fix NULL dereference in rtlwifi driver

In drivers/net/wireless/rtlwifi/pci.c::_rtl_pci_rx_interrupt() we call 
dev_alloc_skb(), which may fail and return NULL, but we do not check the 
returned value against NULL before dereferencing the returned pointer. 
This may lead to a NULL pointer dereference which means we'll crash - not 
good.

This patch tries to solve the issue by testing for NULL and bailing out if 
we couldn't allocate a skb. However, I don't know this code well, so I'm 
not sure that jumping to the 'done' label here is the correct action to 
take. Someone more knowledgable about this code than me should definately 
review it before it is applied anywhere.
While I was in the area I also moved an assignment in 
_rtl_pci_init_rx_ring() a bit - if the dev_alloc_skb() call in that 
function fails there's no reason to waste clock cycles assigning to the 
local variable 'entry', we may as well do that after the NULL check and 
potential bail out.

Here's the proposed patch, but please don't take it as much more than a 
bug report. If it happens to be correct, then by all means apply it, but 
I'm not personally making any guarantees.

Signed-off-by: Jesper Juhl <jj@...osbits.net>
---
 pci.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

  compile tested only, I don't have the hardware to test for real.

diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/pci.c
index 0fa36aa..5e99f89 100644
--- a/drivers/net/wireless/rtlwifi/pci.c
+++ b/drivers/net/wireless/rtlwifi/pci.c
@@ -619,6 +619,13 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
 					struct sk_buff *uskb = NULL;
 					u8 *pdata;
 					uskb = dev_alloc_skb(skb->len + 128);
+					if (!uskb) {
+						RT_TRACE(rtlpriv,
+							(COMP_INTR | COMP_RECV),
+							DBG_DMESG,
+							("can't alloc rx skb\n"));
+						goto done;
+					}
 					memcpy(IEEE80211_SKB_RXCB(uskb),
 							&rx_status,
 							sizeof(rx_status));
@@ -1066,9 +1073,9 @@ static int _rtl_pci_init_rx_ring(struct ieee80211_hw *hw)
 			struct sk_buff *skb =
 			    dev_alloc_skb(rtlpci->rxbuffersize);
 			u32 bufferaddress;
-			entry = &rtlpci->rx_ring[rx_queue_idx].desc[i];
 			if (!skb)
 				return 0;
+			entry = &rtlpci->rx_ring[rx_queue_idx].desc[i];

 			/*skb->dev = dev; */

-- 
Jesper Juhl <jj@...osbits.net>            http://www.chaosbits.net/
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/