2019-09-27 21:46:06

by Connor Kuehl

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

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 <[email protected]>
---
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

2019-10-01 13:15:35

by Dan Carpenter

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

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 <[email protected]>
> ---
> 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

2019-10-03 21:08:01

by Connor Kuehl

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

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