| 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
| ||
|
Date: Tue, 20 May 2014 18:26:51 -0500
From: Larry Finger <Larry.Finger@...inger.net>
To: Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC: Manu Gupta <manugupt1@...il.com>,
Josh Triplett <josh@...htriplett.org>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: rtl8188eu: os_dep: usb_intf.c: Fix for possible
null pointer dereference
On 05/20/2014 04:31 PM, Rickard Strandqvist wrote:
> There is otherwise a risk of a possible null pointer dereference.
>
> Was largely found by using a static code analysis program called cppcheck.
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se>
> ---
> drivers/staging/rtl8188eu/os_dep/usb_intf.c | 127 ++++++++++++++-------------
> 1 file changed, 66 insertions(+), 61 deletions(-)
Although cppcheck's analysis is correct, pointer padapter can never be null in
any of these routines. In routine rtw_drv_init(), rtw_usb_if1_init() is called
to allocate memory for struct adapter for the driver. If that fails, none of
these other routines will ever be called as the driver will not be loaded.
If it is deemed better to fill the code with needless tests because some static
checker points out a place like this, that is OK with me, but I do not see the
point.
Larry
>
> diff --git a/drivers/staging/rtl8188eu/os_dep/usb_intf.c b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> index 2e49cd5..425b955 100644
> --- a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
> @@ -384,9 +384,16 @@ static void process_spec_devid(const struct usb_device_id *pdid)
>
> int rtw_hw_suspend(struct adapter *padapter)
> {
> - struct pwrctrl_priv *pwrpriv = &padapter->pwrctrlpriv;
> - struct net_device *pnetdev = padapter->pnetdev;
> + struct pwrctrl_priv *pwrpriv;
> + struct net_device *pnetdev;
>
> + if (!padapter) {
> + goto error_exit;
> + }
> +
> + /* system suspend */
> + pwrpriv = &padapter->pwrctrlpriv;
> + pnetdev = padapter->pnetdev;
>
> if ((!padapter->bup) || (padapter->bDriverStopped) ||
> (padapter->bSurpriseRemoved)) {
> @@ -396,49 +403,46 @@ int rtw_hw_suspend(struct adapter *padapter)
> goto error_exit;
> }
>
> - if (padapter) { /* system suspend */
> - LeaveAllPowerSaveMode(padapter);
> + LeaveAllPowerSaveMode(padapter);
>
> - DBG_88E("==> rtw_hw_suspend\n");
> - _enter_pwrlock(&pwrpriv->lock);
> - pwrpriv->bips_processing = true;
> - /* s1. */
> - if (pnetdev) {
> - netif_carrier_off(pnetdev);
> - rtw_netif_stop_queue(pnetdev);
> - }
> + DBG_88E("==> rtw_hw_suspend\n");
> + _enter_pwrlock(&pwrpriv->lock);
> + pwrpriv->bips_processing = true;
> + /* s1. */
> + if (pnetdev) {
> + netif_carrier_off(pnetdev);
> + rtw_netif_stop_queue(pnetdev);
> + }
>
> - /* s2. */
> - rtw_disassoc_cmd(padapter, 500, false);
> + /* s2. */
> + rtw_disassoc_cmd(padapter, 500, false);
>
> - /* s2-2. indicate disconnect to os */
> - {
> - struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
> + /* s2-2. indicate disconnect to os */
> + {
> + struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>
> - if (check_fwstate(pmlmepriv, _FW_LINKED)) {
> - _clr_fwstate_(pmlmepriv, _FW_LINKED);
> + if (check_fwstate(pmlmepriv, _FW_LINKED)) {
> + _clr_fwstate_(pmlmepriv, _FW_LINKED);
>
> - rtw_led_control(padapter, LED_CTL_NO_LINK);
> + rtw_led_control(padapter, LED_CTL_NO_LINK);
>
> - rtw_os_indicate_disconnect(padapter);
> + rtw_os_indicate_disconnect(padapter);
>
> - /* donnot enqueue cmd */
> - rtw_lps_ctrl_wk_cmd(padapter, LPS_CTRL_DISCONNECT, 0);
> - }
> + /* donnot enqueue cmd */
> + rtw_lps_ctrl_wk_cmd(padapter, LPS_CTRL_DISCONNECT, 0);
> }
> - /* s2-3. */
> - rtw_free_assoc_resources(padapter, 1);
> + }
> + /* s2-3. */
> + rtw_free_assoc_resources(padapter, 1);
>
> - /* s2-4. */
> - rtw_free_network_queue(padapter, true);
> - rtw_ips_dev_unload(padapter);
> - pwrpriv->rf_pwrstate = rf_off;
> - pwrpriv->bips_processing = false;
> + /* s2-4. */
> + rtw_free_network_queue(padapter, true);
> + rtw_ips_dev_unload(padapter);
> + pwrpriv->rf_pwrstate = rf_off;
> + pwrpriv->bips_processing = false;
> +
> + _exit_pwrlock(&pwrpriv->lock);
>
> - _exit_pwrlock(&pwrpriv->lock);
> - } else {
> - goto error_exit;
> - }
> return 0;
>
> error_exit:
> @@ -448,40 +452,41 @@ error_exit:
>
> int rtw_hw_resume(struct adapter *padapter)
> {
> - struct pwrctrl_priv *pwrpriv = &padapter->pwrctrlpriv;
> - struct net_device *pnetdev = padapter->pnetdev;
> -
> + struct pwrctrl_priv *pwrpriv;
> + struct net_device *pnetdev;
>
> - if (padapter) { /* system resume */
> - DBG_88E("==> rtw_hw_resume\n");
> - _enter_pwrlock(&pwrpriv->lock);
> - pwrpriv->bips_processing = true;
> - rtw_reset_drv_sw(padapter);
> + if (!padapter) {
> + goto error_exit;
> + }
> + /* system resume */
> + pwrpriv = &padapter->pwrctrlpriv;
> + pnetdev = padapter->pnetdev;
>
> - if (pm_netdev_open(pnetdev, false) != 0) {
> - _exit_pwrlock(&pwrpriv->lock);
> - goto error_exit;
> - }
> + DBG_88E("==> rtw_hw_resume\n");
> + _enter_pwrlock(&pwrpriv->lock);
> + pwrpriv->bips_processing = true;
> + rtw_reset_drv_sw(padapter);
>
> - netif_device_attach(pnetdev);
> - netif_carrier_on(pnetdev);
> + if (pm_netdev_open(pnetdev, false) != 0) {
> + _exit_pwrlock(&pwrpriv->lock);
> + goto error_exit;
> + }
>
> - if (!netif_queue_stopped(pnetdev))
> - netif_start_queue(pnetdev);
> - else
> - netif_wake_queue(pnetdev);
> + netif_device_attach(pnetdev);
> + netif_carrier_on(pnetdev);
>
> - pwrpriv->bkeepfwalive = false;
> - pwrpriv->brfoffbyhw = false;
> + if (!netif_queue_stopped(pnetdev))
> + netif_start_queue(pnetdev);
> + else
> + netif_wake_queue(pnetdev);
>
> - pwrpriv->rf_pwrstate = rf_on;
> - pwrpriv->bips_processing = false;
> + pwrpriv->bkeepfwalive = false;
> + pwrpriv->brfoffbyhw = false;
>
> - _exit_pwrlock(&pwrpriv->lock);
> - } else {
> - goto error_exit;
> - }
> + pwrpriv->rf_pwrstate = rf_on;
> + pwrpriv->bips_processing = false;
>
> + _exit_pwrlock(&pwrpriv->lock);
>
> return 0;
> error_exit:
>
--
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/
Powered by blists - more mailing lists