On Fri, Apr 15, 2022 at 06:29:15AM +0200, Fabio M. De Francesco wrote:
> 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@xxxxxxxxx>
> > Signed-off-by: Jaehee Park <jhpark1013@xxxxxxxxx>
> > ---
> > 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".
>
My understanding of Pavel's response is the free_bss_buf member of the
pmlmepriv structure wasn't being used anywhere and that the
rtw_free_mlme_riv_ie_data function frees the memory of the pmlmepriv
structure so the second check is redundant.
However, as Fabio said, the free_bss_buf member is being used and pbuf
memory is not being freed.
So I'll revert the patch as it was originally (which was just removing
the {} around the single if statement).
> 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
> >
>
>
