* [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).