linux-kernel - Re: [PATCH v2 1/6] staging: r8188eu: remove unused member free_bss

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 15 Apr 2022 06:29:15 +0200
From:   "Fabio M. De Francesco" <fmdefrancesco@...il.com>
To:     Larry.Finger@...inger.net, Jaehee Park <jhpark1013@...il.com>
Cc:     phil@...lpotter.co.uk, gregkh@...uxfoundation.org,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
        outreachy@...ts.linux.dev, jhpark1013@...il.com,
        Pavel Skripkin <paskripkin@...il.com>
Subject: Re: [PATCH v2 1/6] staging: r8188eu: remove unused member free_bss_buf

On venerdì 15 aprile 2022 04:48:32 CEST Jaehee Park wrote:
> The free_bss_buf member of pmlmepriv is unused. Remove all related
> lines.
> 
> Suggested-by: Pavel Skripkin <paskripkin@...il.com>
> Signed-off-by: Jaehee Park <jhpark1013@...il.com>
> ---
>  drivers/staging/r8188eu/include/rtw_mlme.h | 1 -
>  drivers/staging/r8188eu/core/rtw_mlme.c    | 7 -------
>  2 files changed, 8 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/include/rtw_mlme.h b/drivers/
staging/r8188eu/include/rtw_mlme.h
> index 1dc1fbf049af..0f03ac43079c 100644
> --- a/drivers/staging/r8188eu/include/rtw_mlme.h
> +++ b/drivers/staging/r8188eu/include/rtw_mlme.h
> @@ -319,7 +319,6 @@ struct mlme_priv {
>  	struct list_head *pscanned;
>  	struct __queue free_bss_pool;
>  	struct __queue scanned_queue;
> -	u8 *free_bss_buf;
>  	u8	key_mask; /* use to restore wep key after hal_init */
>  	u32	num_of_scanned;
>  
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/
r8188eu/core/rtw_mlme.c
> index 3e9882f89f76..aed868d1d47b 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme.c
> @@ -61,7 +61,6 @@ static int _rtw_init_mlme_priv(struct adapter 
*padapter)
>  		res = _FAIL;
>  		goto exit;
>  	}
> -	pmlmepriv->free_bss_buf = pbuf;

Hi Jaehee,

"pmlmepriv->free_bss_buf" is assigned with "pbuf". The latter is a pointer 
to virtually contiguous memory which was allocated by vmalloc() or 
vzalloc() (I didn't check, but the vfree() in _rtw_free_mlme_priv() tells 
me that indeed it was).

>  	pnetwork = (struct wlan_network *)pbuf;
>  
> @@ -109,13 +108,7 @@ void rtw_free_mlme_priv_ie_data(struct mlme_priv 
*pmlmepriv)
>  
>  void _rtw_free_mlme_priv(struct mlme_priv *pmlmepriv)
>  {
> -
>  	rtw_free_mlme_priv_ie_data(pmlmepriv);
> -
> -	if (pmlmepriv) {
> -		vfree(pmlmepriv->free_bss_buf);
> -	}

Therefore, here you are causing a memory leak, which is something you 
should avoid :) 

Why did you delete that call to vfree()? 

I think that you are misunderstanding what Pavel said. Even if it were true 
that the code makes no use of that region of memory (again, I didn't 
check), nevertheless, that memory was allocated somewhere and its address 
is now in "pmlmepriv->free_bss_buf".

If you can confirm that this memory is allocated for no purpose you should 
also remove the call to vmalloc() / vzalloc().

Thanks,

Fabio M. De Francesco  

> -
>  }
>  
>  struct	wlan_network *_rtw_alloc_network(struct	mlme_priv *pmlmepriv)/* 
_queue *free_queue) */
> -- 
> 2.25.1
>