Return-path: Received: from mail-bw0-f222.google.com ([209.85.218.222]:53828 "EHLO mail-bw0-f222.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750974AbZELTFv convert rfc822-to-8bit (ORCPT ); Tue, 12 May 2009 15:05:51 -0400 Received: by bwz22 with SMTP id 22so167455bwz.37 for ; Tue, 12 May 2009 12:05:51 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1242125077.4331.0.camel@johannes.local> References: <1242125077.4331.0.camel@johannes.local> Date: Tue, 12 May 2009 20:05:51 +0100 Message-ID: <3ace41890905121205v76d6b61cg94d0bbe55c04da7e@mail.gmail.com> Subject: Re: [PATCH] cfg80211: fix a couple of bugs with key ioctls From: Hin-Tak Leung To: Johannes Berg Cc: John Linville , linux-wireless Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-wireless-owner@vger.kernel.org List-ID: On Tue, May 12, 2009 at 11:44 AM, Johannes Berg 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 > --- > =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