2014-04-16 11:09:51

by Arik Nemtsov

[permalink] [raw]
Subject: [PATCH] cfg80211: avoid freeing last_request while in flight

Avoid freeing the last request while it is being processed. This can
happen in some cases if reg_work is kicked for some reason while the
currently pending request is in flight.

Signed-off-by: Arik Nemtsov <[email protected]>
---
This is to be applied on top of
"cfg80211: fix processing world regdomain when non modular"

net/wireless/reg.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index f59aaac..5ec04dc 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -240,8 +240,16 @@ static char user_alpha2[2];
module_param(ieee80211_regdom, charp, 0444);
MODULE_PARM_DESC(ieee80211_regdom, "IEEE 802.11 regulatory domain code");

-static void reg_free_request(struct regulatory_request *lr)
+static void reg_free_request(struct regulatory_request *request)
{
+ if (request != get_last_request())
+ kfree(request);
+}
+
+static void reg_free_last_request(void)
+{
+ struct regulatory_request *lr = get_last_request();
+
if (lr != &core_request_world && lr)
kfree_rcu(lr, rcu_head);
}
@@ -254,7 +262,7 @@ static void reg_update_last_request(struct regulatory_request *request)
if (lr == request)
return;

- reg_free_request(lr);
+ reg_free_last_request();
rcu_assign_pointer(last_request, request);
}

@@ -1616,7 +1624,7 @@ reg_process_hint_user(struct regulatory_request *user_request)
treatment = __reg_process_hint_user(user_request);
if (treatment == REG_REQ_IGNORE ||
treatment == REG_REQ_ALREADY_SET) {
- kfree(user_request);
+ reg_free_request(user_request);
return treatment;
}

@@ -1676,14 +1684,14 @@ reg_process_hint_driver(struct wiphy *wiphy,
case REG_REQ_OK:
break;
case REG_REQ_IGNORE:
- kfree(driver_request);
+ reg_free_request(driver_request);
return treatment;
case REG_REQ_INTERSECT:
/* fall through */
case REG_REQ_ALREADY_SET:
regd = reg_copy_regd(get_cfg80211_regdom());
if (IS_ERR(regd)) {
- kfree(driver_request);
+ reg_free_request(driver_request);
return REG_REQ_IGNORE;
}
rcu_assign_pointer(wiphy->regd, regd);
@@ -1777,10 +1785,10 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
case REG_REQ_IGNORE:
/* fall through */
case REG_REQ_ALREADY_SET:
- kfree(country_ie_request);
+ reg_free_request(country_ie_request);
return treatment;
case REG_REQ_INTERSECT:
- kfree(country_ie_request);
+ reg_free_request(country_ie_request);
/*
* This doesn't happen yet, not sure we
* ever want to support it for this case.
@@ -1841,7 +1849,7 @@ static void reg_process_hint(struct regulatory_request *reg_request)
return;

out_free:
- kfree(reg_request);
+ reg_free_request(reg_request);
}

/*
--
1.8.1.2



2014-04-22 03:01:02

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: avoid freeing last_request while in flight

On Thu, Apr 17, 2014 at 5:07 AM, Sander Eikelenboom
<[email protected]> wrote:
> Hi Johannes,
>
> Could you put whatever (combination of) patches you see fit into you -next
> tree .. so i can retest what will hopefully go upstream in the next mergewindow.
> I think that will also increase further testcoverage (and if something else
> arise it gives time to fix it before the next mergewindow). It would be nice
> if it would make it this time around ...

Sander, it'll probably be a while before the series you need gets
merged given the history I see on regression tests now on mac8021-nest
but I'll shortly send two respinned patches you can apply on top of
mac80211-next today if you'd like to test.

Luis

2014-04-22 15:17:35

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: avoid freeing last_request while in flight

On Wed, 2014-04-16 at 14:09 +0300, Arik Nemtsov wrote:
> Avoid freeing the last request while it is being processed. This can
> happen in some cases if reg_work is kicked for some reason while the
> currently pending request is in flight.
>
> Signed-off-by: Arik Nemtsov <[email protected]>
> ---
> This is to be applied on top of
> "cfg80211: fix processing world regdomain when non modular"

I'll apply Luis's version of this, not sure what's up with the respin
since he didn't give me a patch to apply to mac80211.git ...

johannes


2014-04-17 12:07:45

by Sander Eikelenboom

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: avoid freeing last_request while in flight

Hi Johannes,

Could you put whatever (combination of) patches you see fit into you -next
tree .. so i can retest what will hopefully go upstream in the next mergewindow.
I think that will also increase further testcoverage (and if something else
arise it gives time to fix it before the next mergewindow). It would be nice
if it would make it this time around ...

--
Sander


Wednesday, April 16, 2014, 1:09:47 PM, you wrote:

> Avoid freeing the last request while it is being processed. This can
> happen in some cases if reg_work is kicked for some reason while the
> currently pending request is in flight.

> Signed-off-by: Arik Nemtsov <[email protected]>
> ---
> This is to be applied on top of
> "cfg80211: fix processing world regdomain when non modular"

> net/wireless/reg.c | 24 ++++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)

> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index f59aaac..5ec04dc 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -240,8 +240,16 @@ static char user_alpha2[2];
> module_param(ieee80211_regdom, charp, 0444);
> MODULE_PARM_DESC(ieee80211_regdom, "IEEE 802.11 regulatory domain code");
>
> -static void reg_free_request(struct regulatory_request *lr)
> +static void reg_free_request(struct regulatory_request *request)
> {
> + if (request != get_last_request())
> + kfree(request);
> +}
> +
> +static void reg_free_last_request(void)
> +{
> + struct regulatory_request *lr = get_last_request();
> +
> if (lr != &core_request_world && lr)
> kfree_rcu(lr, rcu_head);
> }
> @@ -254,7 +262,7 @@ static void reg_update_last_request(struct regulatory_request *request)
> if (lr == request)
> return;
>
> - reg_free_request(lr);
> + reg_free_last_request();
> rcu_assign_pointer(last_request, request);
> }
>
> @@ -1616,7 +1624,7 @@ reg_process_hint_user(struct regulatory_request *user_request)
> treatment = __reg_process_hint_user(user_request);
> if (treatment == REG_REQ_IGNORE ||
> treatment == REG_REQ_ALREADY_SET) {
> - kfree(user_request);
> + reg_free_request(user_request);
> return treatment;
> }
>
> @@ -1676,14 +1684,14 @@ reg_process_hint_driver(struct wiphy *wiphy,
> case REG_REQ_OK:
> break;
> case REG_REQ_IGNORE:
> - kfree(driver_request);
> + reg_free_request(driver_request);
> return treatment;
> case REG_REQ_INTERSECT:
> /* fall through */
> case REG_REQ_ALREADY_SET:
> regd = reg_copy_regd(get_cfg80211_regdom());
> if (IS_ERR(regd)) {
> - kfree(driver_request);
> + reg_free_request(driver_request);
> return REG_REQ_IGNORE;
> }
> rcu_assign_pointer(wiphy->regd, regd);
> @@ -1777,10 +1785,10 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
> case REG_REQ_IGNORE:
> /* fall through */
> case REG_REQ_ALREADY_SET:
> - kfree(country_ie_request);
> + reg_free_request(country_ie_request);
> return treatment;
> case REG_REQ_INTERSECT:
> - kfree(country_ie_request);
> + reg_free_request(country_ie_request);
> /*
> * This doesn't happen yet, not sure we
> * ever want to support it for this case.
> @@ -1841,7 +1849,7 @@ static void reg_process_hint(struct regulatory_request *reg_request)
> return;
>
> out_free:
> - kfree(reg_request);
> + reg_free_request(reg_request);
> }
>
> /*


2014-04-22 02:58:54

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: avoid freeing last_request while in flight

On Wed, Apr 16, 2014 at 4:09 AM, Arik Nemtsov <[email protected]> wrote:
> Avoid freeing the last request while it is being processed. This can
> happen in some cases if reg_work is kicked for some reason while the
> currently pending request is in flight.
>
> Signed-off-by: Arik Nemtsov <[email protected]>
> ---
> This is to be applied on top of
> "cfg80211: fix processing world regdomain when non modular"

Thanks for help on this Arik, in this case we actually want to apply
this *before* the other regdomain patch, as such I'll address that and
resend your patch before mine, it requires just a minor change.

Luis