Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
From:   "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To:     Larry.Finger@lwfinger.net, Jaehee Park <jhpark1013@gmail.com>
Cc:     phil@philpotter.co.uk, gregkh@linuxfoundation.org,
        linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
        outreachy@lists.linux.dev, jhpark1013@gmail.com,
        Pavel Skripkin <paskripkin@gmail.com>
Subject: Re: [PATCH v2 1/6] staging: r8188eu: remove unused member free_bss_buf
Date:   Fri, 15 Apr 2022 06:29:15 +0200
Message-ID: <3608313.MHq7AAxBmi@leap>
In-Reply-To: <e41e0d3b62395d04f41d6934074074c431e7aacc.1649990500.git.jhpark1013@gmail.com>
References: <cover.1649990500.git.jhpark1013@gmail.com> <e41e0d3b62395d04f41d6934074074c431e7aacc.1649990500.git.jhpark1013@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="ISO-8859-1"
Precedence: bulk

On venerd=EC 15 aprile 2022 04:48:32 CEST Jaehee Park wrote:
> The free_bss_buf member of pmlmepriv is unused. Remove all related
> lines.
>=20
> Suggested-by: Pavel Skripkin <paskripkin@gmail.com>
> Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
> ---
>  drivers/staging/r8188eu/include/rtw_mlme.h | 1 -
>  drivers/staging/r8188eu/core/rtw_mlme.c    | 7 -------
>  2 files changed, 8 deletions(-)
>=20
> 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;
> =20
> 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=20
*padapter)
>  		res =3D _FAIL;
>  		goto exit;
>  	}
> -	pmlmepriv->free_bss_buf =3D pbuf;

Hi Jaehee,

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

>  	pnetwork =3D (struct wlan_network *)pbuf;
> =20
> @@ -109,13 +108,7 @@ void rtw_free_mlme_priv_ie_data(struct mlme_priv=20
*pmlmepriv)
> =20
>  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=20
should avoid :)=20

Why did you delete that call to vfree()?=20

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

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

Thanks,

=46abio M. De Francesco =20

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