* [PATCH] nl80211: avoid possible memleak on nl80211_set_reg
@ 2016-06-06 14:56 Eduardo Abinader
2016-06-09 7:58 ` Johannes Berg
0 siblings, 1 reply; 5+ messages in thread
From: Eduardo Abinader @ 2016-06-06 14:56 UTC (permalink / raw)
To: linux-wireless, johannes; +Cc: Eduardo Abinader, johannes.berg
Setting NULL just after freeing regdomain.
Signed-off-by: Eduardo Abinader <eduardo.abinader@riverbed.com>
---
net/wireless/nl80211.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index d120449..39d107d 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -5839,10 +5839,11 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
r = set_regdom(rd, REGD_SOURCE_CRDA);
/* set_regdom took ownership */
- rd = NULL;
bad_reg:
kfree(rd);
+ rd = NULL;
+
return r;
}
#endif /* CONFIG_CFG80211_CRDA_SUPPORT */
--
2.5.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] nl80211: avoid possible memleak on nl80211_set_reg
2016-06-06 14:56 [PATCH] nl80211: avoid possible memleak on nl80211_set_reg Eduardo Abinader
@ 2016-06-09 7:58 ` Johannes Berg
2016-06-09 8:36 ` Arend Van Spriel
0 siblings, 1 reply; 5+ messages in thread
From: Johannes Berg @ 2016-06-09 7:58 UTC (permalink / raw)
To: Eduardo Abinader, linux-wireless; +Cc: Eduardo Abinader
On Mon, 2016-06-06 at 16:56 +0200, Eduardo Abinader wrote:
> Setting NULL just after freeing regdomain.
>
> Signed-off-by: Eduardo Abinader <eduardo.abinader@riverbed.com>
> ---
> net/wireless/nl80211.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index d120449..39d107d 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -5839,10 +5839,11 @@ static int nl80211_set_reg(struct sk_buff
> *skb, struct genl_info *info)
>
> r = set_regdom(rd, REGD_SOURCE_CRDA);
> /* set_regdom took ownership */
> - rd = NULL;
>
> bad_reg:
> kfree(rd);
> + rd = NULL;
To this I can only say: what?
johannes
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nl80211: avoid possible memleak on nl80211_set_reg
2016-06-09 7:58 ` Johannes Berg
@ 2016-06-09 8:36 ` Arend Van Spriel
2016-06-09 8:39 ` Johannes Berg
0 siblings, 1 reply; 5+ messages in thread
From: Arend Van Spriel @ 2016-06-09 8:36 UTC (permalink / raw)
To: Johannes Berg, Eduardo Abinader, linux-wireless; +Cc: Eduardo Abinader
On 9-6-2016 9:58, Johannes Berg wrote:
> On Mon, 2016-06-06 at 16:56 +0200, Eduardo Abinader wrote:
>> Setting NULL just after freeing regdomain.
>>
>> Signed-off-by: Eduardo Abinader <eduardo.abinader@riverbed.com>
>> ---
>> net/wireless/nl80211.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>> index d120449..39d107d 100644
>> --- a/net/wireless/nl80211.c
>> +++ b/net/wireless/nl80211.c
>> @@ -5839,10 +5839,11 @@ static int nl80211_set_reg(struct sk_buff
>> *skb, struct genl_info *info)
>>
>> r = set_regdom(rd, REGD_SOURCE_CRDA);
>> /* set_regdom took ownership */
>> - rd = NULL;
>>
>> bad_reg:
>> kfree(rd);
>> + rd = NULL;
>
> To this I can only say: what?
The patch is bad, but the confusion starts with the original code
(ab)using kfree() behaviour by setting rd to NULL. Personally, I do not
like it, but prefer it over bugs ;-)
Regards,
Arend
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nl80211: avoid possible memleak on nl80211_set_reg
2016-06-09 8:36 ` Arend Van Spriel
@ 2016-06-09 8:39 ` Johannes Berg
0 siblings, 0 replies; 5+ messages in thread
From: Johannes Berg @ 2016-06-09 8:39 UTC (permalink / raw)
To: Arend Van Spriel, Eduardo Abinader, linux-wireless; +Cc: Eduardo Abinader
> > > +++ b/net/wireless/nl80211.c
> > > @@ -5839,10 +5839,11 @@ static int nl80211_set_reg(struct sk_buff
> > > *skb, struct genl_info *info)
> > >
> > > r = set_regdom(rd, REGD_SOURCE_CRDA);
> > > /* set_regdom took ownership */
> > > - rd = NULL;
> > >
> > > bad_reg:
> > > kfree(rd);
> > > + rd = NULL;
> > To this I can only say: what?
> The patch is bad, but the confusion starts with the original code
> (ab)using kfree() behaviour by setting rd to NULL. Personally, I do
> not like it, but prefer it over bugs ;-)
>
Yeah, fair enough. I'll make the following patch:
- r = set_regdom(rd, REGD_SOURCE_CRDA);
- /* set_regdom took ownership */
- rd = NULL;
+ /* set_regdom takes ownership of rd */
+ return set_regdom(rd, REGD_SOURCE_CRDA);
johannes
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] nl80211: avoid possible memleak on nl80211_set_reg
@ 2016-06-06 14:46 Eduardo Abinader
0 siblings, 0 replies; 5+ messages in thread
From: Eduardo Abinader @ 2016-06-06 14:46 UTC (permalink / raw)
To: linux-wireless, johannes; +Cc: johannes.berg
Setting NULL just after freeing regdomain.
Signed-off-by: Eduardo Abinader <eduardo.abinader@riverbed.com>
---
net/wireless/nl80211.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index d120449..39d107d 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -5839,10 +5839,11 @@ static int nl80211_set_reg(struct sk_buff *skb,
struct genl_info *info)
r = set_regdom(rd, REGD_SOURCE_CRDA);
/* set_regdom took ownership */
- rd = NULL;
bad_reg:
kfree(rd);
+ rd = NULL;
+
return r;
}
#endif /* CONFIG_CFG80211_CRDA_SUPPORT */
--
2.5.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-06-09 8:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-06 14:56 [PATCH] nl80211: avoid possible memleak on nl80211_set_reg Eduardo Abinader
2016-06-09 7:58 ` Johannes Berg
2016-06-09 8:36 ` Arend Van Spriel
2016-06-09 8:39 ` Johannes Berg
-- strict thread matches above, loose matches on Subject: below --
2016-06-06 14:46 Eduardo Abinader
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.