linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] NET: ath5k, check ath5k_eeprom_mode_from_channel retval
@ 2013-02-07 13:44 Jiri Slaby
  2013-02-18  0:47 ` Nick Kossifidis
  0 siblings, 1 reply; 4+ messages in thread
From: Jiri Slaby @ 2013-02-07 13:44 UTC (permalink / raw)
  To: linville
  Cc: linux-wireless, ath5k-devel, jirislaby, linux-kernel,
	Nick Kossifidis, Luis R. Rodriguez

It can, if invalid argument given, return a negative value. In that
case we would access arrays out-of-bounds and such. Check the value
and yell loudly if that happened as it would be a bug in the
implementation. (Instead of silently corrupting memory.)

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Nick Kossifidis <mickflemm@gmail.com>
Cc: "Luis R. Rodriguez" <mcgrof@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath5k/phy.c   | 4 ++++
 drivers/net/wireless/ath/ath5k/reset.c | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
index ab363f3..a78afa9 100644
--- a/drivers/net/wireless/ath/ath5k/phy.c
+++ b/drivers/net/wireless/ath/ath5k/phy.c
@@ -1613,6 +1613,10 @@ ath5k_hw_update_noise_floor(struct ath5k_hw *ah)
 	ah->ah_cal_mask |= AR5K_CALIBRATION_NF;
 
 	ee_mode = ath5k_eeprom_mode_from_channel(ah->ah_current_channel);
+	if (WARN_ON(ee_mode < 0)) {
+		ah->ah_cal_mask &= ~AR5K_CALIBRATION_NF;
+		return;
+	}
 
 	/* completed NF calibration, test threshold */
 	nf = ath5k_hw_read_measured_noise_floor(ah);
diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
index 4084b10..e2d8b2c 100644
--- a/drivers/net/wireless/ath/ath5k/reset.c
+++ b/drivers/net/wireless/ath/ath5k/reset.c
@@ -985,6 +985,8 @@ ath5k_hw_commit_eeprom_settings(struct ath5k_hw *ah,
 		return;
 
 	ee_mode = ath5k_eeprom_mode_from_channel(channel);
+	if (WARN_ON(ee_mode < 0))
+		return;
 
 	/* Adjust power delta for channel 14 */
 	if (channel->center_freq == 2484)
-- 
1.8.1.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] NET: ath5k, check ath5k_eeprom_mode_from_channel retval
  2013-02-07 13:44 [PATCH] NET: ath5k, check ath5k_eeprom_mode_from_channel retval Jiri Slaby
@ 2013-02-18  0:47 ` Nick Kossifidis
  2013-02-19 13:36   ` Jiri Slaby
  0 siblings, 1 reply; 4+ messages in thread
From: Nick Kossifidis @ 2013-02-18  0:47 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: linville, linux-wireless, ath5k-devel, jirislaby, linux-kernel,
	Luis R. Rodriguez

2013/2/7 Jiri Slaby <jslaby@suse.cz>:
> It can, if invalid argument given, return a negative value. In that
> case we would access arrays out-of-bounds and such. Check the value
> and yell loudly if that happened as it would be a bug in the
> implementation. (Instead of silently corrupting memory.)
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Nick Kossifidis <mickflemm@gmail.com>
> Cc: "Luis R. Rodriguez" <mcgrof@qca.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath5k/phy.c   | 4 ++++
>  drivers/net/wireless/ath/ath5k/reset.c | 2 ++
>  2 files changed, 6 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
> index ab363f3..a78afa9 100644
> --- a/drivers/net/wireless/ath/ath5k/phy.c
> +++ b/drivers/net/wireless/ath/ath5k/phy.c
> @@ -1613,6 +1613,10 @@ ath5k_hw_update_noise_floor(struct ath5k_hw *ah)
>         ah->ah_cal_mask |= AR5K_CALIBRATION_NF;
>
>         ee_mode = ath5k_eeprom_mode_from_channel(ah->ah_current_channel);
> +       if (WARN_ON(ee_mode < 0)) {
> +               ah->ah_cal_mask &= ~AR5K_CALIBRATION_NF;
> +               return;
> +       }
>
>         /* completed NF calibration, test threshold */
>         nf = ath5k_hw_read_measured_noise_floor(ah);
> diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
> index 4084b10..e2d8b2c 100644
> --- a/drivers/net/wireless/ath/ath5k/reset.c
> +++ b/drivers/net/wireless/ath/ath5k/reset.c
> @@ -985,6 +985,8 @@ ath5k_hw_commit_eeprom_settings(struct ath5k_hw *ah,
>                 return;
>
>         ee_mode = ath5k_eeprom_mode_from_channel(channel);
> +       if (WARN_ON(ee_mode < 0))
> +               return;
>
>         /* Adjust power delta for channel 14 */
>         if (channel->center_freq == 2484)

int
ath5k_eeprom_mode_from_channel(struct ieee80211_channel *channel)
{
        switch (channel->hw_value) {
        case AR5K_MODE_11A:
                return AR5K_EEPROM_MODE_11A;
        case AR5K_MODE_11G:
                return AR5K_EEPROM_MODE_11G;
        case AR5K_MODE_11B:
                return AR5K_EEPROM_MODE_11B;
        default:
                return -1;
        }
}

I think we should just change that default to return 0 instead and add
an ATH5K_WARN there.



-- 
GPG ID: 0xEE878588
As you read this post global entropy rises. Have Fun ;-)
Nick

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] NET: ath5k, check ath5k_eeprom_mode_from_channel retval
  2013-02-18  0:47 ` Nick Kossifidis
@ 2013-02-19 13:36   ` Jiri Slaby
  2013-02-19 14:54     ` Nick Kossifidis
  0 siblings, 1 reply; 4+ messages in thread
From: Jiri Slaby @ 2013-02-19 13:36 UTC (permalink / raw)
  To: Nick Kossifidis, Jiri Slaby
  Cc: linville, linux-wireless, ath5k-devel, linux-kernel, Luis R. Rodriguez

[-- Attachment #1: Type: text/plain, Size: 712 bytes --]

On 02/18/2013 01:47 AM, Nick Kossifidis wrote:
> int
> ath5k_eeprom_mode_from_channel(struct ieee80211_channel *channel)
> {
>         switch (channel->hw_value) {
>         case AR5K_MODE_11A:
>                 return AR5K_EEPROM_MODE_11A;
>         case AR5K_MODE_11G:
>                 return AR5K_EEPROM_MODE_11G;
>         case AR5K_MODE_11B:
>                 return AR5K_EEPROM_MODE_11B;
>         default:
>                 return -1;
>         }
> }
> 
> I think we should just change that default to return 0 instead and add
> an ATH5K_WARN there.

Something like the attached patch? It needs ah to be propagated to
eeprom. If you are fine with that, I'll send it as patch...

thanks,
-- 
js
suse labs

[-- Attachment #2: 0001-ath5k-cleanup-channel-to-eprom_mode-function.patch --]
[-- Type: text/x-patch, Size: 4040 bytes --]

>From 0e75c33da1f8b35ff1d25f08650e95fc97c01528 Mon Sep 17 00:00:00 2001
From: Jiri Slaby <jslaby@suse.cz>
Date: Tue, 19 Feb 2013 14:31:13 +0100
Subject: [PATCH] ath5k: cleanup channel to eprom_mode function

Stop returning negative values from ath5k_eeprom_mode_from_channel.
Warn about that case instead and return the default/0/A. This cleans
up the callers, but needs to pass ah down to
ath5k_eeprom_mode_from_channel for ATH5K_WARN.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/net/wireless/ath/ath5k/eeprom.c |  6 ++++--
 drivers/net/wireless/ath/ath5k/eeprom.h |  5 ++++-
 drivers/net/wireless/ath/ath5k/phy.c    | 20 +++-----------------
 drivers/net/wireless/ath/ath5k/reset.c  |  4 +---
 4 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/eeprom.c b/drivers/net/wireless/ath/ath5k/eeprom.c
index b7e0258..94d34ee 100644
--- a/drivers/net/wireless/ath/ath5k/eeprom.c
+++ b/drivers/net/wireless/ath/ath5k/eeprom.c
@@ -1779,7 +1779,8 @@ ath5k_eeprom_detach(struct ath5k_hw *ah)
 }
 
 int
-ath5k_eeprom_mode_from_channel(struct ieee80211_channel *channel)
+ath5k_eeprom_mode_from_channel(struct ath5k_hw *ah,
+		struct ieee80211_channel *channel)
 {
 	switch (channel->hw_value) {
 	case AR5K_MODE_11A:
@@ -1789,6 +1790,7 @@ ath5k_eeprom_mode_from_channel(struct ieee80211_channel *channel)
 	case AR5K_MODE_11B:
 		return AR5K_EEPROM_MODE_11B;
 	default:
-		return -1;
+		ATH5K_WARN(ah, "channel is not A/B/G!");
+		return AR5K_EEPROM_MODE_11A;
 	}
 }
diff --git a/drivers/net/wireless/ath/ath5k/eeprom.h b/drivers/net/wireless/ath/ath5k/eeprom.h
index 94a9bbe..dcc3e40 100644
--- a/drivers/net/wireless/ath/ath5k/eeprom.h
+++ b/drivers/net/wireless/ath/ath5k/eeprom.h
@@ -494,5 +494,8 @@ struct ath5k_eeprom_info {
 	u32	ee_antenna[AR5K_EEPROM_N_MODES][AR5K_ANT_MAX];
 };
 
+struct ath5k_hw;
+
 int
-ath5k_eeprom_mode_from_channel(struct ieee80211_channel *channel);
+ath5k_eeprom_mode_from_channel(struct ath5k_hw *ah,
+		struct ieee80211_channel *channel);
diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
index a78afa9..d6bc7cb 100644
--- a/drivers/net/wireless/ath/ath5k/phy.c
+++ b/drivers/net/wireless/ath/ath5k/phy.c
@@ -1612,11 +1612,7 @@ ath5k_hw_update_noise_floor(struct ath5k_hw *ah)
 
 	ah->ah_cal_mask |= AR5K_CALIBRATION_NF;
 
-	ee_mode = ath5k_eeprom_mode_from_channel(ah->ah_current_channel);
-	if (WARN_ON(ee_mode < 0)) {
-		ah->ah_cal_mask &= ~AR5K_CALIBRATION_NF;
-		return;
-	}
+	ee_mode = ath5k_eeprom_mode_from_channel(ah, ah->ah_current_channel);
 
 	/* completed NF calibration, test threshold */
 	nf = ath5k_hw_read_measured_noise_floor(ah);
@@ -2317,12 +2313,7 @@ ath5k_hw_set_antenna_mode(struct ath5k_hw *ah, u8 ant_mode)
 
 	def_ant = ah->ah_def_ant;
 
-	ee_mode = ath5k_eeprom_mode_from_channel(channel);
-	if (ee_mode < 0) {
-		ATH5K_ERR(ah,
-			"invalid channel: %d\n", channel->center_freq);
-		return;
-	}
+	ee_mode = ath5k_eeprom_mode_from_channel(ah, channel);
 
 	switch (ant_mode) {
 	case AR5K_ANTMODE_DEFAULT:
@@ -3622,12 +3613,7 @@ ath5k_hw_txpower(struct ath5k_hw *ah, struct ieee80211_channel *channel,
 		return -EINVAL;
 	}
 
-	ee_mode = ath5k_eeprom_mode_from_channel(channel);
-	if (ee_mode < 0) {
-		ATH5K_ERR(ah,
-			"invalid channel: %d\n", channel->center_freq);
-		return -EINVAL;
-	}
+	ee_mode = ath5k_eeprom_mode_from_channel(ah, channel);
 
 	/* Initialize TX power table */
 	switch (ah->ah_radio) {
diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
index e2d8b2c..a3399c4 100644
--- a/drivers/net/wireless/ath/ath5k/reset.c
+++ b/drivers/net/wireless/ath/ath5k/reset.c
@@ -984,9 +984,7 @@ ath5k_hw_commit_eeprom_settings(struct ath5k_hw *ah,
 	if (ah->ah_version == AR5K_AR5210)
 		return;
 
-	ee_mode = ath5k_eeprom_mode_from_channel(channel);
-	if (WARN_ON(ee_mode < 0))
-		return;
+	ee_mode = ath5k_eeprom_mode_from_channel(ah, channel);
 
 	/* Adjust power delta for channel 14 */
 	if (channel->center_freq == 2484)
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] NET: ath5k, check ath5k_eeprom_mode_from_channel retval
  2013-02-19 13:36   ` Jiri Slaby
@ 2013-02-19 14:54     ` Nick Kossifidis
  0 siblings, 0 replies; 4+ messages in thread
From: Nick Kossifidis @ 2013-02-19 14:54 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: linville, linux-wireless, ath5k-devel, linux-kernel,
	Luis R. Rodriguez, Jiri Slaby

On Tue Feb 19 15:36:07 2013, Jiri Slaby wrote:
> On 02/18/2013 01:47 AM, Nick Kossifidis wrote:
>> int
>> ath5k_eeprom_mode_from_channel(struct ieee80211_channel *channel)
>> {
>>         switch (channel->hw_value) {
>>         case AR5K_MODE_11A:
>>                 return AR5K_EEPROM_MODE_11A;
>>         case AR5K_MODE_11G:
>>                 return AR5K_EEPROM_MODE_11G;
>>         case AR5K_MODE_11B:
>>                 return AR5K_EEPROM_MODE_11B;
>>         default:
>>                 return -1;
>>         }
>> }
>>
>> I think we should just change that default to return 0 instead and add
>> an ATH5K_WARN there.
>
> Something like the attached patch? It needs ah to be propagated to
> eeprom. If you are fine with that, I'll send it as patch...
>
> thanks,

Just move the prototype on ath5k.h with the rest of them...

1523 /* EEPROM access functions */
1524 int ath5k_eeprom_init(struct ath5k_hw *ah);
1525 void ath5k_eeprom_detach(struct ath5k_hw *ah);

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-02-19 14:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-07 13:44 [PATCH] NET: ath5k, check ath5k_eeprom_mode_from_channel retval Jiri Slaby
2013-02-18  0:47 ` Nick Kossifidis
2013-02-19 13:36   ` Jiri Slaby
2013-02-19 14:54     ` Nick Kossifidis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).