[PATCH] staging: rtl8188eu: fix null dereference when kzalloc fails

[PATCH] staging: rtl8188eu: fix null dereference when kzalloc fails

[Connor Kuehl]

If kzalloc() returns NULL, the error path doesn't stop the flow of
control from entering rtw_hal_read_chip_version() which dereferences the
null pointer. Fix this by adding a 'goto' to the error path to more
gracefully handle the issue and avoid proceeding with initialization
steps that we're no longer prepared to handle.

Also update the debug message to be more consistent with the other debug
messages in this function.

Addresses-Coverity: ("Dereference after null check")

Signed-off-by: Connor Kuehl <connor.kuehl@canonical.com>
---
 drivers/staging/rtl8188eu/os_dep/usb_intf.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/os_dep/usb_intf.c b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
index 664d93a7f90d..4fac9dca798e 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
@@ -348,8 +348,10 @@ static struct adapter *rtw_usb_if1_init(struct dvobj_priv *dvobj,
 	}
 
 	padapter->HalData = kzalloc(sizeof(struct hal_data_8188e), GFP_KERNEL);
-	if (!padapter->HalData)
-		DBG_88E("cant not alloc memory for HAL DATA\n");
+	if (!padapter->HalData) {
+		DBG_88E("Failed to allocate memory for HAL data\n");
+		goto free_adapter;
+	}
 
 	/* step read_chip_version */
 	rtw_hal_read_chip_version(padapter);
-- 
2.17.1

Re: [PATCH] staging: rtl8188eu: fix null dereference when kzalloc fails

[Dan Carpenter]

On Fri, Sep 27, 2019 at 02:44:15PM -0700, Connor Kuehl wrote:
> If kzalloc() returns NULL, the error path doesn't stop the flow of
> control from entering rtw_hal_read_chip_version() which dereferences the
> null pointer. Fix this by adding a 'goto' to the error path to more
> gracefully handle the issue and avoid proceeding with initialization
> steps that we're no longer prepared to handle.
> 
> Also update the debug message to be more consistent with the other debug
> messages in this function.
> 
> Addresses-Coverity: ("Dereference after null check")
> 
> Signed-off-by: Connor Kuehl <connor.kuehl@canonical.com>
> ---
>  drivers/staging/rtl8188eu/os_dep/usb_intf.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/os_dep/usb_intf.c b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> index 664d93a7f90d..4fac9dca798e 100644
> --- a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> @@ -348,8 +348,10 @@ static struct adapter *rtw_usb_if1_init(struct dvobj_priv *dvobj,
>  	}
>  

There is another one earlier in the function as well.

drivers/staging/rtl8188eu/os_dep/usb_intf.c
   336  
   337          pnetdev = rtw_init_netdev(padapter);
   338          if (!pnetdev)
   339                  goto free_adapter;
   340          SET_NETDEV_DEV(pnetdev, dvobj_to_dev(dvobj));
   341          padapter = rtw_netdev_priv(pnetdev);
   342  
   343          if (padapter->registrypriv.monitor_enable) {
   344                  pmondev = rtl88eu_mon_init();
   345                  if (!pmondev)
   346                          netdev_warn(pnetdev, "Failed to initialize monitor interface");

goto free_adapter.

   347                  padapter->pmondev = pmondev;
   348          }
   349  
   350          padapter->HalData = kzalloc(sizeof(struct hal_data_8188e), GFP_KERNEL);
   351          if (!padapter->HalData)
   352                  DBG_88E("cant not alloc memory for HAL DATA\n");
   353  

>  	padapter->HalData = kzalloc(sizeof(struct hal_data_8188e), GFP_KERNEL);
> -	if (!padapter->HalData)
> -		DBG_88E("cant not alloc memory for HAL DATA\n");
> +	if (!padapter->HalData) {
> +		DBG_88E("Failed to allocate memory for HAL data\n");

Remove this debug printk.

> +		goto free_adapter;
> +	}


regards,
dan carpenter

Re: [PATCH] staging: rtl8188eu: fix null dereference when kzalloc fails

[Connor Kuehl]

On 10/1/19 6:11 AM, Dan Carpenter wrote:
> 
> There is another one earlier in the function as well.
> 
> drivers/staging/rtl8188eu/os_dep/usb_intf.c
>     336
>     337          pnetdev = rtw_init_netdev(padapter);
>     338          if (!pnetdev)
>     339                  goto free_adapter;
>     340          SET_NETDEV_DEV(pnetdev, dvobj_to_dev(dvobj));
>     341          padapter = rtw_netdev_priv(pnetdev);
>     342
>     343          if (padapter->registrypriv.monitor_enable) {
>     344                  pmondev = rtl88eu_mon_init();
>     345                  if (!pmondev)
>     346                          netdev_warn(pnetdev, "Failed to initialize monitor interface");
> 
> goto free_adapter.
> 
>     347                  padapter->pmondev = pmondev;
>     348          }
>     349
>     350          padapter->HalData = kzalloc(sizeof(struct hal_data_8188e), GFP_KERNEL);
>     351          if (!padapter->HalData)
>     352                  DBG_88E("cant not alloc memory for HAL DATA\n");
>     353
> 
>>   	padapter->HalData = kzalloc(sizeof(struct hal_data_8188e), GFP_KERNEL);
>> -	if (!padapter->HalData)
>> -		DBG_88E("cant not alloc memory for HAL DATA\n");
>> +	if (!padapter->HalData) {
>> +		DBG_88E("Failed to allocate memory for HAL data\n");
> 
> Remove this debug printk.
> 
>> +		goto free_adapter;
>> +	}

Hi Dan,

Sorry for such a late response! By the time I saw the e-mail with your 
feedback I also saw another e-mail saying this patch was accepted into a 
staging-linus tree. I'll address your comments in a separate patch.

Thank you,

Connor