* [PATCH] mac80211: fix rx->key NULL dereference during mic failure
@ 2011-06-18 18:45 Arik Nemtsov
2011-06-19 7:33 ` Johannes Berg
2011-06-19 20:04 ` Kalle Valo
0 siblings, 2 replies; 8+ messages in thread
From: Arik Nemtsov @ 2011-06-18 18:45 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg, Arik Nemtsov
Sometimes when reporting a MIC failure rx->key may be unset. This
code path is hit when receiving a packet meant for a multicast
address, and decryption is performed in HW.
Fortunately, the failing key_idx is not used for anything up to
(and including) usermode, so we allow ourselves to set a bogus one
when a key cannot be retrieved.
Signed-off-by: Arik Nemtsov <arik@wizery.com>
---
net/mac80211/wpa.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index 9dc3b5f..c6dafc1 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -154,7 +154,8 @@ update_iv:
return RX_CONTINUE;
mic_fail:
- mac80211_ev_michael_mic_failure(rx->sdata, rx->key->conf.keyidx,
+ mac80211_ev_michael_mic_failure(rx->sdata,
+ rx->key ? rx->key->conf.keyidx : -1,
(void *) skb->data, NULL, GFP_ATOMIC);
return RX_DROP_UNUSABLE;
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] mac80211: fix rx->key NULL dereference during mic failure
2011-06-18 18:45 [PATCH] mac80211: fix rx->key NULL dereference during mic failure Arik Nemtsov
@ 2011-06-19 7:33 ` Johannes Berg
2011-06-19 20:04 ` Kalle Valo
1 sibling, 0 replies; 8+ messages in thread
From: Johannes Berg @ 2011-06-19 7:33 UTC (permalink / raw)
To: Arik Nemtsov; +Cc: linux-wireless
On Sat, 2011-06-18 at 21:45 +0300, Arik Nemtsov wrote:
> Sometimes when reporting a MIC failure rx->key may be unset. This
> code path is hit when receiving a packet meant for a multicast
> address, and decryption is performed in HW.
>
> Fortunately, the failing key_idx is not used for anything up to
> (and including) usermode, so we allow ourselves to set a bogus one
> when a key cannot be retrieved.
I guess we don't have a choice, but for the record I don't really like
lying. I'd rather also adjust nl80211 to not have the attribute in this
case, rather than an attribute with a bogus value.
johannes
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mac80211: fix rx->key NULL dereference during mic failure
2011-06-18 18:45 [PATCH] mac80211: fix rx->key NULL dereference during mic failure Arik Nemtsov
2011-06-19 7:33 ` Johannes Berg
@ 2011-06-19 20:04 ` Kalle Valo
2011-06-22 20:04 ` John W. Linville
1 sibling, 1 reply; 8+ messages in thread
From: Kalle Valo @ 2011-06-19 20:04 UTC (permalink / raw)
To: Arik Nemtsov; +Cc: linux-wireless, Johannes Berg
Arik Nemtsov <arik@wizery.com> writes:
> Sometimes when reporting a MIC failure rx->key may be unset. This
> code path is hit when receiving a packet meant for a multicast
> address, and decryption is performed in HW.
>
> Fortunately, the failing key_idx is not used for anything up to
> (and including) usermode, so we allow ourselves to set a bogus one
> when a key cannot be retrieved.
[...]
> - mac80211_ev_michael_mic_failure(rx->sdata, rx->key->conf.keyidx,
> + mac80211_ev_michael_mic_failure(rx->sdata,
> + rx->key ? rx->key->conf.keyidx : -1,
> (void *) skb->data, NULL, GFP_ATOMIC);
In special cases like this a comment in the code would be nice.
--
Kalle Valo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mac80211: fix rx->key NULL dereference during mic failure
2011-06-19 20:04 ` Kalle Valo
@ 2011-06-22 20:04 ` John W. Linville
2011-06-22 20:22 ` Arik Nemtsov
0 siblings, 1 reply; 8+ messages in thread
From: John W. Linville @ 2011-06-22 20:04 UTC (permalink / raw)
To: Kalle Valo; +Cc: Arik Nemtsov, linux-wireless, Johannes Berg
On Sun, Jun 19, 2011 at 11:04:12PM +0300, Kalle Valo wrote:
> Arik Nemtsov <arik@wizery.com> writes:
>
> > Sometimes when reporting a MIC failure rx->key may be unset. This
> > code path is hit when receiving a packet meant for a multicast
> > address, and decryption is performed in HW.
> >
> > Fortunately, the failing key_idx is not used for anything up to
> > (and including) usermode, so we allow ourselves to set a bogus one
> > when a key cannot be retrieved.
>
> [...]
>
> > - mac80211_ev_michael_mic_failure(rx->sdata, rx->key->conf.keyidx,
> > + mac80211_ev_michael_mic_failure(rx->sdata,
> > + rx->key ? rx->key->conf.keyidx : -1,
> > (void *) skb->data, NULL, GFP_ATOMIC);
>
> In special cases like this a comment in the code would be nice.
Repost with a comment, as Kalle requested?
--
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] 8+ messages in thread
* Re: [PATCH] mac80211: fix rx->key NULL dereference during mic failure
2011-06-22 20:04 ` John W. Linville
@ 2011-06-22 20:22 ` Arik Nemtsov
0 siblings, 0 replies; 8+ messages in thread
From: Arik Nemtsov @ 2011-06-22 20:22 UTC (permalink / raw)
To: John W. Linville; +Cc: Kalle Valo, linux-wireless, Johannes Berg
> >
> > > - mac80211_ev_michael_mic_failure(rx->sdata, rx->key->conf.keyidx,
> > > + mac80211_ev_michael_mic_failure(rx->sdata,
> > > + rx->key ? rx->key->conf.keyidx : -1,
> > > (void *) skb->data, NULL, GFP_ATOMIC);
> >
> > In special cases like this a comment in the code would be nice.
>
> Repost with a comment, as Kalle requested?
>
Sure thing.
Arik
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mac80211: fix rx->key NULL dereference during mic failure
2011-06-22 20:52 ` Johannes Berg
@ 2011-06-22 20:59 ` Arik Nemtsov
0 siblings, 0 replies; 8+ messages in thread
From: Arik Nemtsov @ 2011-06-22 20:59 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, John W. Linville
On Wed, Jun 22, 2011 at 23:52, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Wed, 2011-06-22 at 23:41 +0300, Arik Nemtsov wrote:
>> Sometimes when reporting a MIC failure rx->key may be unset. This
>> code path is hit when receiving a packet meant for a multicast
>> address, and decryption is performed in HW.
>>
>> Fortunately, the failing key_idx is not used for anything up to
>> (and including) usermode, so we allow ourselves to drop it on the
>> way up when a key cannot be retrieved.
>>
>> Signed-off-by: Arik Nemtsov <arik@wizery.com>
>
> It'd be nice to also update the docs for the cfg80211 function to
> document the -1 special case, but other than that:
Sure why not? :)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mac80211: fix rx->key NULL dereference during mic failure
2011-06-22 20:41 Arik Nemtsov
@ 2011-06-22 20:52 ` Johannes Berg
2011-06-22 20:59 ` Arik Nemtsov
0 siblings, 1 reply; 8+ messages in thread
From: Johannes Berg @ 2011-06-22 20:52 UTC (permalink / raw)
To: Arik Nemtsov; +Cc: linux-wireless, John W. Linville
On Wed, 2011-06-22 at 23:41 +0300, Arik Nemtsov wrote:
> Sometimes when reporting a MIC failure rx->key may be unset. This
> code path is hit when receiving a packet meant for a multicast
> address, and decryption is performed in HW.
>
> Fortunately, the failing key_idx is not used for anything up to
> (and including) usermode, so we allow ourselves to drop it on the
> way up when a key cannot be retrieved.
>
> Signed-off-by: Arik Nemtsov <arik@wizery.com>
It'd be nice to also update the docs for the cfg80211 function to
document the -1 special case, but other than that:
Reviewed-by: Johannes Berg <johannes@sipsolutions.net>
johannes
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] mac80211: fix rx->key NULL dereference during mic failure
@ 2011-06-22 20:41 Arik Nemtsov
2011-06-22 20:52 ` Johannes Berg
0 siblings, 1 reply; 8+ messages in thread
From: Arik Nemtsov @ 2011-06-22 20:41 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg, John W. Linville, Arik Nemtsov
Sometimes when reporting a MIC failure rx->key may be unset. This
code path is hit when receiving a packet meant for a multicast
address, and decryption is performed in HW.
Fortunately, the failing key_idx is not used for anything up to
(and including) usermode, so we allow ourselves to drop it on the
way up when a key cannot be retrieved.
Signed-off-by: Arik Nemtsov <arik@wizery.com>
---
We now discard invalid key indices on the way up, as Johannes
suggested privately.
net/mac80211/wpa.c | 8 +++++++-
net/wireless/nl80211.c | 3 ++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index 9dc3b5f..d91c1a2 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -154,7 +154,13 @@ update_iv:
return RX_CONTINUE;
mic_fail:
- mac80211_ev_michael_mic_failure(rx->sdata, rx->key->conf.keyidx,
+ /*
+ * In some cases the key can be unset - e.g. a multicast packet, in
+ * a driver that supports HW encryption. Send up the key idx only if
+ * the key is set.
+ */
+ mac80211_ev_michael_mic_failure(rx->sdata,
+ rx->key ? rx->key->conf.keyidx : -1,
(void *) skb->data, NULL, GFP_ATOMIC);
return RX_DROP_UNUSABLE;
}
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index ec83f41..785549d 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -6464,7 +6464,8 @@ void nl80211_michael_mic_failure(struct cfg80211_registered_device *rdev,
if (addr)
NLA_PUT(msg, NL80211_ATTR_MAC, ETH_ALEN, addr);
NLA_PUT_U32(msg, NL80211_ATTR_KEY_TYPE, key_type);
- NLA_PUT_U8(msg, NL80211_ATTR_KEY_IDX, key_id);
+ if (key_id != -1)
+ NLA_PUT_U8(msg, NL80211_ATTR_KEY_IDX, key_id);
if (tsc)
NLA_PUT(msg, NL80211_ATTR_KEY_SEQ, 6, tsc);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-06-22 20:59 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-18 18:45 [PATCH] mac80211: fix rx->key NULL dereference during mic failure Arik Nemtsov
2011-06-19 7:33 ` Johannes Berg
2011-06-19 20:04 ` Kalle Valo
2011-06-22 20:04 ` John W. Linville
2011-06-22 20:22 ` Arik Nemtsov
2011-06-22 20:41 Arik Nemtsov
2011-06-22 20:52 ` Johannes Berg
2011-06-22 20:59 ` Arik Nemtsov
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.