Re: [PATCH] cfg80211: fix a couple of bugs with key ioctls

Hin-Tak Leung
2009-05-12T19:05:51Z

On Tue, May 12, 2009 at 11:44 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> There are a few small bugs/oversights in the key handling
> code I wrote:
> =A0* SIOCSIWENCODE should default to setting key 0 if no
> =A0 default key is set already and no index is given,
> =A0* key removal should not require key material,
> =A0* SIOCSIWENCODEEXT should default to changing the default
> =A0 management key if no index is given.
>
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> ---
> =A0net/wireless/wext-compat.c | =A0 14 +++++---------
> =A01 file changed, 5 insertions(+), 9 deletions(-)
>
> --- wireless-testing.orig/net/wireless/wext-compat.c =A0 =A02009-05-1=
2 12:38:41.000000000 +0200
> +++ wireless-testing/net/wireless/wext-compat.c 2009-05-12 12:38:59.0=
00000000 +0200
> @@ -557,7 +557,7 @@ int cfg80211_wext_siwencode(struct net_d
> =A0 =A0 =A0 =A0if (idx =3D=3D 0) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0idx =3D wdev->wext.default_key;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (idx < 0)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EINVAL;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 idx =3D 0;
> =A0 =A0 =A0 =A0} else if (idx < 1 || idx > 4)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -EINVAL;
> =A0 =A0 =A0 =A0else
> @@ -580,7 +580,7 @@ int cfg80211_wext_siwencode(struct net_d
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0params.cipher =3D WLAN_CIPHER_SUITE_WE=
P40;
> =A0 =A0 =A0 =A0else if (erq->length =3D=3D 13)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0params.cipher =3D WLAN_CIPHER_SUITE_WE=
P104;
> - =A0 =A0 =A0 else
> + =A0 =A0 =A0 else if (!remove)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -EINVAL;
>
> =A0 =A0 =A0 =A0return cfg80211_set_encryption(rdev, dev, NULL, remove=
,
> @@ -640,13 +640,9 @@ int cfg80211_wext_siwencodeext(struct ne
> =A0 =A0 =A0 =A0idx =3D erq->flags & IW_ENCODE_INDEX;
> =A0 =A0 =A0 =A0if (cipher =3D=3D WLAN_CIPHER_SUITE_AES_CMAC) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (idx < 4 || idx > 5) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /*
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* XXX: Only wpa_supp=
licant ever used this
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* =A0 =A0 =A0can we =
still change the ABI a little
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* =A0 =A0 =A0so we d=
o not need to keep track of
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* =A0 =A0 =A0the def=
ault key?
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EINVAL;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 idx =3D wdev->wext.defa=
ult_mgmt_key;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (idx < 0)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return =
-EINVAL;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0idx--;
> =A0 =A0 =A0 =A0} else {
>
>
>

I tried this, NM/wpa_suplicany still doesn't work, but if I shut those
down and do it the old fashioned iwconfig way, I can get
connectivity... the last chunk looks a bit funny - '(idx < 4 || idx >
5)' is just '(idx !=3D 4)' so why it isn't written simplier as such?
also the idx < 0 maybe <1 ?

Stiill needs some work...
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html