2021-06-28 13:26:52

by Nguyen Dinh Phi

Subject: [PATCH V2] cfg80211: Fix possible memory leak in function cfg80211_bss_update

When we exceed the limit of BSS entries, this function will free the
new entry, however, at this time, it is the last door to access the
inputed ies, so these ies will be unreferenced objects and cause memory
leak.
Therefore we should free its ies before deallocating the new entry, beside
of dropping it from hidden_list.

Signed-off-by: Nguyen Dinh Phi <[email protected]>
---
V2: - Add subsystem to the subject line.
- Use bss_ref_put function for better clean-up dynamically allocated
cfg80211_internal_bss objects. It helps to clean relative hidden_bss.

net/wireless/scan.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index f03c7ac8e184..7897b1478c3c 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -1754,16 +1754,14 @@ cfg80211_bss_update(struct cfg80211_registered_device *rdev,
* be grouped with this beacon for updates ...
*/
if (!cfg80211_combine_bsses(rdev, new)) {
- kfree(new);
+ bss_ref_put(rdev, new);
goto drop;
}
}

if (rdev->bss_entries >= bss_entries_limit &&
!cfg80211_bss_expire_oldest(rdev)) {
- if (!list_empty(&new->hidden_list))
- list_del(&new->hidden_list);
- kfree(new);
+ bss_ref_put(rdev, new);
goto drop;
}

--
2.25.1

2021-07-29 10:35:13

by Greg KH

Subject: Re: [PATCH V2] cfg80211: Fix possible memory leak in function cfg80211_bss_update

On Mon, Jun 28, 2021 at 09:23:34PM +0800, Nguyen Dinh Phi wrote:
> When we exceed the limit of BSS entries, this function will free the
> new entry, however, at this time, it is the last door to access the
> inputed ies, so these ies will be unreferenced objects and cause memory
> leak.
> Therefore we should free its ies before deallocating the new entry, beside
> of dropping it from hidden_list.
>
> Signed-off-by: Nguyen Dinh Phi <[email protected]>
> ---
> V2: - Add subsystem to the subject line.
> - Use bss_ref_put function for better clean-up dynamically allocated
> cfg80211_internal_bss objects. It helps to clean relative hidden_bss.
>
> net/wireless/scan.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/net/wireless/scan.c b/net/wireless/scan.c
> index f03c7ac8e184..7897b1478c3c 100644
> --- a/net/wireless/scan.c
> +++ b/net/wireless/scan.c
> @@ -1754,16 +1754,14 @@ cfg80211_bss_update(struct cfg80211_registered_device *rdev,
> * be grouped with this beacon for updates ...
> */
> if (!cfg80211_combine_bsses(rdev, new)) {
> - kfree(new);
> + bss_ref_put(rdev, new);
> goto drop;
> }
> }
>
> if (rdev->bss_entries >= bss_entries_limit &&
> !cfg80211_bss_expire_oldest(rdev)) {
> - if (!list_empty(&new->hidden_list))
> - list_del(&new->hidden_list);
> - kfree(new);
> + bss_ref_put(rdev, new);
> goto drop;
> }
>
> --
> 2.25.1

Did this change get lost somewhere?

thanks,

greg k-h