All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [RFT] iwlegacy: don't mess up the SCD when removing a key
  2012-07-01 13:07 [RFT] iwlegacy: don't mess up the SCD when removing a key Emmanuel Grumbach
@ 2012-07-01 12:01 ` Paul Bolle
  2012-07-01 13:29   ` Grumbach, Emmanuel
  2012-07-02  8:26 ` Stanislaw Gruszka
  1 sibling, 1 reply; 6+ messages in thread
From: Paul Bolle @ 2012-07-01 12:01 UTC (permalink / raw)
  To: Emmanuel Grumbach; +Cc: linux-wireless, Stanislaw Gruszka

Emmanuel,

On Sun, 2012-07-01 at 16:07 +0300, Emmanuel Grumbach wrote:
> Paul, can you please test ?

Sure, and thanks for porting that patch.

> If it solve the issues for you, I will send as a patch and Cc stable
> Totally not tested

Note that it might take some days, maybe even a week, before I can be
confident that the patch works. Perhaps I should give suspending and
resuming in a loop a try, that should speed up testing.


Paul Bolle


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFT] iwlegacy: don't mess up the SCD when removing a key
@ 2012-07-01 13:07 Emmanuel Grumbach
  2012-07-01 12:01 ` Paul Bolle
  2012-07-02  8:26 ` Stanislaw Gruszka
  0 siblings, 2 replies; 6+ messages in thread
From: Emmanuel Grumbach @ 2012-07-01 13:07 UTC (permalink / raw)
  To: linux-wireless; +Cc: Stanislaw Gruszka, Emmanuel Grumbach, Paul Bolle

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.

Cc: Paul Bolle <pebolle@tiscali.nl>
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
---

 drivers/net/wireless/iwlegacy/4965-mac.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/iwlegacy/4965-mac.c b/drivers/net/wireless/iwlegacy/4965-mac.c
index d24eaf8..9981f09 100644
--- a/drivers/net/wireless/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/iwlegacy/4965-mac.c
@@ -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: [RFT] iwlegacy: don't mess up the SCD when removing a key
  2012-07-01 12:01 ` Paul Bolle
@ 2012-07-01 13:29   ` Grumbach, Emmanuel
  2012-07-01 13:36     ` Paul Bolle
  0 siblings, 1 reply; 6+ messages in thread
From: Grumbach, Emmanuel @ 2012-07-01 13:29 UTC (permalink / raw)
  To: Paul Bolle; +Cc: linux-wireless, Stanislaw Gruszka

PiANCj4gRW1tYW51ZWwsDQo+IA0KPiBPbiBTdW4sIDIwMTItMDctMDEgYXQgMTY6MDcgKzAzMDAs
IEVtbWFudWVsIEdydW1iYWNoIHdyb3RlOg0KPiA+IFBhdWwsIGNhbiB5b3UgcGxlYXNlIHRlc3Qg
Pw0KPiANCj4gU3VyZSwgYW5kIHRoYW5rcyBmb3IgcG9ydGluZyB0aGF0IHBhdGNoLg0KPiANCj4g
PiBJZiBpdCBzb2x2ZSB0aGUgaXNzdWVzIGZvciB5b3UsIEkgd2lsbCBzZW5kIGFzIGEgcGF0Y2gg
YW5kIENjIHN0YWJsZQ0KPiA+IFRvdGFsbHkgbm90IHRlc3RlZA0KPiANCj4gTm90ZSB0aGF0IGl0
IG1pZ2h0IHRha2Ugc29tZSBkYXlzLCBtYXliZSBldmVuIGEgd2VlaywgYmVmb3JlIEkgY2FuIGJl
DQo+IGNvbmZpZGVudCB0aGF0IHRoZSBwYXRjaCB3b3Jrcy4gUGVyaGFwcyBJIHNob3VsZCBnaXZl
IHN1c3BlbmRpbmcgYW5kDQo+IHJlc3VtaW5nIGluIGEgbG9vcCBhIHRyeSwgdGhhdCBzaG91bGQg
c3BlZWQgdXAgdGVzdGluZy4NCj4gDQoNCllvdSBjYW4gYWxzbyB0cnkgdG8gc2ltcGx5IGFzc29j
aWF0ZSAvIGRpc2Fzc29jaWF0ZSBpbiBhIGxvb3Agd2l0aCBhbmQgd2l0aG91dCBteSBwYXRjaC4g
VGhpcyB3YXMgdGhlIHNjZW5hcmlvIHRoYXQgd2FzIGZpeGVkIGJ5IG15IHBhdGNoIGluIGl3bHdp
ZmkuDQotLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0KSW50ZWwgSXNyYWVsICg3NCkgTGltaXRlZAoKVGhpcyBlLW1haWwg
YW5kIGFueSBhdHRhY2htZW50cyBtYXkgY29udGFpbiBjb25maWRlbnRpYWwgbWF0ZXJpYWwgZm9y
CnRoZSBzb2xlIHVzZSBvZiB0aGUgaW50ZW5kZWQgcmVjaXBpZW50KHMpLiBBbnkgcmV2aWV3IG9y
IGRpc3RyaWJ1dGlvbgpieSBvdGhlcnMgaXMgc3RyaWN0bHkgcHJvaGliaXRlZC4gSWYgeW91IGFy
ZSBub3QgdGhlIGludGVuZGVkCnJlY2lwaWVudCwgcGxlYXNlIGNvbnRhY3QgdGhlIHNlbmRlciBh
bmQgZGVsZXRlIGFsbCBjb3BpZXMuCg==


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [RFT] iwlegacy: don't mess up the SCD when removing a key
  2012-07-01 13:29   ` Grumbach, Emmanuel
@ 2012-07-01 13:36     ` Paul Bolle
  2012-07-01 13:47       ` Grumbach, Emmanuel
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Bolle @ 2012-07-01 13:36 UTC (permalink / raw)
  To: Grumbach, Emmanuel; +Cc: linux-wireless, Stanislaw Gruszka

Emmanuel,

On Sun, 2012-07-01 at 13:29 +0000, Grumbach, Emmanuel wrote:
> You can also try to simply associate / disassociate in a loop with and
> without my patch. This was the scenario that was fixed by my patch in
> iwlwifi.

I'm actually unfamiliar with associating and disassociating. How can I
do that in a loop?


Paul Bolle


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [RFT] iwlegacy: don't mess up the SCD when removing a key
  2012-07-01 13:36     ` Paul Bolle
@ 2012-07-01 13:47       ` Grumbach, Emmanuel
  0 siblings, 0 replies; 6+ messages in thread
From: Grumbach, Emmanuel @ 2012-07-01 13:47 UTC (permalink / raw)
  To: Paul Bolle; +Cc: linux-wireless, Stanislaw Gruszka

DQo+IEVtbWFudWVsLA0KPiANCj4gT24gU3VuLCAyMDEyLTA3LTAxIGF0IDEzOjI5ICswMDAwLCBH
cnVtYmFjaCwgRW1tYW51ZWwgd3JvdGU6DQo+ID4gWW91IGNhbiBhbHNvIHRyeSB0byBzaW1wbHkg
YXNzb2NpYXRlIC8gZGlzYXNzb2NpYXRlIGluIGEgbG9vcCB3aXRoIGFuZA0KPiA+IHdpdGhvdXQg
bXkgcGF0Y2guIFRoaXMgd2FzIHRoZSBzY2VuYXJpbyB0aGF0IHdhcyBmaXhlZCBieSBteSBwYXRj
aCBpbg0KPiA+IGl3bHdpZmkuDQo+IA0KPiBJJ20gYWN0dWFsbHkgdW5mYW1pbGlhciB3aXRoIGFz
c29jaWF0aW5nIGFuZCBkaXNhc3NvY2lhdGluZy4gSG93IGNhbiBJIGRvIHRoYXQgaW4NCj4gYSBs
b29wPw0KPiANCg0KSSBndWVzcyB5b3UgaGF2ZSBhIHNlY3VyZWQgcHJvZmlsZSByaWdodD8gSWYg
eW91IGRvbid0LCBteSBwYXRjaCBpcyBpcnJlbGV2YW50Lg0KSnVzdCBjbGljayBvbiBkaXNjb25u
ZWN0IGluIHlvdXIgR1VJLCBJIGd1ZXNzIGl0IHNob3VsZCBtYWtlIGl0Lg0KDQpZb3UgY2FuIGFs
c28gZG86DQoNCndwYV9jbGkgZGlzY29ubmVjdA0Kd3BhX2NsaSByZWNvbm5lY3QNCg0Kb3Igc29t
ZXRoaW5nIGxpa2UgdGhhdC4NCi0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQpJbnRlbCBJc3JhZWwgKDc0KSBMaW1pdGVk
CgpUaGlzIGUtbWFpbCBhbmQgYW55IGF0dGFjaG1lbnRzIG1heSBjb250YWluIGNvbmZpZGVudGlh
bCBtYXRlcmlhbCBmb3IKdGhlIHNvbGUgdXNlIG9mIHRoZSBpbnRlbmRlZCByZWNpcGllbnQocyku
IEFueSByZXZpZXcgb3IgZGlzdHJpYnV0aW9uCmJ5IG90aGVycyBpcyBzdHJpY3RseSBwcm9oaWJp
dGVkLiBJZiB5b3UgYXJlIG5vdCB0aGUgaW50ZW5kZWQKcmVjaXBpZW50LCBwbGVhc2UgY29udGFj
dCB0aGUgc2VuZGVyIGFuZCBkZWxldGUgYWxsIGNvcGllcy4K


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFT] iwlegacy: don't mess up the SCD when removing a key
  2012-07-01 13:07 [RFT] iwlegacy: don't mess up the SCD when removing a key Emmanuel Grumbach
  2012-07-01 12:01 ` Paul Bolle
@ 2012-07-02  8:26 ` Stanislaw Gruszka
  1 sibling, 0 replies; 6+ messages in thread
From: Stanislaw Gruszka @ 2012-07-02  8:26 UTC (permalink / raw)
  To: Emmanuel Grumbach; +Cc: linux-wireless, Paul Bolle

On Sun, Jul 01, 2012 at 04:07:03PM +0300, Emmanuel Grumbach wrote:
> diff --git a/drivers/net/wireless/iwlegacy/4965-mac.c b/drivers/net/wireless/iwlegacy/4965-mac.c
> index d24eaf8..9981f09 100644
> --- a/drivers/net/wireless/iwlegacy/4965-mac.c
> +++ b/drivers/net/wireless/iwlegacy/4965-mac.c
> @@ -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;

Thanks for the patch. Just small issue with it. We use key_offset to
check if the key is invalid on the below code:

        if (il->stations[sta_id].sta.key.key_offset == WEP_INVALID_OFFSET) {
                IL_WARN("Removing wrong key %d 0x%x\n", keyconf->keyidx,
                        key_flags);
                spin_unlock_irqrestore(&il->sta_lock, flags);
                return 0;
        }

Since you changed the key_offset, above check should be changed as well,
probably using STA_KEY_FLG_INVALID flag.

Stanislaw 
 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-07-02  8:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-01 13:07 [RFT] iwlegacy: don't mess up the SCD when removing a key Emmanuel Grumbach
2012-07-01 12:01 ` Paul Bolle
2012-07-01 13:29   ` Grumbach, Emmanuel
2012-07-01 13:36     ` Paul Bolle
2012-07-01 13:47       ` Grumbach, Emmanuel
2012-07-02  8:26 ` Stanislaw Gruszka

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.