Re: [PATCH] iwlwifi: fix oops on wep key insertion

[Dan Williams <dcbw@redhat.com>]

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
>