Bug 60451

Summary: Missing NULL check of the return value of dev_alloc_skb() in function fwSendNullPacket() in file drivers/staging/rtl8192u/r819xU_firmware.c
Product: Drivers Reporter: RUC_Soft_Sec (rucsoftsec)
Component: StagingAssignee: drivers_staging (drivers_staging)
Status: RESOLVED CODE_FIX    
Severity: normal CC: levex, mrazec
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 3.10 Subsystem:
Regression: No Bisected commit-id:
Attachments: PATCH dev_alloc_skb() is checking against NULL value before it is used.

Description RUC_Soft_Sec 2013-07-04 06:36:37 UTC
In function fwSendNullPacket() at drivers/staging/rtl8192u/r819xU_firmware.c:110, the call to dev_alloc_skb() at line 126 may return a NULL pointer when there is no enough memory, but its return value is never checked against NULL before it is dereferenced at line 127, and thus an invalid memory access error may be triggered.
The related code snippets in function fwSendNullPacket() are as followings.
fwSendNullPacket @ drivers/staging/rtl8192u/r819xU_firmware.c:110
 110bool
 111fwSendNullPacket(
 112        struct net_device *dev,
 113        u32                     Length
 114)
 115{
           ...
 126        skb  = dev_alloc_skb(Length+ 4);
            //NOTE: skb should be checked against NULL
 127        memcpy((unsigned char *)(skb->cb),&dev,sizeof(dev));
 128        tcb_desc = (cb_desc*)(skb->cb + MAX_DEV_ADDR_SIZE);
 129        tcb_desc->queue_index = TXCMD_QUEUE;
 130        tcb_desc->bCmdOrInit = DESC_PACKET_TYPE_INIT;
 131        tcb_desc->bLastIniPkt = bLastInitPacket;
 132        ptr_buf = skb_put(skb, Length);
 133        memset(ptr_buf,0,Length);
 134        tcb_desc->txbuf_size= (u16)Length;
 135
 136        if(!priv->ieee80211->check_nic_enough_desc(dev,tcb_desc->queue_index)||
 137                        (!skb_queue_empty(&priv->ieee80211->skb_waitQ[tcb_desc->queue_index]))||\
 138                        (priv->ieee80211->queue_stop) ) {
 139                        RT_TRACE(COMP_FIRMWARE,"===================NULL packet==================================> tx full!\n");
 140                        skb_queue_tail(&priv->ieee80211->skb_waitQ[tcb_desc->queue_index], skb);
 141                } else {
 142                        priv->ieee80211->softmac_hard_start_xmit(skb,dev);
 143                }
 144
 145        //PlatformReleaseSpinLock(Adapter, RT_TX_SPINLOCK);
 146        return rtStatus;
 147}

Generally, the return value of dev_alloc_skb() shall be checked against NULL before it is used, like the following code snippets in function ieee80211_send_bar().
prism2sta_inf_authreq @ drivers/staging/wlan-ng/prism2sta.c:1554
1554static void prism2sta_inf_authreq(wlandevice_t *wlandev,
1555                                  hfa384x_InfFrame_t *inf)
1556{
1557        hfa384x_t *hw = (hfa384x_t *) wlandev->priv;
1558        struct sk_buff *skb;
1559
1560        skb = dev_alloc_skb(sizeof(*inf));
1561        if (skb) {
1562                skb_put(skb, sizeof(*inf));
1563                memcpy(skb->data, inf, sizeof(*inf));
1564                skb_queue_tail(&hw->authq, skb);
1565                schedule_work(&hw->link_bh);
1566        }
1567}

Thak you!

RUC_Soft_Sec, supported by China.X.Orion
Comment 1 Mario "Razec" Ponciano 2013-07-30 17:37:02 UTC
Created attachment 107044 [details]
PATCH dev_alloc_skb() is checking against NULL value before it is used.

According to the tip RUC_Soft_Sec, I'm creating a patch for verification of this value(skb = dev_alloc_skb()) when it is null and we will launch a warning(pr_warn) if this occurs. Please let me know if you have any questions or concerns.

Thanks in advance,

Mario 'Razec' Ponciano <mrazec@gmail.com>
Comment 2 Levente Kurusa 2014-02-15 12:51:53 UTC
(In reply to Mario "Razec" Ponciano from comment #1)
> Created attachment 107044 [details]
> PATCH dev_alloc_skb() is checking against NULL value before it is used.
> 
> According to the tip RUC_Soft_Sec, I'm creating a patch for verification of
> this value(skb = dev_alloc_skb()) when it is null and we will launch a
> warning(pr_warn) if this occurs. Please let me know if you have any
> questions or concerns.
> 
> Thanks in advance,
> 
> Mario 'Razec' Ponciano <mrazec@gmail.com>

Hi,

can you please post the patch with a proper warning message like this:

dev_warn(&dev->dev, "%s: Unable to allocate memory for sk buffer!\n", __func__);


To: Greg Kroah-Hartman <gregkh@linuxfoundation.org> 
To: Xenia Ragiadakou <burzalodowa@gmail.com>
Cc: devel@driverdev.osuosl.org
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: Levente Kurusa <levex@linux.com>

Thanks!
Levente Kurusa