2023-05-15 04:06:22

by shaozhengchao

Subject: [PATCH net-next] mac80211_hwsim: fix memory leak in hwsim_new_radio_nl

When parse_pmsr_capa failed in hwsim_new_radio_nl, the memory resources
applied for by pmsr_capa are not released. Add release processing to the
incorrect path.

Fixes: 92d13386ec55 ("mac80211_hwsim: add PMSR capability support")
Reported-by: [email protected]
Signed-off-by: Zhengchao Shao <[email protected]>
---
drivers/net/wireless/virtual/mac80211_hwsim.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/virtual/mac80211_hwsim.c b/drivers/net/wireless/virtual/mac80211_hwsim.c
index 9a8faaf4c6b6..6a50858a5645 100644
--- a/drivers/net/wireless/virtual/mac80211_hwsim.c
+++ b/drivers/net/wireless/virtual/mac80211_hwsim.c
@@ -5965,8 +5965,10 @@ static int hwsim_new_radio_nl(struct sk_buff *msg, struct genl_info *info)
goto out_free;
}
ret = parse_pmsr_capa(info->attrs[HWSIM_ATTR_PMSR_SUPPORT], pmsr_capa, info);
- if (ret)
+ if (ret) {
+ kfree(pmsr_capa);
goto out_free;
+ }
param.pmsr_capa = pmsr_capa;
}

--
2.34.1

2023-05-15 08:35:34

by Steen Hegelund

Subject: Re: [PATCH net-next] mac80211_hwsim: fix memory leak in hwsim_new_radio_nl

Hi Shao,

On Mon, 2023-05-15 at 11:47 +0800, Zhengchao Shao wrote:
> [You don't often get email from [email protected]. Learn why this is
> important at https://aka.ms/LearnAboutSenderIdentification ]
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> When parse_pmsr_capa failed in hwsim_new_radio_nl, the memory resources
> applied for by pmsr_capa are not released. Add release processing to the
> incorrect path.
>
> Fixes: 92d13386ec55 ("mac80211_hwsim: add PMSR capability support")
> Reported-by: [email protected]
> Signed-off-by: Zhengchao Shao <[email protected]>
> ---
>  drivers/net/wireless/virtual/mac80211_hwsim.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/virtual/mac80211_hwsim.c
> b/drivers/net/wireless/virtual/mac80211_hwsim.c
> index 9a8faaf4c6b6..6a50858a5645 100644
> --- a/drivers/net/wireless/virtual/mac80211_hwsim.c
> +++ b/drivers/net/wireless/virtual/mac80211_hwsim.c
> @@ -5965,8 +5965,10 @@ static int hwsim_new_radio_nl(struct sk_buff *msg,
> struct genl_info *info)
>                         goto out_free;
>                 }
>                 ret = parse_pmsr_capa(info->attrs[HWSIM_ATTR_PMSR_SUPPORT],
> pmsr_capa, info);
> -               if (ret)
> +               if (ret) {
> +                       kfree(pmsr_capa);

This should not be needed, see below.

>                         goto out_free;
> +               }
>                 param.pmsr_capa = pmsr_capa;


Why don't you just move this line up before the parse_pmsr_capa as there is
already a kfree(param.pmsr_capa) under the out_free label?

>         }
>
> --
> 2.34.1
>
>

BR
Steen

2023-05-15 09:10:50

by shaozhengchao

Subject: Re: [PATCH net-next] mac80211_hwsim: fix memory leak in hwsim_new_radio_nl

On 2023/5/15 16:30, Steen Hegelund wrote:
> Hi Shao,
>
> On Mon, 2023-05-15 at 11:47 +0800, Zhengchao Shao wrote:
>> [You don't often get email from [email protected]. Learn why this is
>> important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
>> content is safe
>>
>> When parse_pmsr_capa failed in hwsim_new_radio_nl, the memory resources
>> applied for by pmsr_capa are not released. Add release processing to the
>> incorrect path.
>>
>> Fixes: 92d13386ec55 ("mac80211_hwsim: add PMSR capability support")
>> Reported-by: [email protected]
>> Signed-off-by: Zhengchao Shao <[email protected]>
>> ---
>>  drivers/net/wireless/virtual/mac80211_hwsim.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/virtual/mac80211_hwsim.c
>> b/drivers/net/wireless/virtual/mac80211_hwsim.c
>> index 9a8faaf4c6b6..6a50858a5645 100644
>> --- a/drivers/net/wireless/virtual/mac80211_hwsim.c
>> +++ b/drivers/net/wireless/virtual/mac80211_hwsim.c
>> @@ -5965,8 +5965,10 @@ static int hwsim_new_radio_nl(struct sk_buff *msg,
>> struct genl_info *info)
>>                         goto out_free;
>>                 }
>>                 ret = parse_pmsr_capa(info->attrs[HWSIM_ATTR_PMSR_SUPPORT],
>> pmsr_capa, info);
>> -               if (ret)
>> +               if (ret) {
>> +                       kfree(pmsr_capa);
>
> This should not be needed, see below.
>
>>                         goto out_free;
>> +               }
>>                 param.pmsr_capa = pmsr_capa;
>
>
> Why don't you just move this line up before the parse_pmsr_capa as there is
> already a kfree(param.pmsr_capa) under the out_free label?
>

Hi Steen:
Your suggestion looks good. I will send V2.

Zhengchao Shao

>>         }
>>
>> --
>> 2.34.1
>>
>>
>
> BR
> Steen