From patchwork Thu Jan 20 22:18:42 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jesper Juhl X-Patchwork-Id: 79774 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 45861B7105 for ; Fri, 21 Jan 2011 09:18:47 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753397Ab1ATWSY (ORCPT ); Thu, 20 Jan 2011 17:18:24 -0500 Received: from swampdragon.chaosbits.net ([90.184.90.115]:23964 "EHLO swampdragon.chaosbits.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752254Ab1ATWSX (ORCPT ); Thu, 20 Jan 2011 17:18:23 -0500 Received: by swampdragon.chaosbits.net (Postfix, from userid 1000) id 55C3A9403D; Thu, 20 Jan 2011 23:18:42 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by swampdragon.chaosbits.net (Postfix) with ESMTP id 532B99403B; Thu, 20 Jan 2011 23:18:42 +0100 (CET) Date: Thu, 20 Jan 2011 23:18:42 +0100 (CET) From: Jesper Juhl To: Larry Finger cc: netdev@vger.kernel.org, linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org, "John W. Linville" , Chaoming Li Subject: [PATCH] Fix NULL dereference in rtlwifi driver Message-ID: User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 Signed-off-by: Larry Finger --- 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; */