* [PATCH] iwlegacy: don't mess up the SCD when removing a key
@ 2012-07-02 11:31 Emmanuel Grumbach
2012-07-04 11:22 ` Stanislaw Gruszka
0 siblings, 1 reply; 6+ messages in thread
From: Emmanuel Grumbach @ 2012-07-02 11:31 UTC (permalink / raw)
To: linux-wireless; +Cc: Stanislaw Gruszka, Emmanuel Grumbach
When we remove a key, we put a key index which was supposed
to tell the fw that we are actually removing the key. But
instead the fw took that index as a valid index and messed
up the SRAM of the device.
This memory corruption on the device mangled the data of
the SCD. The impact on the user is that SCD queue 2 got
stuck after having removed keys.
Change-Id: I721f71c1a3a0af6058abe1975cebd9b613c7ff2b
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
Paul, can you please test ?
If it solve the issues for you, I will send as a patch and Cc stable Totally not tested
v2: first hunk of this patch thanks to Stanislaw's review
---
drivers/net/wireless/iwlegacy/4965-mac.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/iwlegacy/4965-mac.c b/drivers/net/wireless/iwlegacy/4965-mac.c
index d24eaf8..34f61a0 100644
--- a/drivers/net/wireless/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/iwlegacy/4965-mac.c
@@ -3405,7 +3405,7 @@ il4965_remove_dynamic_key(struct il_priv *il,
return 0;
}
- if (il->stations[sta_id].sta.key.key_offset == WEP_INVALID_OFFSET) {
+ if (il->stations[sta_id].sta.key.key_flags & STA_KEY_FLG_INVALID) {
IL_WARN("Removing wrong key %d 0x%x\n", keyconf->keyidx,
key_flags);
spin_unlock_irqrestore(&il->sta_lock, flags);
@@ -3420,7 +3420,7 @@ il4965_remove_dynamic_key(struct il_priv *il,
memset(&il->stations[sta_id].sta.key, 0, sizeof(struct il4965_keyinfo));
il->stations[sta_id].sta.key.key_flags =
STA_KEY_FLG_NO_ENC | STA_KEY_FLG_INVALID;
- il->stations[sta_id].sta.key.key_offset = WEP_INVALID_OFFSET;
+ il->stations[sta_id].sta.key.key_offset = keyconf->hw_key_idx;
il->stations[sta_id].sta.sta.modify_mask = STA_MODIFY_KEY_MASK;
il->stations[sta_id].sta.mode = STA_CONTROL_MODIFY_MSK;
--
1.7.1
---------------------------------------------------------------------
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] iwlegacy: don't mess up the SCD when removing a key
2012-07-02 11:31 [PATCH] iwlegacy: don't mess up the SCD when removing a key Emmanuel Grumbach
@ 2012-07-04 11:22 ` Stanislaw Gruszka
2012-07-04 11:33 ` Emmanuel Grumbach
0 siblings, 1 reply; 6+ messages in thread
From: Stanislaw Gruszka @ 2012-07-04 11:22 UTC (permalink / raw)
To: Emmanuel Grumbach; +Cc: linux-wireless
On Mon, Jul 02, 2012 at 02:31:21PM +0300, Emmanuel Grumbach wrote:
> When we remove a key, we put a key index which was supposed
> to tell the fw that we are actually removing the key. But
> instead the fw took that index as a valid index and messed
> up the SRAM of the device.
>
> This memory corruption on the device mangled the data of
> the SCD. The impact on the user is that SCD queue 2 got
> stuck after having removed keys.
>
> Change-Id: I721f71c1a3a0af6058abe1975cebd9b613c7ff2b
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> ---
> Paul, can you please test ?
> If it solve the issues for you, I will send as a patch and Cc stable Totally not tested
> v2: first hunk of this patch thanks to Stanislaw's review
I tested that patch and did not found any problem.
Stanislaw
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iwlegacy: don't mess up the SCD when removing a key
2012-07-04 11:22 ` Stanislaw Gruszka
@ 2012-07-04 11:33 ` Emmanuel Grumbach
2012-07-04 11:40 ` Stanislaw Gruszka
0 siblings, 1 reply; 6+ messages in thread
From: Emmanuel Grumbach @ 2012-07-04 11:33 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Emmanuel Grumbach, linux-wireless
> On Mon, Jul 02, 2012 at 02:31:21PM +0300, Emmanuel Grumbach wrote:
> > When we remove a key, we put a key index which was supposed
> > to tell the fw that we are actually removing the key. But
> > instead the fw took that index as a valid index and messed
> > up the SRAM of the device.
> >
> > This memory corruption on the device mangled the data of
> > the SCD. The impact on the user is that SCD queue 2 got
> > stuck after having removed keys.
> >
> > Change-Id: I721f71c1a3a0af6058abe1975cebd9b613c7ff2b
> > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> > ---
> > Paul, can you please test ?
> > If it solve the issues for you, I will send as a patch and Cc stable
> > Totally not tested
> > v2: first hunk of this patch thanks to Stanislaw's review
>
> I tested that patch and did not found any problem.
>
Ok, but doest it solve the bug ?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iwlegacy: don't mess up the SCD when removing a key
2012-07-04 11:33 ` Emmanuel Grumbach
@ 2012-07-04 11:40 ` Stanislaw Gruszka
2012-07-04 11:49 ` Grumbach, Emmanuel
2012-07-04 12:03 ` Paul Bolle
0 siblings, 2 replies; 6+ messages in thread
From: Stanislaw Gruszka @ 2012-07-04 11:40 UTC (permalink / raw)
To: Emmanuel Grumbach; +Cc: Emmanuel Grumbach, linux-wireless, Paul Bolle
On Wed, Jul 04, 2012 at 02:33:48PM +0300, Emmanuel Grumbach wrote:
> > On Mon, Jul 02, 2012 at 02:31:21PM +0300, Emmanuel Grumbach wrote:
> > > When we remove a key, we put a key index which was supposed
> > > to tell the fw that we are actually removing the key. But
> > > instead the fw took that index as a valid index and messed
> > > up the SRAM of the device.
> > >
> > > This memory corruption on the device mangled the data of
> > > the SCD. The impact on the user is that SCD queue 2 got
> > > stuck after having removed keys.
> > >
> > > Change-Id: I721f71c1a3a0af6058abe1975cebd9b613c7ff2b
> > > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> > > ---
> > > Paul, can you please test ?
> > > If it solve the issues for you, I will send as a patch and Cc stable
> > > Totally not tested
> > > v2: first hunk of this patch thanks to Stanislaw's review
> >
> > I tested that patch and did not found any problem.
> >
>
> Ok, but doest it solve the bug ?
CCing Poul.
But even if not, we still probably need that patch, right? Or is
possible that 4965 firmware do not corrupt memory when we provide
wrong key offset to it? Or maybe for 4965 key offset 0xff is something
that is expected and needed to invalidate the key?
Stanislaw
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] iwlegacy: don't mess up the SCD when removing a key
2012-07-04 11:40 ` Stanislaw Gruszka
@ 2012-07-04 11:49 ` Grumbach, Emmanuel
2012-07-04 12:03 ` Paul Bolle
1 sibling, 0 replies; 6+ messages in thread
From: Grumbach, Emmanuel @ 2012-07-04 11:49 UTC (permalink / raw)
To: Stanislaw Gruszka, Emmanuel Grumbach
Cc: linux-wireless@vger.kernel.org, Paul Bolle
>
> On Wed, Jul 04, 2012 at 02:33:48PM +0300, Emmanuel Grumbach wrote:
> > > On Mon, Jul 02, 2012 at 02:31:21PM +0300, Emmanuel Grumbach wrote:
> > > > When we remove a key, we put a key index which was supposed to
> > > > tell the fw that we are actually removing the key. But instead the
> > > > fw took that index as a valid index and messed up the SRAM of the
> > > > device.
> > > >
> > > > This memory corruption on the device mangled the data of the SCD.
> > > > The impact on the user is that SCD queue 2 got stuck after having
> > > > removed keys.
> > > >
> > > > Change-Id: I721f71c1a3a0af6058abe1975cebd9b613c7ff2b
> > > > Signed-off-by: Emmanuel Grumbach
> <emmanuel.grumbach@intel.com>
> > > > ---
> > > > Paul, can you please test ?
> > > > If it solve the issues for you, I will send as a patch and Cc
> > > > stable Totally not tested
> > > > v2: first hunk of this patch thanks to Stanislaw's review
> > >
> > > I tested that patch and did not found any problem.
> > >
> >
> > Ok, but doest it solve the bug ?
>
> CCing Poul.
>
> But even if not, we still probably need that patch, right? Or is possible that
> 4965 firmware do not corrupt memory when we provide wrong key offset to
> it? Or maybe for 4965 key offset 0xff is something that is expected and
> needed to invalidate the key?
>
I guess the patch is needed for 4965. This code in fw is quite old and hasn't changed for a while, so...
---------------------------------------------------------------------
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iwlegacy: don't mess up the SCD when removing a key
2012-07-04 11:40 ` Stanislaw Gruszka
2012-07-04 11:49 ` Grumbach, Emmanuel
@ 2012-07-04 12:03 ` Paul Bolle
1 sibling, 0 replies; 6+ messages in thread
From: Paul Bolle @ 2012-07-04 12:03 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Emmanuel Grumbach, Emmanuel Grumbach, linux-wireless
On Wed, 2012-07-04 at 13:40 +0200, Stanislaw Gruszka wrote:
> On Wed, Jul 04, 2012 at 02:33:48PM +0300, Emmanuel Grumbach wrote:
> > Ok, but doest it solve the bug ?
>
> CCing Poul.
Well I didn't see the queue-2-is-stuck error while testing that patch
for a few days (mostly by disconnecting and reconnecting to my wlan
through clicking a GUI applet).
That being said, since a day or two I'm again running without that patch
and also haven't noticed that error. So perhaps the jury's still out on
this patch ...
Maybe I should again try suspending and resuming in a loop. But that
raises the chance of suspend/resume bugs influencing the results.
Testing bugs like these (in network code, only triggered every now and
then) apparently isn't easy.
> But even if not, we still probably need that patch, right? Or is
> possible that 4965 firmware do not corrupt memory when we provide
> wrong key offset to it? Or maybe for 4965 key offset 0xff is something
> that is expected and needed to invalidate the key?
Paul Bolle
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-04 12:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-02 11:31 [PATCH] iwlegacy: don't mess up the SCD when removing a key Emmanuel Grumbach
2012-07-04 11:22 ` Stanislaw Gruszka
2012-07-04 11:33 ` Emmanuel Grumbach
2012-07-04 11:40 ` Stanislaw Gruszka
2012-07-04 11:49 ` Grumbach, Emmanuel
2012-07-04 12:03 ` Paul Bolle
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.