* [PATCH] ath9k: Prevent divide by zero kernel crash.
@ 2014-04-17 0:40 greearb
2014-04-17 7:47 ` Felix Fietkau
0 siblings, 1 reply; 5+ messages in thread
From: greearb @ 2014-04-17 0:40 UTC (permalink / raw)
To: ath9k-devel; +Cc: linux-wireless, Ben Greear
From: Ben Greear <greearb@candelatech.com>
Make sure we cannot ever assign beacon interval to zero.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
drivers/net/wireless/ath/ath9k/beacon.c | 4 ++++
drivers/net/wireless/ath/ath9k/recv.c | 3 ++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
index 2e8bba0..5391f01 100644
--- a/drivers/net/wireless/ath/ath9k/beacon.c
+++ b/drivers/net/wireless/ath/ath9k/beacon.c
@@ -443,6 +443,8 @@ static u32 ath9k_mod_tsf64_tu(u64 tsf, u32 div_tu)
{
u32 tsf_mod, tsf_hi, tsf_lo, mod_hi, mod_lo;
+ if (WARN_ON_ONCE(div_tu == 0))
+ div_tu = 100;
tsf_mod = tsf & (BIT(10) - 1);
tsf_hi = tsf >> 32;
tsf_lo = ((u32) tsf) >> 10;
@@ -667,6 +669,8 @@ static void ath9k_cache_beacon_config(struct ath_softc *sc,
"Caching beacon data for BSS: %pM\n", bss_conf->bssid);
cur_conf->beacon_interval = bss_conf->beacon_int;
+ if (WARN_ON_ONCE(cur_conf->beacon_interval == 0))
+ cur_conf->beacon_interval = 100;
cur_conf->dtim_period = bss_conf->dtim_period;
cur_conf->listen_interval = 1;
cur_conf->dtim_count = 1;
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index b97217d..79c20c7 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -538,7 +538,8 @@ static void ath_rx_ps_beacon(struct ath_softc *sc, struct sk_buff *skb)
sc->ps_flags &= ~PS_BEACON_SYNC;
ath_dbg(common, PS,
"Reconfigure beacon timers based on synchronized timestamp\n");
- ath9k_set_beacon(sc);
+ if (!(WARN_ON_ONCE(sc->cur_beacon_conf.beacon_interval == 0)))
+ ath9k_set_beacon(sc);
}
if (ath_beacon_dtim_pending_cab(skb)) {
--
1.7.11.7
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] ath9k: Prevent divide by zero kernel crash.
2014-04-17 0:40 [PATCH] ath9k: Prevent divide by zero kernel crash greearb
@ 2014-04-17 7:47 ` Felix Fietkau
2014-04-22 17:53 ` John W. Linville
2014-04-30 18:47 ` Ben Greear
0 siblings, 2 replies; 5+ messages in thread
From: Felix Fietkau @ 2014-04-17 7:47 UTC (permalink / raw)
To: greearb, ath9k-devel; +Cc: linux-wireless
On 2014-04-17 02:40, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
>
> Make sure we cannot ever assign beacon interval to zero.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
> drivers/net/wireless/ath/ath9k/beacon.c | 4 ++++
> drivers/net/wireless/ath/ath9k/recv.c | 3 ++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
> index 2e8bba0..5391f01 100644
> --- a/drivers/net/wireless/ath/ath9k/beacon.c
> +++ b/drivers/net/wireless/ath/ath9k/beacon.c
> @@ -443,6 +443,8 @@ static u32 ath9k_mod_tsf64_tu(u64 tsf, u32 div_tu)
> {
> u32 tsf_mod, tsf_hi, tsf_lo, mod_hi, mod_lo;
>
> + if (WARN_ON_ONCE(div_tu == 0))
> + div_tu = 100;
> tsf_mod = tsf & (BIT(10) - 1);
> tsf_hi = tsf >> 32;
> tsf_lo = ((u32) tsf) >> 10;
Why add this warning here if you already have the additions below? We
don't need multiple layers of defensive checks for the same thing.
> @@ -667,6 +669,8 @@ static void ath9k_cache_beacon_config(struct ath_softc *sc,
> "Caching beacon data for BSS: %pM\n", bss_conf->bssid);
>
> cur_conf->beacon_interval = bss_conf->beacon_int;
> + if (WARN_ON_ONCE(cur_conf->beacon_interval == 0))
> + cur_conf->beacon_interval = 100;
> cur_conf->dtim_period = bss_conf->dtim_period;
> cur_conf->listen_interval = 1;
> cur_conf->dtim_count = 1;
> diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
> index b97217d..79c20c7 100644
> --- a/drivers/net/wireless/ath/ath9k/recv.c
> +++ b/drivers/net/wireless/ath/ath9k/recv.c
> @@ -538,7 +538,8 @@ static void ath_rx_ps_beacon(struct ath_softc *sc, struct sk_buff *skb)
> sc->ps_flags &= ~PS_BEACON_SYNC;
> ath_dbg(common, PS,
> "Reconfigure beacon timers based on synchronized timestamp\n");
> - ath9k_set_beacon(sc);
> + if (!(WARN_ON_ONCE(sc->cur_beacon_conf.beacon_interval == 0)))
> + ath9k_set_beacon(sc);
> }
>
> if (ath_beacon_dtim_pending_cab(skb)) {
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ath9k: Prevent divide by zero kernel crash.
2014-04-17 7:47 ` Felix Fietkau
@ 2014-04-22 17:53 ` John W. Linville
2014-04-22 18:03 ` Ben Greear
2014-04-30 18:47 ` Ben Greear
1 sibling, 1 reply; 5+ messages in thread
From: John W. Linville @ 2014-04-22 17:53 UTC (permalink / raw)
To: Felix Fietkau; +Cc: greearb, ath9k-devel, linux-wireless
Please consider Felix's comments while refactoring this for the
current code structure...
John
On Thu, Apr 17, 2014 at 09:47:26AM +0200, Felix Fietkau wrote:
> On 2014-04-17 02:40, greearb@candelatech.com wrote:
> > From: Ben Greear <greearb@candelatech.com>
> >
> > Make sure we cannot ever assign beacon interval to zero.
> >
> > Signed-off-by: Ben Greear <greearb@candelatech.com>
> > ---
> > drivers/net/wireless/ath/ath9k/beacon.c | 4 ++++
> > drivers/net/wireless/ath/ath9k/recv.c | 3 ++-
> > 2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
> > index 2e8bba0..5391f01 100644
> > --- a/drivers/net/wireless/ath/ath9k/beacon.c
> > +++ b/drivers/net/wireless/ath/ath9k/beacon.c
> > @@ -443,6 +443,8 @@ static u32 ath9k_mod_tsf64_tu(u64 tsf, u32 div_tu)
> > {
> > u32 tsf_mod, tsf_hi, tsf_lo, mod_hi, mod_lo;
> >
> > + if (WARN_ON_ONCE(div_tu == 0))
> > + div_tu = 100;
> > tsf_mod = tsf & (BIT(10) - 1);
> > tsf_hi = tsf >> 32;
> > tsf_lo = ((u32) tsf) >> 10;
> Why add this warning here if you already have the additions below? We
> don't need multiple layers of defensive checks for the same thing.
>
> > @@ -667,6 +669,8 @@ static void ath9k_cache_beacon_config(struct ath_softc *sc,
> > "Caching beacon data for BSS: %pM\n", bss_conf->bssid);
> >
> > cur_conf->beacon_interval = bss_conf->beacon_int;
> > + if (WARN_ON_ONCE(cur_conf->beacon_interval == 0))
> > + cur_conf->beacon_interval = 100;
> > cur_conf->dtim_period = bss_conf->dtim_period;
> > cur_conf->listen_interval = 1;
> > cur_conf->dtim_count = 1;
> > diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
> > index b97217d..79c20c7 100644
> > --- a/drivers/net/wireless/ath/ath9k/recv.c
> > +++ b/drivers/net/wireless/ath/ath9k/recv.c
> > @@ -538,7 +538,8 @@ static void ath_rx_ps_beacon(struct ath_softc *sc, struct sk_buff *skb)
> > sc->ps_flags &= ~PS_BEACON_SYNC;
> > ath_dbg(common, PS,
> > "Reconfigure beacon timers based on synchronized timestamp\n");
> > - ath9k_set_beacon(sc);
> > + if (!(WARN_ON_ONCE(sc->cur_beacon_conf.beacon_interval == 0)))
> > + ath9k_set_beacon(sc);
> > }
> >
> > if (ath_beacon_dtim_pending_cab(skb)) {
> >
>
> --
> 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.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ath9k: Prevent divide by zero kernel crash.
2014-04-22 17:53 ` John W. Linville
@ 2014-04-22 18:03 ` Ben Greear
0 siblings, 0 replies; 5+ messages in thread
From: Ben Greear @ 2014-04-22 18:03 UTC (permalink / raw)
To: John W. Linville; +Cc: Felix Fietkau, ath9k-devel, linux-wireless
On 04/22/2014 10:53 AM, John W. Linville wrote:
> Please consider Felix's comments while refactoring this for the
> current code structure...
I'm low on time, but will resend a patch when I get a chance.
If someone else wants to send in the updated patch that is fine by me.
Thanks,
Ben
>
> John
>
> On Thu, Apr 17, 2014 at 09:47:26AM +0200, Felix Fietkau wrote:
>> On 2014-04-17 02:40, greearb@candelatech.com wrote:
>>> From: Ben Greear <greearb@candelatech.com>
>>>
>>> Make sure we cannot ever assign beacon interval to zero.
>>>
>>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>>> ---
>>> drivers/net/wireless/ath/ath9k/beacon.c | 4 ++++
>>> drivers/net/wireless/ath/ath9k/recv.c | 3 ++-
>>> 2 files changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
>>> index 2e8bba0..5391f01 100644
>>> --- a/drivers/net/wireless/ath/ath9k/beacon.c
>>> +++ b/drivers/net/wireless/ath/ath9k/beacon.c
>>> @@ -443,6 +443,8 @@ static u32 ath9k_mod_tsf64_tu(u64 tsf, u32 div_tu)
>>> {
>>> u32 tsf_mod, tsf_hi, tsf_lo, mod_hi, mod_lo;
>>>
>>> + if (WARN_ON_ONCE(div_tu == 0))
>>> + div_tu = 100;
>>> tsf_mod = tsf & (BIT(10) - 1);
>>> tsf_hi = tsf >> 32;
>>> tsf_lo = ((u32) tsf) >> 10;
>> Why add this warning here if you already have the additions below? We
>> don't need multiple layers of defensive checks for the same thing.
>>
>>> @@ -667,6 +669,8 @@ static void ath9k_cache_beacon_config(struct ath_softc *sc,
>>> "Caching beacon data for BSS: %pM\n", bss_conf->bssid);
>>>
>>> cur_conf->beacon_interval = bss_conf->beacon_int;
>>> + if (WARN_ON_ONCE(cur_conf->beacon_interval == 0))
>>> + cur_conf->beacon_interval = 100;
>>> cur_conf->dtim_period = bss_conf->dtim_period;
>>> cur_conf->listen_interval = 1;
>>> cur_conf->dtim_count = 1;
>>> diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
>>> index b97217d..79c20c7 100644
>>> --- a/drivers/net/wireless/ath/ath9k/recv.c
>>> +++ b/drivers/net/wireless/ath/ath9k/recv.c
>>> @@ -538,7 +538,8 @@ static void ath_rx_ps_beacon(struct ath_softc *sc, struct sk_buff *skb)
>>> sc->ps_flags &= ~PS_BEACON_SYNC;
>>> ath_dbg(common, PS,
>>> "Reconfigure beacon timers based on synchronized timestamp\n");
>>> - ath9k_set_beacon(sc);
>>> + if (!(WARN_ON_ONCE(sc->cur_beacon_conf.beacon_interval == 0)))
>>> + ath9k_set_beacon(sc);
>>> }
>>>
>>> if (ath_beacon_dtim_pending_cab(skb)) {
>>>
>>
>> --
>> 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
>>
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ath9k: Prevent divide by zero kernel crash.
2014-04-17 7:47 ` Felix Fietkau
2014-04-22 17:53 ` John W. Linville
@ 2014-04-30 18:47 ` Ben Greear
1 sibling, 0 replies; 5+ messages in thread
From: Ben Greear @ 2014-04-30 18:47 UTC (permalink / raw)
To: Felix Fietkau; +Cc: ath9k-devel, linux-wireless
On 04/17/2014 12:47 AM, Felix Fietkau wrote:
> On 2014-04-17 02:40, greearb@candelatech.com wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> Make sure we cannot ever assign beacon interval to zero.
>>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>> ---
>> drivers/net/wireless/ath/ath9k/beacon.c | 4 ++++
>> drivers/net/wireless/ath/ath9k/recv.c | 3 ++-
>> 2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
>> index 2e8bba0..5391f01 100644
>> --- a/drivers/net/wireless/ath/ath9k/beacon.c
>> +++ b/drivers/net/wireless/ath/ath9k/beacon.c
>> @@ -443,6 +443,8 @@ static u32 ath9k_mod_tsf64_tu(u64 tsf, u32 div_tu)
>> {
>> u32 tsf_mod, tsf_hi, tsf_lo, mod_hi, mod_lo;
>>
>> + if (WARN_ON_ONCE(div_tu == 0))
>> + div_tu = 100;
>> tsf_mod = tsf & (BIT(10) - 1);
>> tsf_hi = tsf >> 32;
>> tsf_lo = ((u32) tsf) >> 10;
> Why add this warning here if you already have the additions below? We
> don't need multiple layers of defensive checks for the same thing.
I am not sure I can find all cases that can send bad data to this
call, and in other places, it seems having an invalid beacon interval
might mess up other calculations, so better to check and set it to
a better value there as well.
So, I'd prefer to leave all three warnings in, and if we ever see
the one hit down in mod_tsf64_tu, then probably more protection
is needed elsewhere.
Or, just treat this patch as bug report and maybe someone will
fix it better...
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-04-30 18:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-17 0:40 [PATCH] ath9k: Prevent divide by zero kernel crash greearb
2014-04-17 7:47 ` Felix Fietkau
2014-04-22 17:53 ` John W. Linville
2014-04-22 18:03 ` Ben Greear
2014-04-30 18:47 ` Ben Greear
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).