* [PATCH] iwlwifi: fix oops on wep key insertion @ 2008-05-27 5:13 Joonwoo Park 2008-05-27 6:41 ` Tomas Winkler 0 siblings, 1 reply; 18+ messages in thread From: Joonwoo Park @ 2008-05-27 5:13 UTC (permalink / raw) To: Zhu Yi, Reinette Chatre; +Cc: Joonwoo Park, linux-wireless Fix overflow which is occured by long wep key configuring. Also, this patch contains some cleanup for not to do memcpy with zero size. $sudo iwconfig wlan0 enc AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 IP: [memcpy_c+0xb/0x20] memcpy_c+0xb/0x20 PGD 13a590067 PUD 12e471067 PMD 0 Oops: 0000 [1] PREEMPT SMP CPU 1 ... Pid: 10, comm: events/1 Not tainted 2.6.26-rc2 #9 ... Call Trace: [iwl4965:iwl4965_rx_scan_start_notif+0xb/0x20] ? :iwl4965:iwl4965_enqueue_hcmd+0x12b/0x220 [hci_usb:init_module+0xe97/0x28cb0] :iwlcore:iwl_send_cmd_sync+0x67/0x290 [save_trace+0x3f/0xb0] ? save_trace+0x3f/0xb0 ... Signed-off-by: Joonwoo Park <joonwpark81@gmail.com> --- drivers/net/wireless/iwlwifi/iwl-sta.c | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/iwlwifi/iwl-sta.c b/drivers/net/wireless/iwlwifi/iwl-sta.c index e4fdfaa..c3dd138 100644 --- a/drivers/net/wireless/iwlwifi/iwl-sta.c +++ b/drivers/net/wireless/iwlwifi/iwl-sta.c @@ -51,7 +51,7 @@ int iwl_get_free_ucode_key_index(struct iwl_priv *priv) int iwl_send_static_wepkey_cmd(struct iwl_priv *priv, u8 send_if_empty) { - int i, not_empty = 0; + int i, left, not_empty = 0; u8 buff[sizeof(struct iwl_wep_cmd) + sizeof(struct iwl_wep_key) * WEP_KEYS_MAX]; struct iwl_wep_cmd *wep_cmd = (struct iwl_wep_cmd *)buff; @@ -67,16 +67,20 @@ int iwl_send_static_wepkey_cmd(struct iwl_priv *priv, u8 send_if_empty) for (i = 0; i < WEP_KEYS_MAX ; i++) { wep_cmd->key[i].key_index = i; - if (priv->wep_keys[i].key_size) { + wep_cmd->key[i].key_size = priv->wep_keys[i].key_size; + left = sizeof(wep_cmd->key[i].key) + - sizeof(wep_cmd->key[i].key[0]) * 3; + BUG_ON(left < 0); + + if (wep_cmd->key[i].key_size + && left >= wep_cmd->key[i].key_size) { wep_cmd->key[i].key_offset = i; not_empty = 1; + memcpy(&wep_cmd->key[i].key[3], priv->wep_keys[i].key, + wep_cmd->key[i].key_size); } else { wep_cmd->key[i].key_offset = WEP_INVALID_OFFSET; } - - wep_cmd->key[i].key_size = priv->wep_keys[i].key_size; - memcpy(&wep_cmd->key[i].key[3], priv->wep_keys[i].key, - priv->wep_keys[i].key_size); } wep_cmd->global_key_type = WEP_KEY_WEP_TYPE; -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-05-27 5:13 [PATCH] iwlwifi: fix oops on wep key insertion Joonwoo Park @ 2008-05-27 6:41 ` Tomas Winkler 2008-05-27 7:10 ` Joonwoo Park 2008-05-27 10:58 ` JMF 0 siblings, 2 replies; 18+ messages in thread From: Tomas Winkler @ 2008-05-27 6:41 UTC (permalink / raw) To: Joonwoo Park; +Cc: Zhu Yi, Reinette Chatre, linux-wireless On Tue, May 27, 2008 at 8:13 AM, Joonwoo Park <joonwpark81@gmail.com> wrote: > Fix overflow which is occured by long wep key configuring. > Also, this patch contains some cleanup for not to do memcpy with > zero size. > > $sudo iwconfig wlan0 enc AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA > WEP key is of fixed size 13 or 5 bytes, this setting should be refused somewhere in the mac wext handler. NACK. Tomas ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-05-27 6:41 ` Tomas Winkler @ 2008-05-27 7:10 ` Joonwoo Park 2008-05-27 10:58 ` JMF 1 sibling, 0 replies; 18+ messages in thread From: Joonwoo Park @ 2008-05-27 7:10 UTC (permalink / raw) To: Tomas Winkler; +Cc: Zhu Yi, Reinette Chatre, linux-wireless 2008/5/26 Tomas Winkler <tomasw@gmail.com>: > On Tue, May 27, 2008 at 8:13 AM, Joonwoo Park <joonwpark81@gmail.com> wrote: >> Fix overflow which is occured by long wep key configuring. >> Also, this patch contains some cleanup for not to do memcpy with >> zero size. >> >> $sudo iwconfig wlan0 enc AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA >> > > WEP key is of fixed size 13 or 5 bytes, this setting should be > refused somewhere in the mac wext handler. > NACK. > Tomas > It sounds reasonable. I'll dig this problem more. Thanks, Joonwoo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-05-27 6:41 ` Tomas Winkler 2008-05-27 7:10 ` Joonwoo Park @ 2008-05-27 10:58 ` JMF 2008-05-27 12:41 ` Tomas Winkler 1 sibling, 1 reply; 18+ messages in thread From: JMF @ 2008-05-27 10:58 UTC (permalink / raw) To: linux-wireless Tomas Winkler <tomasw@...> writes: > WEP key is of fixed size 13 or 5 bytes, this setting should be > refused somewhere in the mac wext handler. > NACK. > Tomas What about 256-bit WEP key (232-bit key I think), it won't be supported? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-05-27 10:58 ` JMF @ 2008-05-27 12:41 ` Tomas Winkler 2008-05-27 13:53 ` Dan Williams 0 siblings, 1 reply; 18+ messages in thread From: Tomas Winkler @ 2008-05-27 12:41 UTC (permalink / raw) To: JMF; +Cc: linux-wireless On Tue, May 27, 2008 at 1:58 PM, JMF <tolas_feup@hotmail.com> wrote: > Tomas Winkler <tomasw@...> writes: > >> WEP key is of fixed size 13 or 5 bytes, this setting should be >> refused somewhere in the mac wext handler. >> NACK. >> Tomas > What about 256-bit WEP key (232-bit key I think), it won't be supported? > Not supported by iwlwifi HW, have to be done by SW, We need to check then probably in driver whether we support it or not and return error value in this case, in other cases I'm not sure how defensive the code should be.... By the way isn't this a waste of bits. AES seams to be secure enough. Who is implementing this at all? Thanks Tomas Tomas ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-05-27 12:41 ` Tomas Winkler @ 2008-05-27 13:53 ` Dan Williams 2008-05-28 0:41 ` John W. Linville 2008-05-28 10:14 ` JMF 0 siblings, 2 replies; 18+ messages in thread From: Dan Williams @ 2008-05-27 13:53 UTC (permalink / raw) To: Tomas Winkler; +Cc: JMF, linux-wireless On Tue, 2008-05-27 at 15:41 +0300, Tomas Winkler wrote: > On Tue, May 27, 2008 at 1:58 PM, JMF <tolas_feup@hotmail.com> wrote: > > Tomas Winkler <tomasw@...> writes: > > > >> WEP key is of fixed size 13 or 5 bytes, this setting should be > >> refused somewhere in the mac wext handler. > >> NACK. > >> Tomas > > What about 256-bit WEP key (232-bit key I think), it won't be supported? > > > Not supported by iwlwifi HW, have to be done by SW, We need to check > then probably in driver whether we support it or not and return error > value in this case, in other cases I'm not sure how defensive the code > should be.... > By the way isn't this a waste of bits. AES seams to be secure enough. > Who is implementing this at all? IIRC D-Link is one manufacturer who implemented 152-bit and higher WEP keys when people started to get scared about WEP, but before 802.11i/WPA actually came out. I'm pretty sure that not too many sites use greater than 104/128-bit WEP, and the cards that support it in hardware (most cards were fullmac at the time) we can probably count on one hand. For softmac cards, we'd have to weigh the maintenance cost of keeping the codepaths around that most people wouldn't use and for hardware that nobody developing the stack currently has to test against against the benefit of enabling these users to work with legacy APs. I've gotten maybe 1 or 2 requests for > 104/128-bit WEP key support for NM in 3 years. Nice to have, but I'm not sure it's worth the extra code and maintenance burden? Would be good to have somebody tell us what hardware (APs and cards) support this though. Dan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-05-27 13:53 ` Dan Williams @ 2008-05-28 0:41 ` John W. Linville 2008-06-15 16:46 ` Joonwoo Park 2008-05-28 10:14 ` JMF 1 sibling, 1 reply; 18+ messages in thread From: John W. Linville @ 2008-05-28 0:41 UTC (permalink / raw) To: Dan Williams; +Cc: Tomas Winkler, JMF, linux-wireless On Tue, May 27, 2008 at 09:53:43AM -0400, Dan Williams wrote: > I've gotten maybe 1 or 2 requests for > 104/128-bit WEP key support for > NM in 3 years. Nice to have, but I'm not sure it's worth the extra code > and maintenance burden? Would be good to have somebody tell us what > hardware (APs and cards) support this though. I'm inclined to think that it is _not_ worth the trouble for this particular feature. John -- John W. Linville linville@tuxdriver.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-05-28 0:41 ` John W. Linville @ 2008-06-15 16:46 ` Joonwoo Park 2008-06-15 16:53 ` Tomas Winkler 0 siblings, 1 reply; 18+ messages in thread From: Joonwoo Park @ 2008-06-15 16:46 UTC (permalink / raw) To: John W. Linville; +Cc: Dan Williams, Tomas Winkler, JMF, linux-wireless On Tue, May 27, 2008 at 08:41:00PM -0400, John W. Linville wrote: > On Tue, May 27, 2008 at 09:53:43AM -0400, Dan Williams wrote: > > > I've gotten maybe 1 or 2 requests for > 104/128-bit WEP key support for > > NM in 3 years. Nice to have, but I'm not sure it's worth the extra code > > and maintenance burden? Would be good to have somebody tell us what > > hardware (APs and cards) support this though. > > I'm inclined to think that it is _not_ worth the trouble for this > particular feature. > > John This patch limits wep key size to 128/104-bits. I hope you guys like this. Thanks, Joonwoo --- [PATCH] wireless: Limit wep key size to 128/104-bits This patch prevents overflow which is occured by invalid long wep key insertion $sudo iwconfig wlan0 enc AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 IP: [memcpy_c+0xb/0x20] memcpy_c+0xb/0x20 PGD 13a590067 PUD 12e471067 PMD 0 Oops: 0000 [1] PREEMPT SMP CPU 1 ... Pid: 10, comm: events/1 Not tainted 2.6.26-rc2 #9 ... Call Trace: [iwl4965:iwl4965_rx_scan_start_notif+0xb/0x20] ? :iwl4965:iwl4965_enqueue_hcmd+0x12b/0x220 [hci_usb:init_module+0xe97/0x28cb0] :iwlcore:iwl_send_cmd_sync+0x67/0x290 [save_trace+0x3f/0xb0] ? save_trace+0x3f/0xb0 ... Signed-off-by: Joonwoo Park <joonwpark81@gmail.com> --- net/wireless/wext.c | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-) diff --git a/net/wireless/wext.c b/net/wireless/wext.c index 947188a..c8ef55b 100644 --- a/net/wireless/wext.c +++ b/net/wireless/wext.c @@ -102,6 +102,8 @@ #include <asm/uaccess.h> /* copy_to_user() */ +#define KEY_SIZE_WEP104 13 + /************************* GLOBAL VARIABLES *************************/ /* * You should not use global variables, because of re-entrancy. @@ -740,8 +742,8 @@ static int ioctl_standard_call(struct net_device * dev, * for max space. Easier, and won't last long... */ extra_size = descr->max_tokens * descr->token_size; - /* Check need for ESSID compatibility for WE < 21 */ switch (cmd) { + /* Check need for ESSID compatibility for WE < 21 */ case SIOCSIWESSID: case SIOCGIWESSID: case SIOCSIWNICKN: @@ -761,6 +763,13 @@ static int ioctl_standard_call(struct net_device * dev, essid_compat = 1; } break; + + /* Limit wep key size to 128/104-bits */ + case SIOCSIWENCODE: + if (iwr->u.data.length > KEY_SIZE_WEP104) + return -EINVAL; + break; + default: break; } -- 1.5.4.3 --- ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-06-15 16:46 ` Joonwoo Park @ 2008-06-15 16:53 ` Tomas Winkler 2008-06-16 8:46 ` Johannes Berg 0 siblings, 1 reply; 18+ messages in thread From: Tomas Winkler @ 2008-06-15 16:53 UTC (permalink / raw) To: Joonwoo Park; +Cc: John W. Linville, Dan Williams, JMF, linux-wireless On Sun, Jun 15, 2008 at 7:46 PM, Joonwoo Park <joonwpark81@gmail.com> wrote: > On Tue, May 27, 2008 at 08:41:00PM -0400, John W. Linville wrote: >> On Tue, May 27, 2008 at 09:53:43AM -0400, Dan Williams wrote: >> >> > I've gotten maybe 1 or 2 requests for > 104/128-bit WEP key support for >> > NM in 3 years. Nice to have, but I'm not sure it's worth the extra code >> > and maintenance burden? Would be good to have somebody tell us what >> > hardware (APs and cards) support this though. >> >> I'm inclined to think that it is _not_ worth the trouble for this >> particular feature. >> >> John > > This patch limits wep key size to 128/104-bits. > I hope you guys like this. > > Thanks, > > Joonwoo > > --- > [PATCH] wireless: Limit wep key size to 128/104-bits > > This patch prevents overflow which is occured by invalid long wep key > insertion > > $sudo iwconfig wlan0 enc AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 > IP: [memcpy_c+0xb/0x20] memcpy_c+0xb/0x20 > PGD 13a590067 PUD 12e471067 PMD 0 > Oops: 0000 [1] PREEMPT SMP > CPU 1 > ... > Pid: 10, comm: events/1 Not tainted 2.6.26-rc2 #9 > ... > Call Trace: > [iwl4965:iwl4965_rx_scan_start_notif+0xb/0x20] ? :iwl4965:iwl4965_enqueue_hcmd+0x12b/0x220 > [hci_usb:init_module+0xe97/0x28cb0] :iwlcore:iwl_send_cmd_sync+0x67/0x290 > [save_trace+0x3f/0xb0] ? save_trace+0x3f/0xb0 > ... > > Signed-off-by: Joonwoo Park <joonwpark81@gmail.com> > --- > net/wireless/wext.c | 11 ++++++++++- > 1 files changed, 10 insertions(+), 1 deletions(-) > > diff --git a/net/wireless/wext.c b/net/wireless/wext.c > index 947188a..c8ef55b 100644 > --- a/net/wireless/wext.c > +++ b/net/wireless/wext.c > @@ -102,6 +102,8 @@ > > #include <asm/uaccess.h> /* copy_to_user() */ > > +#define KEY_SIZE_WEP104 13 > + > /************************* GLOBAL VARIABLES *************************/ > /* > * You should not use global variables, because of re-entrancy. > @@ -740,8 +742,8 @@ static int ioctl_standard_call(struct net_device * dev, > * for max space. Easier, and won't last long... */ > extra_size = descr->max_tokens * descr->token_size; > > - /* Check need for ESSID compatibility for WE < 21 */ > switch (cmd) { > + /* Check need for ESSID compatibility for WE < 21 */ > case SIOCSIWESSID: > case SIOCGIWESSID: > case SIOCSIWNICKN: > @@ -761,6 +763,13 @@ static int ioctl_standard_call(struct net_device * dev, > essid_compat = 1; > } > break; > + > + /* Limit wep key size to 128/104-bits */ > + case SIOCSIWENCODE: > + if (iwr->u.data.length > KEY_SIZE_WEP104) > + return -EINVAL; > + break; > + > default: > break; > } > -- > 1.5.4.3 We've already posted a patch 'iwlwifi: add bad length check for WEP keys' that fixes this for iwlwifi. I believe that long keys still can be handled by software crypto. If the key length is not supported driver returns error values and we fail down to software crypto John, our patch is probably 2.6.26 material. I didn't check it though if if applies on wireless-2.6.git yet Thanks Tomas ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-06-15 16:53 ` Tomas Winkler @ 2008-06-16 8:46 ` Johannes Berg 2008-06-16 14:30 ` Dan Williams 2008-06-27 15:28 ` John W. Linville 0 siblings, 2 replies; 18+ messages in thread From: Johannes Berg @ 2008-06-16 8:46 UTC (permalink / raw) To: Tomas Winkler Cc: Joonwoo Park, John W. Linville, Dan Williams, JMF, linux-wireless [-- Attachment #1: Type: text/plain, Size: 1173 bytes --] > > [PATCH] wireless: Limit wep key size to 128/104-bits > > > > This patch prevents overflow which is occured by invalid long wep key > > insertion > > > > $sudo iwconfig wlan0 enc AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA > > > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 > > IP: [memcpy_c+0xb/0x20] memcpy_c+0xb/0x20 > > PGD 13a590067 PUD 12e471067 PMD 0 > > Oops: 0000 [1] PREEMPT SMP > > CPU 1 > > ... > > Pid: 10, comm: events/1 Not tainted 2.6.26-rc2 #9 > > ... > > Call Trace: > > [iwl4965:iwl4965_rx_scan_start_notif+0xb/0x20] ? :iwl4965:iwl4965_enqueue_hcmd+0x12b/0x220 > > [hci_usb:init_module+0xe97/0x28cb0] :iwlcore:iwl_send_cmd_sync+0x67/0x290 > > [save_trace+0x3f/0xb0] ? save_trace+0x3f/0xb0 > > ... > > > > Signed-off-by: Joonwoo Park <joonwpark81@gmail.com> > > --- > > net/wireless/wext.c | 11 ++++++++++- I'm sure Jean will cry murder because he expects there are some stupid full-mac cards that actually support other sizes. Can't somebody just post a patch to mac80211 that only accepts the two correct sizes like cfg80211 does? johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-06-16 8:46 ` Johannes Berg @ 2008-06-16 14:30 ` Dan Williams 2008-06-27 15:28 ` John W. Linville 1 sibling, 0 replies; 18+ messages in thread From: Dan Williams @ 2008-06-16 14:30 UTC (permalink / raw) To: Johannes Berg Cc: Tomas Winkler, Joonwoo Park, John W. Linville, JMF, linux-wireless On Mon, 2008-06-16 at 10:46 +0200, Johannes Berg wrote: > > > [PATCH] wireless: Limit wep key size to 128/104-bits > > > > > > This patch prevents overflow which is occured by invalid long wep key > > > insertion > > > > > > $sudo iwconfig wlan0 enc AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA > > > > > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 > > > IP: [memcpy_c+0xb/0x20] memcpy_c+0xb/0x20 > > > PGD 13a590067 PUD 12e471067 PMD 0 > > > Oops: 0000 [1] PREEMPT SMP > > > CPU 1 > > > ... > > > Pid: 10, comm: events/1 Not tainted 2.6.26-rc2 #9 > > > ... > > > Call Trace: > > > [iwl4965:iwl4965_rx_scan_start_notif+0xb/0x20] ? :iwl4965:iwl4965_enqueue_hcmd+0x12b/0x220 > > > [hci_usb:init_module+0xe97/0x28cb0] :iwlcore:iwl_send_cmd_sync+0x67/0x290 > > > [save_trace+0x3f/0xb0] ? save_trace+0x3f/0xb0 > > > ... > > > > > > Signed-off-by: Joonwoo Park <joonwpark81@gmail.com> > > > --- > > > net/wireless/wext.c | 11 ++++++++++- > > I'm sure Jean will cry murder because he expects there are some stupid > full-mac cards that actually support other sizes. > > Can't somebody just post a patch to mac80211 that only accepts the two > correct sizes like cfg80211 does? I'd +1 that. The hardware that appears to support 152-bit WEP would be driven by madwifi/ath5k [1] [2], and maybe rtl8185 [3]. Most other references I've found to 152-bit encryption support are for "108Mbps" or "Turbo" products, which are almost always Atheros-based chipsets. Since they are mac80211-driven cards with ath5k, they will obviously support WPA too. The only case I can think of for really supporting 152-bit WEP is if some fullmac part can't handle WPA but can handle 152-bit WEP. And no fullmac parts that I know of support 152-bit WEP. Dan [1] http://www.seattlewireless.net/HardwareComparison (Proxim Orinoco Atheros-based) [2] http://support.dlink.com/products/view.asp?productid=DWL%2DAB650 [3] http://svp.co.uk/product/tp-link_tl-wn353g_54mbps_wireless_pci_adapter_4669 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-06-16 8:46 ` Johannes Berg 2008-06-16 14:30 ` Dan Williams @ 2008-06-27 15:28 ` John W. Linville 2008-06-27 16:07 ` Dan Williams 1 sibling, 1 reply; 18+ messages in thread From: John W. Linville @ 2008-06-27 15:28 UTC (permalink / raw) To: Johannes Berg Cc: Tomas Winkler, Joonwoo Park, Dan Williams, JMF, linux-wireless On Mon, Jun 16, 2008 at 10:46:29AM +0200, Johannes Berg wrote: > > > > [PATCH] wireless: Limit wep key size to 128/104-bits > > > > > > This patch prevents overflow which is occured by invalid long wep key > > > insertion > > > > > > $sudo iwconfig wlan0 enc AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA > > > > > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 > > > IP: [memcpy_c+0xb/0x20] memcpy_c+0xb/0x20 > > > PGD 13a590067 PUD 12e471067 PMD 0 > > > Oops: 0000 [1] PREEMPT SMP > > > CPU 1 > > > ... > > > Pid: 10, comm: events/1 Not tainted 2.6.26-rc2 #9 > > > ... > > > Call Trace: > > > [iwl4965:iwl4965_rx_scan_start_notif+0xb/0x20] ? :iwl4965:iwl4965_enqueue_hcmd+0x12b/0x220 > > > [hci_usb:init_module+0xe97/0x28cb0] :iwlcore:iwl_send_cmd_sync+0x67/0x290 > > > [save_trace+0x3f/0xb0] ? save_trace+0x3f/0xb0 > > > ... > > > > > > Signed-off-by: Joonwoo Park <joonwpark81@gmail.com> > > > --- > > > net/wireless/wext.c | 11 ++++++++++- > > I'm sure Jean will cry murder because he expects there are some stupid > full-mac cards that actually support other sizes. > > Can't somebody just post a patch to mac80211 that only accepts the two > correct sizes like cfg80211 does? Strawman patch below... --- From: John W. Linville <linville@tuxdriver.com> Subject: [PATCH] mac80211: allow only standard size WEP keys through WEXT Limit ieee80211_ioctl_siwencode to only accept standard sized WEP keys. Signed-off-by: John W. Linville <linville@tuxdriver.com> --- net/mac80211/wext.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c index 5af3862..d16b975 100644 --- a/net/mac80211/wext.c +++ b/net/mac80211/wext.c @@ -26,6 +26,8 @@ #include "wpa.h" #include "aes_ccm.h" +#define KEY_SIZE_WEP104 13 /* 104/128-bit WEP keys */ +#define KEY_SIZE_WEP40 5 /* 40/64-bit WEP keys */ static int ieee80211_set_encryption(struct net_device *dev, u8 *sta_addr, int idx, int alg, int remove, @@ -879,6 +881,14 @@ static int ieee80211_ioctl_siwencode(struct net_device *dev, u8 bcaddr[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; int remove = 0; + switch (erq->length) { + case KEY_SIZE_WEP40: + case KEY_SIZE_WEP104: + break; + default: + return -EINVAL; + } + sdata = IEEE80211_DEV_TO_SUB_IF(dev); idx = erq->flags & IW_ENCODE_INDEX; -- 1.5.5.1 -- John W. Linville linville@tuxdriver.com ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-06-27 15:28 ` John W. Linville @ 2008-06-27 16:07 ` Dan Williams 2008-06-27 16:22 ` Tomas Winkler 0 siblings, 1 reply; 18+ messages in thread From: Dan Williams @ 2008-06-27 16:07 UTC (permalink / raw) To: John W. Linville Cc: Johannes Berg, Tomas Winkler, Joonwoo Park, JMF, linux-wireless On Fri, 2008-06-27 at 11:28 -0400, John W. Linville wrote: > On Mon, Jun 16, 2008 at 10:46:29AM +0200, Johannes Berg wrote: > > > > > > [PATCH] wireless: Limit wep key size to 128/104-bits > > > > > > > > This patch prevents overflow which is occured by invalid long wep key > > > > insertion > > > > > > > > $sudo iwconfig wlan0 enc AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA > > > > > > > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 > > > > IP: [memcpy_c+0xb/0x20] memcpy_c+0xb/0x20 > > > > PGD 13a590067 PUD 12e471067 PMD 0 > > > > Oops: 0000 [1] PREEMPT SMP > > > > CPU 1 > > > > ... > > > > Pid: 10, comm: events/1 Not tainted 2.6.26-rc2 #9 > > > > ... > > > > Call Trace: > > > > [iwl4965:iwl4965_rx_scan_start_notif+0xb/0x20] ? :iwl4965:iwl4965_enqueue_hcmd+0x12b/0x220 > > > > [hci_usb:init_module+0xe97/0x28cb0] :iwlcore:iwl_send_cmd_sync+0x67/0x290 > > > > [save_trace+0x3f/0xb0] ? save_trace+0x3f/0xb0 > > > > ... > > > > > > > > Signed-off-by: Joonwoo Park <joonwpark81@gmail.com> > > > > --- > > > > net/wireless/wext.c | 11 ++++++++++- > > > > I'm sure Jean will cry murder because he expects there are some stupid > > full-mac cards that actually support other sizes. > > > > Can't somebody just post a patch to mac80211 that only accepts the two > > correct sizes like cfg80211 does? > > Strawman patch below... You need to allow 0 through, since you can just set the transmit key index via ENCODE without setting the key. So the legal values are 0, 5, and 13. Add 'case 0: /* just setting TX index */' or something and I'll definitely ack it. Dan > --- > > From: John W. Linville <linville@tuxdriver.com> > Subject: [PATCH] mac80211: allow only standard size WEP keys through WEXT > > Limit ieee80211_ioctl_siwencode to only accept standard sized WEP keys. > > Signed-off-by: John W. Linville <linville@tuxdriver.com> > --- > net/mac80211/wext.c | 10 ++++++++++ > 1 files changed, 10 insertions(+), 0 deletions(-) > > diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c > index 5af3862..d16b975 100644 > --- a/net/mac80211/wext.c > +++ b/net/mac80211/wext.c > @@ -26,6 +26,8 @@ > #include "wpa.h" > #include "aes_ccm.h" > > +#define KEY_SIZE_WEP104 13 /* 104/128-bit WEP keys */ > +#define KEY_SIZE_WEP40 5 /* 40/64-bit WEP keys */ > > static int ieee80211_set_encryption(struct net_device *dev, u8 *sta_addr, > int idx, int alg, int remove, > @@ -879,6 +881,14 @@ static int ieee80211_ioctl_siwencode(struct net_device *dev, > u8 bcaddr[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; > int remove = 0; > > + switch (erq->length) { > + case KEY_SIZE_WEP40: > + case KEY_SIZE_WEP104: > + break; > + default: > + return -EINVAL; > + } > + > sdata = IEEE80211_DEV_TO_SUB_IF(dev); > > idx = erq->flags & IW_ENCODE_INDEX; > -- > 1.5.5.1 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-06-27 16:07 ` Dan Williams @ 2008-06-27 16:22 ` Tomas Winkler 2008-06-27 18:10 ` Dan Williams 0 siblings, 1 reply; 18+ messages in thread From: Tomas Winkler @ 2008-06-27 16:22 UTC (permalink / raw) To: Dan Williams Cc: John W. Linville, Johannes Berg, Joonwoo Park, JMF, linux-wireless On Fri, Jun 27, 2008 at 7:07 PM, Dan Williams <dcbw@redhat.com> wrote: > On Fri, 2008-06-27 at 11:28 -0400, John W. Linville wrote: >> On Mon, Jun 16, 2008 at 10:46:29AM +0200, Johannes Berg wrote: >> > >> > > > [PATCH] wireless: Limit wep key size to 128/104-bits >> > > > >> > > > This patch prevents overflow which is occured by invalid long wep key >> > > > insertion >> > > > >> > > > $sudo iwconfig wlan0 enc AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA >> > > > >> > > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 >> > > > IP: [memcpy_c+0xb/0x20] memcpy_c+0xb/0x20 >> > > > PGD 13a590067 PUD 12e471067 PMD 0 >> > > > Oops: 0000 [1] PREEMPT SMP >> > > > CPU 1 >> > > > ... >> > > > Pid: 10, comm: events/1 Not tainted 2.6.26-rc2 #9 >> > > > ... >> > > > Call Trace: >> > > > [iwl4965:iwl4965_rx_scan_start_notif+0xb/0x20] ? :iwl4965:iwl4965_enqueue_hcmd+0x12b/0x220 >> > > > [hci_usb:init_module+0xe97/0x28cb0] :iwlcore:iwl_send_cmd_sync+0x67/0x290 >> > > > [save_trace+0x3f/0xb0] ? save_trace+0x3f/0xb0 >> > > > ... >> > > > >> > > > Signed-off-by: Joonwoo Park <joonwpark81@gmail.com> >> > > > --- >> > > > net/wireless/wext.c | 11 ++++++++++- >> > >> > I'm sure Jean will cry murder because he expects there are some stupid >> > full-mac cards that actually support other sizes. >> > >> > Can't somebody just post a patch to mac80211 that only accepts the two >> > correct sizes like cfg80211 does? >> >> Strawman patch below... > > You need to allow 0 through, since you can just set the transmit key > index via ENCODE without setting the key. So the legal values are 0, 5, > and 13. Add 'case 0: /* just setting TX index */' or something and I'll > definitely ack it. > > Dan > >> --- >> >> From: John W. Linville <linville@tuxdriver.com> >> Subject: [PATCH] mac80211: allow only standard size WEP keys through WEXT >> >> Limit ieee80211_ioctl_siwencode to only accept standard sized WEP keys. >> >> Signed-off-by: John W. Linville <linville@tuxdriver.com> >> --- >> net/mac80211/wext.c | 10 ++++++++++ >> 1 files changed, 10 insertions(+), 0 deletions(-) >> >> diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c >> index 5af3862..d16b975 100644 >> --- a/net/mac80211/wext.c >> +++ b/net/mac80211/wext.c >> @@ -26,6 +26,8 @@ >> #include "wpa.h" >> #include "aes_ccm.h" >> >> +#define KEY_SIZE_WEP104 13 /* 104/128-bit WEP keys */ >> +#define KEY_SIZE_WEP40 5 /* 40/64-bit WEP keys */ >> >> static int ieee80211_set_encryption(struct net_device *dev, u8 *sta_addr, >> int idx, int alg, int remove, >> @@ -879,6 +881,14 @@ static int ieee80211_ioctl_siwencode(struct net_device *dev, >> u8 bcaddr[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; >> int remove = 0; >> >> + switch (erq->length) { >> + case KEY_SIZE_WEP40: >> + case KEY_SIZE_WEP104: >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> sdata = IEEE80211_DEV_TO_SUB_IF(dev); >> >> idx = erq->flags & IW_ENCODE_INDEX; >> -- >> 1.5.5.1 I'm not against this patch just I thought we agreed to support the weird WEP keys in the software. We fixed this in the driver with similar check so now it falls back to software encryption in mac80211. Tomas ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-06-27 16:22 ` Tomas Winkler @ 2008-06-27 18:10 ` Dan Williams 2008-06-27 19:12 ` Johannes Berg 0 siblings, 1 reply; 18+ messages in thread From: Dan Williams @ 2008-06-27 18:10 UTC (permalink / raw) To: Tomas Winkler Cc: John W. Linville, Johannes Berg, Joonwoo Park, JMF, linux-wireless On Fri, 2008-06-27 at 19:22 +0300, Tomas Winkler wrote: > On Fri, Jun 27, 2008 at 7:07 PM, Dan Williams <dcbw@redhat.com> wrote= : > > On Fri, 2008-06-27 at 11:28 -0400, John W. Linville wrote: > >> On Mon, Jun 16, 2008 at 10:46:29AM +0200, Johannes Berg wrote: > >> > > >> > > > [PATCH] wireless: Limit wep key size to 128/104-bits > >> > > > > >> > > > This patch prevents overflow which is occured by invalid lon= g wep key > >> > > > insertion > >> > > > > >> > > > $sudo iwconfig wlan0 enc AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-= AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA-AAAA > >> > > > > >> > > > BUG: unable to handle kernel NULL pointer dereference at 000= 0000000000000 > >> > > > IP: [memcpy_c+0xb/0x20] memcpy_c+0xb/0x20 > >> > > > PGD 13a590067 PUD 12e471067 PMD 0 > >> > > > Oops: 0000 [1] PREEMPT SMP > >> > > > CPU 1 > >> > > > ... > >> > > > Pid: 10, comm: events/1 Not tainted 2.6.26-rc2 #9 > >> > > > ... > >> > > > Call Trace: > >> > > > [iwl4965:iwl4965_rx_scan_start_notif+0xb/0x20] ? :iwl4965:i= wl4965_enqueue_hcmd+0x12b/0x220 > >> > > > [hci_usb:init_module+0xe97/0x28cb0] :iwlcore:iwl_send_cmd_s= ync+0x67/0x290 > >> > > > [save_trace+0x3f/0xb0] ? save_trace+0x3f/0xb0 > >> > > > ... > >> > > > > >> > > > Signed-off-by: Joonwoo Park <joonwpark81@gmail.com> > >> > > > --- > >> > > > net/wireless/wext.c | 11 ++++++++++- > >> > > >> > I'm sure Jean will cry murder because he expects there are some = stupid > >> > full-mac cards that actually support other sizes. > >> > > >> > Can't somebody just post a patch to mac80211 that only accepts t= he two > >> > correct sizes like cfg80211 does? > >> > >> Strawman patch below... > > > > You need to allow 0 through, since you can just set the transmit ke= y > > index via ENCODE without setting the key. So the legal values are = 0, 5, > > and 13. Add 'case 0: /* just setting TX index */' or something and= I'll > > definitely ack it. > > > > Dan > > > >> --- > >> > >> From: John W. Linville <linville@tuxdriver.com> > >> Subject: [PATCH] mac80211: allow only standard size WEP keys throu= gh WEXT > >> > >> Limit ieee80211_ioctl_siwencode to only accept standard sized WEP = keys. > >> > >> Signed-off-by: John W. Linville <linville@tuxdriver.com> > >> --- > >> net/mac80211/wext.c | 10 ++++++++++ > >> 1 files changed, 10 insertions(+), 0 deletions(-) > >> > >> diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c > >> index 5af3862..d16b975 100644 > >> --- a/net/mac80211/wext.c > >> +++ b/net/mac80211/wext.c > >> @@ -26,6 +26,8 @@ > >> #include "wpa.h" > >> #include "aes_ccm.h" > >> > >> +#define KEY_SIZE_WEP104 13 /* 104/128-bit WEP keys */ > >> +#define KEY_SIZE_WEP40 5 /* 40/64-bit WEP keys */ > >> > >> static int ieee80211_set_encryption(struct net_device *dev, u8 *s= ta_addr, > >> int idx, int alg, int remove, > >> @@ -879,6 +881,14 @@ static int ieee80211_ioctl_siwencode(struct n= et_device *dev, > >> u8 bcaddr[ETH_ALEN] =3D { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff= }; > >> int remove =3D 0; > >> > >> + switch (erq->length) { > >> + case KEY_SIZE_WEP40: > >> + case KEY_SIZE_WEP104: > >> + break; > >> + default: > >> + return -EINVAL; > >> + } > >> + > >> sdata =3D IEEE80211_DEV_TO_SUB_IF(dev); > >> > >> idx =3D erq->flags & IW_ENCODE_INDEX; > >> -- > >> 1.5.5.1 >=20 > I'm not against this patch just I thought we agreed to support the > weird WEP keys in the software. We fixed this in the driver with > similar check so now it falls back to software encryption in mac80211= =2E I thought we agreed to ignore > 104/128-bit WEP keys [1][2] until enoug= h people yelled about it (I've heard from 1 or 2 people in the past three years about this for NetworkManager). I'm personally of the opinion that since so few APs support it, it's going to be another codepath that's just not going to get tested becaus= e nobody has the client or AP hardware and nobody cares enough. I don't really care one way or the other; if you guys need to support it for customers that's fine. Dan [1] "=EF=BB=BFWe decided not to care about those right now, but I didn'= t think that would impact TKIP at all" from "=EF=BB=BFRe: iwl4965 oops in 2.6.26-rc5 x86_64" [2] =EF=BB=BFRe: [PATCH] iwlwifi: fix oops on wep key insertion (=EF=BB= =BFMon, 16 Jun 2008 10:30:59 -0400) -- 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-06-27 18:10 ` Dan Williams @ 2008-06-27 19:12 ` Johannes Berg 2008-06-27 23:33 ` Tomas Winkler 0 siblings, 1 reply; 18+ messages in thread From: Johannes Berg @ 2008-06-27 19:12 UTC (permalink / raw) To: Dan Williams Cc: Tomas Winkler, John W. Linville, Joonwoo Park, JMF, linux-wireless [-- Attachment #1: Type: text/plain, Size: 1290 bytes --] > > >> Strawman patch below... > > > > > > You need to allow 0 through, since you can just set the transmit key > > > index via ENCODE without setting the key. So the legal values are 0, 5, > > > and 13. Add 'case 0: /* just setting TX index */' or something and I'll > > > definitely ack it. > > >> + switch (erq->length) { > > >> + case KEY_SIZE_WEP40: > > >> + case KEY_SIZE_WEP104: > > >> + break; > > >> + default: > > >> + return -EINVAL; > > >> + } > > >> + > > >> sdata = IEEE80211_DEV_TO_SUB_IF(dev); > > >> > > >> idx = erq->flags & IW_ENCODE_INDEX; > > >> -- > > >> 1.5.5.1 > > > > I'm not against this patch just I thought we agreed to support the > > weird WEP keys in the software. We fixed this in the driver with > > similar check so now it falls back to software encryption in mac80211. > > I thought we agreed to ignore > 104/128-bit WEP keys [1][2] until enough > people yelled about it (I've heard from 1 or 2 people in the past three > years about this for NetworkManager). I agree, who wants to write the support for that in mac80211. I said that we don't want to catch it in wext itself like an early patch did because some fullmac drivers might support it. johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-06-27 19:12 ` Johannes Berg @ 2008-06-27 23:33 ` Tomas Winkler 0 siblings, 0 replies; 18+ messages in thread From: Tomas Winkler @ 2008-06-27 23:33 UTC (permalink / raw) To: Johannes Berg Cc: Dan Williams, John W. Linville, Joonwoo Park, JMF, linux-wireless On Fri, Jun 27, 2008 at 10:12 PM, Johannes Berg <johannes@sipsolutions.net> wrote: > >> > >> Strawman patch below... >> > > >> > > You need to allow 0 through, since you can just set the transmit key >> > > index via ENCODE without setting the key. So the legal values are 0, 5, >> > > and 13. Add 'case 0: /* just setting TX index */' or something and I'll >> > > definitely ack it. > >> > >> + switch (erq->length) { >> > >> + case KEY_SIZE_WEP40: >> > >> + case KEY_SIZE_WEP104: >> > >> + break; >> > >> + default: >> > >> + return -EINVAL; >> > >> + } >> > >> + >> > >> sdata = IEEE80211_DEV_TO_SUB_IF(dev); >> > >> >> > >> idx = erq->flags & IW_ENCODE_INDEX; >> > >> -- >> > >> 1.5.5.1 >> > >> > I'm not against this patch just I thought we agreed to support the >> > weird WEP keys in the software. We fixed this in the driver with >> > similar check so now it falls back to software encryption in mac80211. >> >> I thought we agreed to ignore > 104/128-bit WEP keys [1][2] until enough >> people yelled about it (I've heard from 1 or 2 people in the past three >> years about this for NetworkManager). > > I agree, who wants to write the support for that in mac80211. I said > that we don't want to catch it in wext itself like an early patch did > because some fullmac drivers might support it. > Fair enough. Actually have a similar patch in my stock. I will send it out. Tomas ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] iwlwifi: fix oops on wep key insertion 2008-05-27 13:53 ` Dan Williams 2008-05-28 0:41 ` John W. Linville @ 2008-05-28 10:14 ` JMF 1 sibling, 0 replies; 18+ messages in thread From: JMF @ 2008-05-28 10:14 UTC (permalink / raw) To: linux-wireless I see... That's the idea I had, WEP greater than 128-bit as an extra, an attempt to add more "security" by some manufacturers. If the wireless drivers and current hardware were made with the standards in mind, then maybe adding support for more than 128-bit WEP may require more work than it deserves :) I have a Dlink router and I don't remember ever seeing 256bit WEP or something like that. When I bought it I didn't know a thing about wireless, but when I read about WEP and TKIP back then, the choice was clear. Maybe not supporting these WEP "modes" will keep more people from using it, preferring better protection instead... ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2008-06-27 23:33 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-05-27 5:13 [PATCH] iwlwifi: fix oops on wep key insertion Joonwoo Park 2008-05-27 6:41 ` Tomas Winkler 2008-05-27 7:10 ` Joonwoo Park 2008-05-27 10:58 ` JMF 2008-05-27 12:41 ` Tomas Winkler 2008-05-27 13:53 ` Dan Williams 2008-05-28 0:41 ` John W. Linville 2008-06-15 16:46 ` Joonwoo Park 2008-06-15 16:53 ` Tomas Winkler 2008-06-16 8:46 ` Johannes Berg 2008-06-16 14:30 ` Dan Williams 2008-06-27 15:28 ` John W. Linville 2008-06-27 16:07 ` Dan Williams 2008-06-27 16:22 ` Tomas Winkler 2008-06-27 18:10 ` Dan Williams 2008-06-27 19:12 ` Johannes Berg 2008-06-27 23:33 ` Tomas Winkler 2008-05-28 10:14 ` JMF
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).