[PATCH 00/0] iwmc3200wifi: fix WPA connect problem

[PATCH 00/0] iwmc3200wifi: fix WPA connect problem

[Zhu Yi]

Hi,

Below patches make iwmc3200wifi WPA work again with the latest cfg80211
change.

[PATCH 1/5] cfg80211: set_default_key only for WEP
[PATCH 2/5] cfg80211: fix typo of IWEVASSOCRESPIE
[PATCH 3/5] iwmc3200wifi: use cfg80211_connect_result to send req/resp IE
[PATCH 4/5] iwmc3200wifi: fix cfg80211_connect_result is called in IBSS
[PATCH 5/5] iwmc3200wifi: fix a use-after-free bug

Thanks,
-yi

[PATCH 1/5] cfg80211: set_default_key only for WEP

[Zhu Yi]

We invoke the cfg80211 set_default_key callback only for WEP key
configuring.

Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
 net/wireless/wext-compat.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
index aa80c0c..c63e10e 100644
--- a/net/wireless/wext-compat.c
+++ b/net/wireless/wext-compat.c
@@ -531,7 +531,8 @@ static int __cfg80211_set_encryption(struct cfg80211_registered_device *rdev,
 			wdev->wext.keys->data[idx];
 	}
 
-	if (params->cipher != WLAN_CIPHER_SUITE_AES_CMAC &&
+	if ((params->cipher == WLAN_CIPHER_SUITE_WEP40 ||
+	     params->cipher == WLAN_CIPHER_SUITE_WEP104) &&
 	    (tx_key || (!addr && wdev->wext.default_key == -1))) {
 		if (wdev->current_bss)
 			err = rdev->ops->set_default_key(&rdev->wiphy,
-- 
1.6.0.4

[PATCH 2/5] cfg80211: fix typo of IWEVASSOCRESPIE

[Zhu Yi]

It should be IWEVASSOCREQIE instead.

Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
 net/wireless/sme.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index e7a8851..82de2d9 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -341,7 +341,7 @@ void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
 		if (req_ie && status == WLAN_STATUS_SUCCESS) {
 			memset(&wrqu, 0, sizeof(wrqu));
 			wrqu.data.length = req_ie_len;
-			wireless_send_event(dev, IWEVASSOCRESPIE, &wrqu, req_ie);
+			wireless_send_event(dev, IWEVASSOCREQIE, &wrqu, req_ie);
 		}
 
 		if (resp_ie && status == WLAN_STATUS_SUCCESS) {
@@ -474,7 +474,7 @@ void __cfg80211_roamed(struct wireless_dev *wdev, const u8 *bssid,
 	if (req_ie) {
 		memset(&wrqu, 0, sizeof(wrqu));
 		wrqu.data.length = req_ie_len;
-		wireless_send_event(wdev->netdev, IWEVASSOCRESPIE,
+		wireless_send_event(wdev->netdev, IWEVASSOCREQIE,
 				    &wrqu, req_ie);
 	}
 
-- 
1.6.0.4

[PATCH 3/5] iwmc3200wifi: use cfg80211_connect_result to send req/resp IE

[Zhu Yi]

cfg80211_connect_result() let us specify associate request and
response IEs as parameters after we are connected. We use this
capability instead of doing it ourselves with WEXT.

Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
 drivers/net/wireless/iwmc3200wifi/iwm.h  |    5 ++++
 drivers/net/wireless/iwmc3200wifi/main.c |    7 +++++
 drivers/net/wireless/iwmc3200wifi/rx.c   |   36 +++++++++++++++++++----------
 3 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/iwmc3200wifi/iwm.h b/drivers/net/wireless/iwmc3200wifi/iwm.h
index 79d9d89..2175a48 100644
--- a/drivers/net/wireless/iwmc3200wifi/iwm.h
+++ b/drivers/net/wireless/iwmc3200wifi/iwm.h
@@ -281,6 +281,11 @@ struct iwm_priv {
 	struct work_struct reset_worker;
 	struct mutex mutex;
 
+	u8 *req_ie;
+	int req_ie_len;
+	u8 *resp_ie;
+	int resp_ie_len;
+
 	char private[0] __attribute__((__aligned__(NETDEV_ALIGN)));
 };
 
diff --git a/drivers/net/wireless/iwmc3200wifi/main.c b/drivers/net/wireless/iwmc3200wifi/main.c
index 484f110..cf25744 100644
--- a/drivers/net/wireless/iwmc3200wifi/main.c
+++ b/drivers/net/wireless/iwmc3200wifi/main.c
@@ -497,6 +497,13 @@ void iwm_link_off(struct iwm_priv *iwm)
 	memset(wstats, 0, sizeof(struct iw_statistics));
 	wstats->qual.updated = IW_QUAL_ALL_INVALID;
 
+	kfree(iwm->req_ie);
+	iwm->req_ie = NULL;
+	iwm->req_ie_len = 0;
+	kfree(iwm->resp_ie);
+	iwm->resp_ie = NULL;
+	iwm->resp_ie_len = 0;
+
 	del_timer_sync(&iwm->watchdog);
 }
 
diff --git a/drivers/net/wireless/iwmc3200wifi/rx.c b/drivers/net/wireless/iwmc3200wifi/rx.c
index 82b572a..6743391 100644
--- a/drivers/net/wireless/iwmc3200wifi/rx.c
+++ b/drivers/net/wireless/iwmc3200wifi/rx.c
@@ -519,7 +519,8 @@ static int iwm_mlme_assoc_complete(struct iwm_priv *iwm, u8 *buf,
 
 		cfg80211_connect_result(iwm_to_ndev(iwm),
 					complete->bssid,
-					NULL, 0, NULL, 0,
+					iwm->req_ie, iwm->req_ie_len,
+					iwm->resp_ie, iwm->resp_ie_len,
 					WLAN_STATUS_SUCCESS, GFP_KERNEL);
 		break;
 	case UMAC_ASSOC_COMPLETE_FAILURE:
@@ -771,37 +772,46 @@ static int iwm_mlme_mgt_frame(struct iwm_priv *iwm, u8 *buf,
 			      unsigned long buf_size, struct iwm_wifi_cmd *cmd)
 {
 	struct iwm_umac_notif_mgt_frame *mgt_frame =
-	(struct iwm_umac_notif_mgt_frame *)buf;
+			(struct iwm_umac_notif_mgt_frame *)buf;
 	struct ieee80211_mgmt *mgt = (struct ieee80211_mgmt *)mgt_frame->frame;
 	u8 *ie;
-	unsigned int event;
-	union iwreq_data wrqu;
 
 	IWM_HEXDUMP(iwm, DBG, MLME, "MGT: ", mgt_frame->frame,
 		    le16_to_cpu(mgt_frame->len));
 
 	if (ieee80211_is_assoc_req(mgt->frame_control)) {
 		ie = mgt->u.assoc_req.variable;;
-		event = IWEVASSOCREQIE;
+		iwm->req_ie_len =
+				le16_to_cpu(mgt_frame->len) - (ie - (u8 *)mgt);
+		kfree(iwm->req_ie);
+		iwm->req_ie = kmemdup(mgt->u.assoc_req.variable,
+				      iwm->req_ie_len, GFP_KERNEL);
 	} else if (ieee80211_is_reassoc_req(mgt->frame_control)) {
 		ie = mgt->u.reassoc_req.variable;;
-		event = IWEVASSOCREQIE;
+		iwm->req_ie_len =
+				le16_to_cpu(mgt_frame->len) - (ie - (u8 *)mgt);
+		kfree(iwm->req_ie);
+		iwm->req_ie = kmemdup(mgt->u.reassoc_req.variable,
+				      iwm->req_ie_len, GFP_KERNEL);
 	} else if (ieee80211_is_assoc_resp(mgt->frame_control)) {
 		ie = mgt->u.assoc_resp.variable;;
-		event = IWEVASSOCRESPIE;
+		iwm->resp_ie_len =
+				le16_to_cpu(mgt_frame->len) - (ie - (u8 *)mgt);
+		kfree(iwm->resp_ie);
+		iwm->resp_ie = kmemdup(mgt->u.assoc_resp.variable,
+				       iwm->resp_ie_len, GFP_KERNEL);
 	} else if (ieee80211_is_reassoc_resp(mgt->frame_control)) {
 		ie = mgt->u.reassoc_resp.variable;;
-		event = IWEVASSOCRESPIE;
+		iwm->resp_ie_len =
+				le16_to_cpu(mgt_frame->len) - (ie - (u8 *)mgt);
+		kfree(iwm->resp_ie);
+		iwm->resp_ie = kmemdup(mgt->u.reassoc_resp.variable,
+				       iwm->resp_ie_len, GFP_KERNEL);
 	} else {
 		IWM_ERR(iwm, "Unsupported management frame");
 		return 0;
 	}
 
-	wrqu.data.length = le16_to_cpu(mgt_frame->len) - (ie - (u8 *)mgt);
-
-	IWM_HEXDUMP(iwm, DBG, MLME, "EVT: ", ie, wrqu.data.length);
-	wireless_send_event(iwm_to_ndev(iwm), event, &wrqu, ie);
-
 	return 0;
 }
 
-- 
1.6.0.4

[PATCH 4/5] iwmc3200wifi: fix cfg80211_connect_result is called in IBSS

[Zhu Yi]

Avoid calling cfg80211_connect_result() in IBSS mode.

Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
 drivers/net/wireless/iwmc3200wifi/rx.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/iwmc3200wifi/rx.c b/drivers/net/wireless/iwmc3200wifi/rx.c
index 6743391..86079a1 100644
--- a/drivers/net/wireless/iwmc3200wifi/rx.c
+++ b/drivers/net/wireless/iwmc3200wifi/rx.c
@@ -517,6 +517,9 @@ static int iwm_mlme_assoc_complete(struct iwm_priv *iwm, u8 *buf,
 
 		iwm_link_on(iwm);
 
+		if (iwm->conf.mode == UMAC_MODE_IBSS)
+			goto ibss;
+
 		cfg80211_connect_result(iwm_to_ndev(iwm),
 					complete->bssid,
 					iwm->req_ie, iwm->req_ie_len,
@@ -530,6 +533,9 @@ static int iwm_mlme_assoc_complete(struct iwm_priv *iwm, u8 *buf,
 
 		iwm_link_off(iwm);
 
+		if (iwm->conf.mode == UMAC_MODE_IBSS)
+			goto ibss;
+
 		cfg80211_connect_result(iwm_to_ndev(iwm), complete->bssid,
 					NULL, 0, NULL, 0,
 					WLAN_STATUS_UNSPECIFIED_FAILURE,
@@ -538,11 +544,10 @@ static int iwm_mlme_assoc_complete(struct iwm_priv *iwm, u8 *buf,
 		break;
 	}
 
-	if (iwm->conf.mode == UMAC_MODE_IBSS) {
-		cfg80211_ibss_joined(iwm_to_ndev(iwm), iwm->bssid, GFP_KERNEL);
-		return 0;
-	}
+	return 0;
 
+ ibss:
+	cfg80211_ibss_joined(iwm_to_ndev(iwm), iwm->bssid, GFP_KERNEL);
 	return 0;
 }
 
-- 
1.6.0.4

[PATCH 5/5] iwmc3200wifi: fix a use-after-free bug

[Zhu Yi]

The patch fixes a use-after-free bug for cmd->seq_num;

Reported-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
 drivers/net/wireless/iwmc3200wifi/hal.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/iwmc3200wifi/hal.c b/drivers/net/wireless/iwmc3200wifi/hal.c
index ee127fe..c430418 100644
--- a/drivers/net/wireless/iwmc3200wifi/hal.c
+++ b/drivers/net/wireless/iwmc3200wifi/hal.c
@@ -105,9 +105,9 @@
 #include "umac.h"
 #include "debug.h"
 
-static void iwm_nonwifi_cmd_init(struct iwm_priv *iwm,
-				 struct iwm_nonwifi_cmd *cmd,
-				 struct iwm_udma_nonwifi_cmd *udma_cmd)
+static int iwm_nonwifi_cmd_init(struct iwm_priv *iwm,
+				struct iwm_nonwifi_cmd *cmd,
+				struct iwm_udma_nonwifi_cmd *udma_cmd)
 {
 	INIT_LIST_HEAD(&cmd->pending);
 
@@ -118,7 +118,7 @@ static void iwm_nonwifi_cmd_init(struct iwm_priv *iwm,
 	cmd->seq_num = iwm->nonwifi_seq_num;
 	udma_cmd->seq_num = cpu_to_le16(cmd->seq_num);
 
-	cmd->seq_num = iwm->nonwifi_seq_num++;
+	iwm->nonwifi_seq_num++;
 	iwm->nonwifi_seq_num %= UMAC_NONWIFI_SEQ_NUM_MAX;
 
 	if (udma_cmd->resp)
@@ -130,6 +130,8 @@ static void iwm_nonwifi_cmd_init(struct iwm_priv *iwm,
 	cmd->buf.len = 0;
 
 	memcpy(&cmd->udma_cmd, udma_cmd, sizeof(*udma_cmd));
+
+	return cmd->seq_num;
 }
 
 u16 iwm_alloc_wifi_cmd_seq(struct iwm_priv *iwm)
@@ -369,7 +371,7 @@ int iwm_hal_send_target_cmd(struct iwm_priv *iwm,
 			    const void *payload)
 {
 	struct iwm_nonwifi_cmd *cmd;
-	int ret;
+	int ret, seq_num;
 
 	cmd = kzalloc(sizeof(struct iwm_nonwifi_cmd), GFP_KERNEL);
 	if (!cmd) {
@@ -377,7 +379,7 @@ int iwm_hal_send_target_cmd(struct iwm_priv *iwm,
 		return -ENOMEM;
 	}
 
-	iwm_nonwifi_cmd_init(iwm, cmd, udma_cmd);
+	seq_num = iwm_nonwifi_cmd_init(iwm, cmd, udma_cmd);
 
 	if (cmd->udma_cmd.opcode == UMAC_HDI_OUT_OPCODE_WRITE ||
 	    cmd->udma_cmd.opcode == UMAC_HDI_OUT_OPCODE_WRITE_PERSISTENT) {
@@ -393,7 +395,7 @@ int iwm_hal_send_target_cmd(struct iwm_priv *iwm,
 	if (ret < 0)
 		return ret;
 
-	return cmd->seq_num;
+	return seq_num;
 }
 
 static void iwm_build_lmac_hdr(struct iwm_priv *iwm, struct iwm_lmac_hdr *hdr,
-- 
1.6.0.4

Re: [PATCH 1/5] cfg80211: set_default_key only for WEP

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]

On Mon, 2009-07-20 at 11:47 +0800, Zhu Yi wrote:
> We invoke the cfg80211 set_default_key callback only for WEP key
> configuring.
> 
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>

Acked-by: Johannes Berg <johannes@sipsolutions.net>

> ---
>  net/wireless/wext-compat.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
> index aa80c0c..c63e10e 100644
> --- a/net/wireless/wext-compat.c
> +++ b/net/wireless/wext-compat.c
> @@ -531,7 +531,8 @@ static int __cfg80211_set_encryption(struct cfg80211_registered_device *rdev,
>  			wdev->wext.keys->data[idx];
>  	}
>  
> -	if (params->cipher != WLAN_CIPHER_SUITE_AES_CMAC &&
> +	if ((params->cipher == WLAN_CIPHER_SUITE_WEP40 ||
> +	     params->cipher == WLAN_CIPHER_SUITE_WEP104) &&
>  	    (tx_key || (!addr && wdev->wext.default_key == -1))) {
>  		if (wdev->current_bss)
>  			err = rdev->ops->set_default_key(&rdev->wiphy,

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/5] cfg80211: fix typo of IWEVASSOCRESPIE

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1157 bytes --]

On Mon, 2009-07-20 at 11:47 +0800, Zhu Yi wrote:
> It should be IWEVASSOCREQIE instead.

Indeed, thanks.

> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
> ---
>  net/wireless/sme.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/wireless/sme.c b/net/wireless/sme.c
> index e7a8851..82de2d9 100644
> --- a/net/wireless/sme.c
> +++ b/net/wireless/sme.c
> @@ -341,7 +341,7 @@ void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
>  		if (req_ie && status == WLAN_STATUS_SUCCESS) {
>  			memset(&wrqu, 0, sizeof(wrqu));
>  			wrqu.data.length = req_ie_len;
> -			wireless_send_event(dev, IWEVASSOCRESPIE, &wrqu, req_ie);
> +			wireless_send_event(dev, IWEVASSOCREQIE, &wrqu, req_ie);
>  		}
>  
>  		if (resp_ie && status == WLAN_STATUS_SUCCESS) {
> @@ -474,7 +474,7 @@ void __cfg80211_roamed(struct wireless_dev *wdev, const u8 *bssid,
>  	if (req_ie) {
>  		memset(&wrqu, 0, sizeof(wrqu));
>  		wrqu.data.length = req_ie_len;
> -		wireless_send_event(wdev->netdev, IWEVASSOCRESPIE,
> +		wireless_send_event(wdev->netdev, IWEVASSOCREQIE,
>  				    &wrqu, req_ie);
>  	}
>  

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 3/5] iwmc3200wifi: use cfg80211_connect_result to send req/resp IE

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 343 bytes --]

On Mon, 2009-07-20 at 11:47 +0800, Zhu Yi wrote:
> cfg80211_connect_result() let us specify associate request and
> response IEs as parameters after we are connected. We use this
> capability instead of doing it ourselves with WEXT.

I think with this, you can now remove

        select WIRELESS_EXT

from your Kconfig.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 3/5] iwmc3200wifi: use cfg80211_connect_result to send req/resp IE

[Zhu Yi]

On Mon, 2009-07-20 at 18:37 +0800, Johannes Berg wrote:
> On Mon, 2009-07-20 at 11:47 +0800, Zhu Yi wrote:
> > cfg80211_connect_result() let us specify associate request and
> > response IEs as parameters after we are connected. We use this
> > capability instead of doing it ourselves with WEXT.
> 
> I think with this, you can now remove
> 
>         select WIRELESS_EXT
> 
> from your Kconfig.

Um, not yet. I need to assign the netdev->wireless_handlers to
&iwm_iw_handler_def. The former is ifdef-ed CONFIG_WIRELESS_EXT in
struct net_device. I think this can be done until cdg80211 provides a
generic iw_handler.

Thanks,
-yi

Re: [PATCH 3/5] iwmc3200wifi: use cfg80211_connect_result to send req/resp IE

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 836 bytes --]

On Tue, 2009-07-21 at 09:22 +0800, Zhu Yi wrote:
> On Mon, 2009-07-20 at 18:37 +0800, Johannes Berg wrote:
> > On Mon, 2009-07-20 at 11:47 +0800, Zhu Yi wrote:
> > > cfg80211_connect_result() let us specify associate request and
> > > response IEs as parameters after we are connected. We use this
> > > capability instead of doing it ourselves with WEXT.
> > 
> > I think with this, you can now remove
> > Indeed, I have a patch to remove that.
> >         select WIRELESS_EXT
> > 
> > from your Kconfig.
> 
> Um, not yet. I need to assign the netdev->wireless_handlers to
> &iwm_iw_handler_def. The former is ifdef-ed CONFIG_WIRELESS_EXT in
> struct net_device. I think this can be done until cdg80211 provides a
> generic iw_handler.

Oops, you're right. I have a patch, but John needs to merge stuff.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]