patch to fix potential problem with wiphy_update_regulatory

patch to fix potential problem with wiphy_update_regulatory

[Sven Neumann]

While investigating the crashes I found that wiphy_update_regulatory
is from one place being called without the lock held that protects
the static variable it uses. I am sending you a patch that corrects
this issue. Unfortunately this change does not fix the crashes that
I reported. I am sending you the patch for review nevertheless.


Regards,
Sven

[PATCH] cfg80211: hold reg_mutex when updating regulatory

[Sven Neumann]

The function wiphy_update_regulatory() uses the static variable
last_request and thus needs to be called with reg_mutex held.
This is the case for all users in reg.c, but the function was
exported for use by wiphy_register(), from where it is called
without the lock being held.

Fix this by making wiphy_update_regulatory() private and introducing
regulatory_update() as a wrapper that acquires and holds the lock.

Signed-off-by: Sven Neumann <s.neumann@raumfeld.com>
---
 net/wireless/core.c |    2 +-
 net/wireless/core.h |    2 --
 net/wireless/reg.c  |   14 ++++++++++++--
 net/wireless/reg.h  |    2 ++
 4 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index c22ef34..179921e 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -577,7 +577,7 @@ int wiphy_register(struct wiphy *wiphy)
 	}
 
 	/* set up regulatory info */
-	wiphy_update_regulatory(wiphy, NL80211_REGDOM_SET_BY_CORE);
+	regulatory_update(wiphy, NL80211_REGDOM_SET_BY_CORE);
 
 	list_add_rcu(&rdev->list, &cfg80211_rdev_list);
 	cfg80211_rdev_list_generation++;
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 3dce1f1..0517d03 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -277,8 +277,6 @@ extern int cfg80211_dev_rename(struct cfg80211_registered_device *rdev,
 			       char *newname);
 
 void ieee80211_set_bitrate_flags(struct wiphy *wiphy);
-void wiphy_update_regulatory(struct wiphy *wiphy,
-			     enum nl80211_reg_initiator setby);
 
 void cfg80211_bss_expire(struct cfg80211_registered_device *dev);
 void cfg80211_bss_age(struct cfg80211_registered_device *dev,
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 1ad0f39..c61fd89 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1119,11 +1119,13 @@ static void reg_process_ht_flags(struct wiphy *wiphy)
 
 }
 
-void wiphy_update_regulatory(struct wiphy *wiphy,
-			     enum nl80211_reg_initiator initiator)
+static void wiphy_update_regulatory(struct wiphy *wiphy,
+				    enum nl80211_reg_initiator initiator)
 {
 	enum ieee80211_band band;
 
+	assert_reg_lock();
+
 	if (ignore_reg_update(wiphy, initiator))
 		goto out;
 	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
@@ -1137,6 +1139,14 @@ out:
 		wiphy->reg_notifier(wiphy, last_request);
 }
 
+void regulatory_update(struct wiphy *wiphy,
+		       enum nl80211_reg_initiator setby)
+{
+	mutex_lock(&reg_mutex);
+	wiphy_update_regulatory(wiphy, setby);
+	mutex_unlock(&reg_mutex);
+}
+
 static void handle_channel_custom(struct wiphy *wiphy,
 				  enum ieee80211_band band,
 				  unsigned int chan_idx,
diff --git a/net/wireless/reg.h b/net/wireless/reg.h
index b67d1c3..4a56799 100644
--- a/net/wireless/reg.h
+++ b/net/wireless/reg.h
@@ -16,6 +16,8 @@ void regulatory_exit(void);
 
 int set_regdom(const struct ieee80211_regdomain *rd);
 
+void regulatory_update(struct wiphy *wiphy, enum nl80211_reg_initiator setby);
+
 /**
  * regulatory_hint_found_beacon - hints a beacon was found on a channel
  * @wiphy: the wireless device where the beacon was found on
-- 
1.7.1

patch to fix potential problem with wiphy_update_regulatory (update)

[Sven Neumann]

Hi,

I am sorry, I've picked up the wrong version of the patch. In order
to compile the functions need to be reordered a little. Here's an
updated version that actually compiles (and works for me).


Regards,
Sven

[PATCH] cfg80211: hold reg_mutex when updating regulatory

[Sven Neumann]

The function wiphy_update_regulatory() uses the static variable
last_request and thus needs to be called with reg_mutex held.
This is the case for all users in reg.c, but the function was
exported for use by wiphy_register(), from where it is called
without the lock being held.

Fix this by making wiphy_update_regulatory() private and introducing
regulatory_update() as a wrapper that acquires and holds the lock.

Signed-off-by: Sven Neumann <s.neumann@raumfeld.com>
---
 net/wireless/core.c |    2 +-
 net/wireless/core.h |    2 -
 net/wireless/reg.c  |   62 +++++++++++++++++++++++++++++---------------------
 net/wireless/reg.h  |    2 +
 4 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index c22ef34..179921e 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -577,7 +577,7 @@ int wiphy_register(struct wiphy *wiphy)
 	}
 
 	/* set up regulatory info */
-	wiphy_update_regulatory(wiphy, NL80211_REGDOM_SET_BY_CORE);
+	regulatory_update(wiphy, NL80211_REGDOM_SET_BY_CORE);
 
 	list_add_rcu(&rdev->list, &cfg80211_rdev_list);
 	cfg80211_rdev_list_generation++;
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 3dce1f1..0517d03 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -277,8 +277,6 @@ extern int cfg80211_dev_rename(struct cfg80211_registered_device *rdev,
 			       char *newname);
 
 void ieee80211_set_bitrate_flags(struct wiphy *wiphy);
-void wiphy_update_regulatory(struct wiphy *wiphy,
-			     enum nl80211_reg_initiator setby);
 
 void cfg80211_bss_expire(struct cfg80211_registered_device *dev);
 void cfg80211_bss_age(struct cfg80211_registered_device *dev,
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 1ad0f39..a10dc34 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -912,14 +912,6 @@ static bool ignore_reg_update(struct wiphy *wiphy,
 	return false;
 }
 
-static void update_all_wiphy_regulatory(enum nl80211_reg_initiator initiator)
-{
-	struct cfg80211_registered_device *rdev;
-
-	list_for_each_entry(rdev, &cfg80211_rdev_list, list)
-		wiphy_update_regulatory(&rdev->wiphy, initiator);
-}
-
 static void handle_reg_beacon(struct wiphy *wiphy,
 			      unsigned int chan_idx,
 			      struct reg_beacon *reg_beacon)
@@ -1119,24 +1111,6 @@ static void reg_process_ht_flags(struct wiphy *wiphy)
 
 }
 
-void wiphy_update_regulatory(struct wiphy *wiphy,
-			     enum nl80211_reg_initiator initiator)
-{
-	enum ieee80211_band band;
-
-	if (ignore_reg_update(wiphy, initiator))
-		goto out;
-	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
-		if (wiphy->bands[band])
-			handle_band(wiphy, band, initiator);
-	}
-out:
-	reg_process_beacons(wiphy);
-	reg_process_ht_flags(wiphy);
-	if (wiphy->reg_notifier)
-		wiphy->reg_notifier(wiphy, last_request);
-}
-
 static void handle_channel_custom(struct wiphy *wiphy,
 				  enum ieee80211_band band,
 				  unsigned int chan_idx,
@@ -1423,6 +1397,42 @@ new_request:
 	return call_crda(last_request->alpha2);
 }
 
+static void wiphy_update_regulatory(struct wiphy *wiphy,
+				    enum nl80211_reg_initiator initiator)
+{
+	enum ieee80211_band band;
+
+	assert_reg_lock();
+
+	if (ignore_reg_update(wiphy, initiator))
+		goto out;
+	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
+		if (wiphy->bands[band])
+			handle_band(wiphy, band, initiator);
+	}
+out:
+	reg_process_beacons(wiphy);
+	reg_process_ht_flags(wiphy);
+	if (wiphy->reg_notifier)
+		wiphy->reg_notifier(wiphy, last_request);
+}
+
+static void update_all_wiphy_regulatory(enum nl80211_reg_initiator initiator)
+{
+	struct cfg80211_registered_device *rdev;
+
+	list_for_each_entry(rdev, &cfg80211_rdev_list, list)
+		wiphy_update_regulatory(&rdev->wiphy, initiator);
+}
+
+void regulatory_update(struct wiphy *wiphy,
+		       enum nl80211_reg_initiator setby)
+{
+	mutex_lock(&reg_mutex);
+	wiphy_update_regulatory(wiphy, setby);
+	mutex_unlock(&reg_mutex);
+}
+
 /* This processes *all* regulatory hints */
 static void reg_process_hint(struct regulatory_request *reg_request)
 {
diff --git a/net/wireless/reg.h b/net/wireless/reg.h
index b67d1c3..4a56799 100644
--- a/net/wireless/reg.h
+++ b/net/wireless/reg.h
@@ -16,6 +16,8 @@ void regulatory_exit(void);
 
 int set_regdom(const struct ieee80211_regdomain *rd);
 
+void regulatory_update(struct wiphy *wiphy, enum nl80211_reg_initiator setby);
+
 /**
  * regulatory_hint_found_beacon - hints a beacon was found on a channel
  * @wiphy: the wireless device where the beacon was found on
-- 
1.7.1

Re: [PATCH] cfg80211: hold reg_mutex when updating regulatory

[John W. Linville]

Luis, any comment on this?

On Tue, Jul 12, 2011 at 09:52:39AM +0200, Sven Neumann wrote:
> The function wiphy_update_regulatory() uses the static variable
> last_request and thus needs to be called with reg_mutex held.
> This is the case for all users in reg.c, but the function was
> exported for use by wiphy_register(), from where it is called
> without the lock being held.
> 
> Fix this by making wiphy_update_regulatory() private and introducing
> regulatory_update() as a wrapper that acquires and holds the lock.
> 
> Signed-off-by: Sven Neumann <s.neumann@raumfeld.com>
> ---
>  net/wireless/core.c |    2 +-
>  net/wireless/core.h |    2 -
>  net/wireless/reg.c  |   62 +++++++++++++++++++++++++++++---------------------
>  net/wireless/reg.h  |    2 +
>  4 files changed, 39 insertions(+), 29 deletions(-)
> 
> diff --git a/net/wireless/core.c b/net/wireless/core.c
> index c22ef34..179921e 100644
> --- a/net/wireless/core.c
> +++ b/net/wireless/core.c
> @@ -577,7 +577,7 @@ int wiphy_register(struct wiphy *wiphy)
>  	}
>  
>  	/* set up regulatory info */
> -	wiphy_update_regulatory(wiphy, NL80211_REGDOM_SET_BY_CORE);
> +	regulatory_update(wiphy, NL80211_REGDOM_SET_BY_CORE);
>  
>  	list_add_rcu(&rdev->list, &cfg80211_rdev_list);
>  	cfg80211_rdev_list_generation++;
> diff --git a/net/wireless/core.h b/net/wireless/core.h
> index 3dce1f1..0517d03 100644
> --- a/net/wireless/core.h
> +++ b/net/wireless/core.h
> @@ -277,8 +277,6 @@ extern int cfg80211_dev_rename(struct cfg80211_registered_device *rdev,
>  			       char *newname);
>  
>  void ieee80211_set_bitrate_flags(struct wiphy *wiphy);
> -void wiphy_update_regulatory(struct wiphy *wiphy,
> -			     enum nl80211_reg_initiator setby);
>  
>  void cfg80211_bss_expire(struct cfg80211_registered_device *dev);
>  void cfg80211_bss_age(struct cfg80211_registered_device *dev,
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 1ad0f39..a10dc34 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -912,14 +912,6 @@ static bool ignore_reg_update(struct wiphy *wiphy,
>  	return false;
>  }
>  
> -static void update_all_wiphy_regulatory(enum nl80211_reg_initiator initiator)
> -{
> -	struct cfg80211_registered_device *rdev;
> -
> -	list_for_each_entry(rdev, &cfg80211_rdev_list, list)
> -		wiphy_update_regulatory(&rdev->wiphy, initiator);
> -}
> -
>  static void handle_reg_beacon(struct wiphy *wiphy,
>  			      unsigned int chan_idx,
>  			      struct reg_beacon *reg_beacon)
> @@ -1119,24 +1111,6 @@ static void reg_process_ht_flags(struct wiphy *wiphy)
>  
>  }
>  
> -void wiphy_update_regulatory(struct wiphy *wiphy,
> -			     enum nl80211_reg_initiator initiator)
> -{
> -	enum ieee80211_band band;
> -
> -	if (ignore_reg_update(wiphy, initiator))
> -		goto out;
> -	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
> -		if (wiphy->bands[band])
> -			handle_band(wiphy, band, initiator);
> -	}
> -out:
> -	reg_process_beacons(wiphy);
> -	reg_process_ht_flags(wiphy);
> -	if (wiphy->reg_notifier)
> -		wiphy->reg_notifier(wiphy, last_request);
> -}
> -
>  static void handle_channel_custom(struct wiphy *wiphy,
>  				  enum ieee80211_band band,
>  				  unsigned int chan_idx,
> @@ -1423,6 +1397,42 @@ new_request:
>  	return call_crda(last_request->alpha2);
>  }
>  
> +static void wiphy_update_regulatory(struct wiphy *wiphy,
> +				    enum nl80211_reg_initiator initiator)
> +{
> +	enum ieee80211_band band;
> +
> +	assert_reg_lock();
> +
> +	if (ignore_reg_update(wiphy, initiator))
> +		goto out;
> +	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
> +		if (wiphy->bands[band])
> +			handle_band(wiphy, band, initiator);
> +	}
> +out:
> +	reg_process_beacons(wiphy);
> +	reg_process_ht_flags(wiphy);
> +	if (wiphy->reg_notifier)
> +		wiphy->reg_notifier(wiphy, last_request);
> +}
> +
> +static void update_all_wiphy_regulatory(enum nl80211_reg_initiator initiator)
> +{
> +	struct cfg80211_registered_device *rdev;
> +
> +	list_for_each_entry(rdev, &cfg80211_rdev_list, list)
> +		wiphy_update_regulatory(&rdev->wiphy, initiator);
> +}
> +
> +void regulatory_update(struct wiphy *wiphy,
> +		       enum nl80211_reg_initiator setby)
> +{
> +	mutex_lock(&reg_mutex);
> +	wiphy_update_regulatory(wiphy, setby);
> +	mutex_unlock(&reg_mutex);
> +}
> +
>  /* This processes *all* regulatory hints */
>  static void reg_process_hint(struct regulatory_request *reg_request)
>  {
> diff --git a/net/wireless/reg.h b/net/wireless/reg.h
> index b67d1c3..4a56799 100644
> --- a/net/wireless/reg.h
> +++ b/net/wireless/reg.h
> @@ -16,6 +16,8 @@ void regulatory_exit(void);
>  
>  int set_regdom(const struct ieee80211_regdomain *rd);
>  
> +void regulatory_update(struct wiphy *wiphy, enum nl80211_reg_initiator setby);
> +
>  /**
>   * regulatory_hint_found_beacon - hints a beacon was found on a channel
>   * @wiphy: the wireless device where the beacon was found on
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH] cfg80211: hold reg_mutex when updating regulatory

[Sven Neumann]

Hi,

On Fri, 2011-07-15 at 13:32 -0400, John W. Linville wrote:
> Luis, any comment on this?
> 
> On Tue, Jul 12, 2011 at 09:52:39AM +0200, Sven Neumann wrote:
> > The function wiphy_update_regulatory() uses the static variable
> > last_request and thus needs to be called with reg_mutex held.
> > This is the case for all users in reg.c, but the function was
> > exported for use by wiphy_register(), from where it is called
> > without the lock being held.
> > 
> > Fix this by making wiphy_update_regulatory() private and introducing
> > regulatory_update() as a wrapper that acquires and holds the lock.
> > 
> > Signed-off-by: Sven Neumann <s.neumann@raumfeld.com>

I'd appreciate if someone would review this patch. But probably this is
not really an issue except that it's somewhat ugly to export a function
that should be called with a lock held and that lock is actually
private. But in this particular case it is not a problem, as far as I
can see, since the only user of wiphy_update_regulatory() outside
net/wireless/reg.c is initialization code. So there is not likely going
to be a race condition here.


Sven

Re: [PATCH] cfg80211: hold reg_mutex when updating regulatory

[Luis R. Rodriguez]

On Fri, Jul 22, 2011 at 1:37 PM, Sven Neumann <s.neumann@raumfeld.com> wrote:
> Hi,
>
> On Fri, 2011-07-15 at 13:32 -0400, John W. Linville wrote:
>> Luis, any comment on this?
>>
>> On Tue, Jul 12, 2011 at 09:52:39AM +0200, Sven Neumann wrote:
>> > The function wiphy_update_regulatory() uses the static variable
>> > last_request and thus needs to be called with reg_mutex held.
>> > This is the case for all users in reg.c, but the function was
>> > exported for use by wiphy_register(), from where it is called
>> > without the lock being held.
>> >
>> > Fix this by making wiphy_update_regulatory() private and introducing
>> > regulatory_update() as a wrapper that acquires and holds the lock.
>> >
>> > Signed-off-by: Sven Neumann <s.neumann@raumfeld.com>
>
> I'd appreciate if someone would review this patch. But probably this is
> not really an issue except that it's somewhat ugly to export a function
> that should be called with a lock held and that lock is actually
> private. But in this particular case it is not a problem, as far as I
> can see, since the only user of wiphy_update_regulatory() outside
> net/wireless/reg.c is initialization code. So there is not likely going
> to be a race condition here.

Apologies for the delay and thanks for the patch. The patch seems good
except for the fact that there are so many changes reflected on the
patch itself and this can be avoided by splitting the work into a few
more patches so that the actual code changes required are reflected
cleanly in one patch. Can you perhaps split up your changes so that
moves of code are just that and actual code changes are reflected
elsewhere?

  Luis

Re: [PATCH] cfg80211: hold reg_mutex when updating regulatory

[Sven Neumann]

On Mon, 2011-07-25 at 13:21 -0700, Luis R. Rodriguez wrote:
> On Fri, Jul 22, 2011 at 1:37 PM, Sven Neumann <s.neumann@raumfeld.com> wrote:
> > Hi,
> >
> > On Fri, 2011-07-15 at 13:32 -0400, John W. Linville wrote:
> >> Luis, any comment on this?
> >>
> >> On Tue, Jul 12, 2011 at 09:52:39AM +0200, Sven Neumann wrote:
> >> > The function wiphy_update_regulatory() uses the static variable
> >> > last_request and thus needs to be called with reg_mutex held.
> >> > This is the case for all users in reg.c, but the function was
> >> > exported for use by wiphy_register(), from where it is called
> >> > without the lock being held.
> >> >
> >> > Fix this by making wiphy_update_regulatory() private and introducing
> >> > regulatory_update() as a wrapper that acquires and holds the lock.
> >> >
> >> > Signed-off-by: Sven Neumann <s.neumann@raumfeld.com>
> >
> > I'd appreciate if someone would review this patch. But probably this is
> > not really an issue except that it's somewhat ugly to export a function
> > that should be called with a lock held and that lock is actually
> > private. But in this particular case it is not a problem, as far as I
> > can see, since the only user of wiphy_update_regulatory() outside
> > net/wireless/reg.c is initialization code. So there is not likely going
> > to be a race condition here.
> 
> Apologies for the delay and thanks for the patch. The patch seems good
> except for the fact that there are so many changes reflected on the
> patch itself and this can be avoided by splitting the work into a few
> more patches so that the actual code changes required are reflected
> cleanly in one patch. Can you perhaps split up your changes so that
> moves of code are just that and actual code changes are reflected
> elsewhere?

Sure, I can do that. But I won't get to it before next week as I am
currently on vacation.


Thanks for the review,
Sven

Re: [PATCH] cfg80211: hold reg_mutex when updating regulatory

[John W. Linville]

On Sun, Jul 31, 2011 at 10:24:21AM +0200, Sven Neumann wrote:
> On Mon, 2011-07-25 at 13:21 -0700, Luis R. Rodriguez wrote:
> > On Fri, Jul 22, 2011 at 1:37 PM, Sven Neumann <s.neumann@raumfeld.com> wrote:
> > > Hi,
> > >
> > > On Fri, 2011-07-15 at 13:32 -0400, John W. Linville wrote:
> > >> Luis, any comment on this?
> > >>
> > >> On Tue, Jul 12, 2011 at 09:52:39AM +0200, Sven Neumann wrote:
> > >> > The function wiphy_update_regulatory() uses the static variable
> > >> > last_request and thus needs to be called with reg_mutex held.
> > >> > This is the case for all users in reg.c, but the function was
> > >> > exported for use by wiphy_register(), from where it is called
> > >> > without the lock being held.
> > >> >
> > >> > Fix this by making wiphy_update_regulatory() private and introducing
> > >> > regulatory_update() as a wrapper that acquires and holds the lock.
> > >> >
> > >> > Signed-off-by: Sven Neumann <s.neumann@raumfeld.com>
> > >
> > > I'd appreciate if someone would review this patch. But probably this is
> > > not really an issue except that it's somewhat ugly to export a function
> > > that should be called with a lock held and that lock is actually
> > > private. But in this particular case it is not a problem, as far as I
> > > can see, since the only user of wiphy_update_regulatory() outside
> > > net/wireless/reg.c is initialization code. So there is not likely going
> > > to be a race condition here.
> > 
> > Apologies for the delay and thanks for the patch. The patch seems good
> > except for the fact that there are so many changes reflected on the
> > patch itself and this can be avoided by splitting the work into a few
> > more patches so that the actual code changes required are reflected
> > cleanly in one patch. Can you perhaps split up your changes so that
> > moves of code are just that and actual code changes are reflected
> > elsewhere?
> 
> Sure, I can do that. But I won't get to it before next week as I am
> currently on vacation.

Did you have a nice vacation?  Are you ready to work on Linux again? :-)

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

[PATCH 1/2] cfg80211: hold reg_mutex when updating regulatory

[Sven Neumann]

The function wiphy_update_regulatory() uses the static variable
last_request and thus needs to be called with reg_mutex held.
This is the case for all users in reg.c, but the function was
exported for use by wiphy_register(), from where it is called
without the lock being held.

Fix this by making wiphy_update_regulatory() private and introducing
regulatory_update() as a wrapper that acquires and holds the lock.

Signed-off-by: Sven Neumann <s.neumann@raumfeld.com>
Cc: John W. Linville <linville@tuxdriver.com>
Cc: Luis R. Rodriguez <mcgrof@gmail.com>
Cc: Daniel Mack <daniel@zonque.org>
Cc: linux-wireless@vger.kernel.org
---
 net/wireless/core.c |    2 +-
 net/wireless/core.h |    2 --
 net/wireless/reg.c  |   17 +++++++++++++++--
 net/wireless/reg.h  |    2 ++
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index c148651..220f3bd 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -582,7 +582,7 @@ int wiphy_register(struct wiphy *wiphy)
 	}
 
 	/* set up regulatory info */
-	wiphy_update_regulatory(wiphy, NL80211_REGDOM_SET_BY_CORE);
+	regulatory_update(wiphy, NL80211_REGDOM_SET_BY_CORE);
 
 	list_add_rcu(&rdev->list, &cfg80211_rdev_list);
 	cfg80211_rdev_list_generation++;
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 8672e02..796a4bd 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -279,8 +279,6 @@ extern int cfg80211_dev_rename(struct cfg80211_registered_device *rdev,
 			       char *newname);
 
 void ieee80211_set_bitrate_flags(struct wiphy *wiphy);
-void wiphy_update_regulatory(struct wiphy *wiphy,
-			     enum nl80211_reg_initiator setby);
 
 void cfg80211_bss_expire(struct cfg80211_registered_device *dev);
 void cfg80211_bss_age(struct cfg80211_registered_device *dev,
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 02751db..c50016e 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -103,6 +103,9 @@ struct reg_beacon {
 	struct ieee80211_channel chan;
 };
 
+static void wiphy_update_regulatory(struct wiphy *wiphy,
+				    enum nl80211_reg_initiator initiator);
+
 static void reg_todo(struct work_struct *work);
 static DECLARE_WORK(reg_work, reg_todo);
 
@@ -1119,11 +1122,13 @@ static void reg_process_ht_flags(struct wiphy *wiphy)
 
 }
 
-void wiphy_update_regulatory(struct wiphy *wiphy,
-			     enum nl80211_reg_initiator initiator)
+static void wiphy_update_regulatory(struct wiphy *wiphy,
+				    enum nl80211_reg_initiator initiator)
 {
 	enum ieee80211_band band;
 
+	assert_reg_lock();
+
 	if (ignore_reg_update(wiphy, initiator))
 		return;
 
@@ -1138,6 +1143,14 @@ void wiphy_update_regulatory(struct wiphy *wiphy,
 		wiphy->reg_notifier(wiphy, last_request);
 }
 
+void regulatory_update(struct wiphy *wiphy,
+		       enum nl80211_reg_initiator setby)
+{
+	mutex_lock(&reg_mutex);
+	wiphy_update_regulatory(wiphy, setby);
+	mutex_unlock(&reg_mutex);
+}
+
 static void handle_channel_custom(struct wiphy *wiphy,
 				  enum ieee80211_band band,
 				  unsigned int chan_idx,
diff --git a/net/wireless/reg.h b/net/wireless/reg.h
index b67d1c3..4a56799 100644
--- a/net/wireless/reg.h
+++ b/net/wireless/reg.h
@@ -16,6 +16,8 @@ void regulatory_exit(void);
 
 int set_regdom(const struct ieee80211_regdomain *rd);
 
+void regulatory_update(struct wiphy *wiphy, enum nl80211_reg_initiator setby);
+
 /**
  * regulatory_hint_found_beacon - hints a beacon was found on a channel
  * @wiphy: the wireless device where the beacon was found on
-- 
1.7.4.1

[PATCH 2/2] cfg80211: reorder code to obsolete forward declaration

[Sven Neumann]

Reorder functions to remove the need for a forward declaration
introduced by the last commit.

Signed-off-by: Sven Neumann <s.neumann@raumfeld.com>
Cc: John W. Linville <linville@tuxdriver.com>
Cc: Luis R. Rodriguez <mcgrof@gmail.com>
Cc: Daniel Mack <daniel@zonque.org>
Cc: linux-wireless@vger.kernel.org
---
 net/wireless/reg.c |   19 ++++++++-----------
 1 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index c50016e..c77928b 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -103,9 +103,6 @@ struct reg_beacon {
 	struct ieee80211_channel chan;
 };
 
-static void wiphy_update_regulatory(struct wiphy *wiphy,
-				    enum nl80211_reg_initiator initiator);
-
 static void reg_todo(struct work_struct *work);
 static DECLARE_WORK(reg_work, reg_todo);
 
@@ -915,14 +912,6 @@ static bool ignore_reg_update(struct wiphy *wiphy,
 	return false;
 }
 
-static void update_all_wiphy_regulatory(enum nl80211_reg_initiator initiator)
-{
-	struct cfg80211_registered_device *rdev;
-
-	list_for_each_entry(rdev, &cfg80211_rdev_list, list)
-		wiphy_update_regulatory(&rdev->wiphy, initiator);
-}
-
 static void handle_reg_beacon(struct wiphy *wiphy,
 			      unsigned int chan_idx,
 			      struct reg_beacon *reg_beacon)
@@ -1151,6 +1140,14 @@ void regulatory_update(struct wiphy *wiphy,
 	mutex_unlock(&reg_mutex);
 }
 
+static void update_all_wiphy_regulatory(enum nl80211_reg_initiator initiator)
+{
+	struct cfg80211_registered_device *rdev;
+
+	list_for_each_entry(rdev, &cfg80211_rdev_list, list)
+		wiphy_update_regulatory(&rdev->wiphy, initiator);
+}
+
 static void handle_channel_custom(struct wiphy *wiphy,
 				  enum ieee80211_band band,
 				  unsigned int chan_idx,
-- 
1.7.4.1

Re: [PATCH 1/2] cfg80211: hold reg_mutex when updating regulatory

[Luis R. Rodriguez]

On Tue, Aug 30, 2011 at 2:38 PM, Sven Neumann <s.neumann@raumfeld.com> wrote:
> The function wiphy_update_regulatory() uses the static variable
> last_request and thus needs to be called with reg_mutex held.
> This is the case for all users in reg.c, but the function was
> exported for use by wiphy_register(), from where it is called
> without the lock being held.
>
> Fix this by making wiphy_update_regulatory() private and introducing
> regulatory_update() as a wrapper that acquires and holds the lock.
>
> Signed-off-by: Sven Neumann <s.neumann@raumfeld.com>
> Cc: John W. Linville <linville@tuxdriver.com>
> Cc: Luis R. Rodriguez <mcgrof@gmail.com>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: linux-wireless@vger.kernel.org

Acked-by:  Luis R. Rodriguez <mcgrof@qca.qualcomm.com>

  Luis

Re: [PATCH 2/2] cfg80211: reorder code to obsolete forward declaration

[Luis R. Rodriguez]

On Tue, Aug 30, 2011 at 2:38 PM, Sven Neumann <s.neumann@raumfeld.com> wrote:
> Reorder functions to remove the need for a forward declaration
> introduced by the last commit.
>
> Signed-off-by: Sven Neumann <s.neumann@raumfeld.com>
> Cc: John W. Linville <linville@tuxdriver.com>
> Cc: Luis R. Rodriguez <mcgrof@gmail.com>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: linux-wireless@vger.kernel.org

Acked-by: Luis R. Rodriguez <mcgrof@qca.qualcomm.com>

  Luis