2009-07-20 03:46:25

by Zhu Yi

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

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


2009-07-20 10:36:10

by Johannes Berg

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

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

Indeed, thanks.

> Signed-off-by: Zhu Yi <[email protected]>
> ---
> 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);
> }
>


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-07-20 03:46:29

by Zhu Yi

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

Avoid calling cfg80211_connect_result() in IBSS mode.

Signed-off-by: Zhu Yi <[email protected]>
---
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


2009-07-20 10:34:48

by Johannes Berg

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

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 <[email protected]>

Acked-by: Johannes Berg <[email protected]>

> ---
> 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,


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-07-20 03:46:30

by Zhu Yi

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

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

Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Zhu Yi <[email protected]>
---
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


2009-07-20 03:46:27

by Zhu Yi

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

It should be IWEVASSOCREQIE instead.

Signed-off-by: Zhu Yi <[email protected]>
---
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


2009-07-20 03:46:26

by Zhu Yi

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

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

Signed-off-by: Zhu Yi <[email protected]>
---
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


2009-07-21 10:50:48

by Johannes Berg

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

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


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-07-20 10:37:25

by Johannes Berg

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

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


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-07-21 01:22:16

by Zhu Yi

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

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


2009-07-20 03:46:28

by Zhu Yi

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

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 <[email protected]>
---
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