Setting NULL just after freeing regdomain.
Signed-off-by: Eduardo Abinader <[email protected]>
---
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
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 <[email protected]>
>> ---
>> 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
On Mon, 2016-06-06 at 16:56 +0200, Eduardo Abinader wrote:
> Setting NULL just after freeing regdomain.
>
> Signed-off-by: Eduardo Abinader <[email protected]>
> ---
> 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
> > > +++ 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