linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mac80211: don't kmalloc 16 bytes
@ 2010-10-10 16:52 Johannes Berg
  2010-10-10 17:50 ` Michael Büsch
  0 siblings, 1 reply; 3+ messages in thread
From: Johannes Berg @ 2010-10-10 16:52 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless@vger.kernel.org

From: Johannes Berg <johannes.berg@intel.com>

Since this small buffer isn't used for DMA,
we can simply allocate it on the stack, it
just needs to be 16 bytes of which only 8
will be used for WEP40 keys.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/wep.c |    8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

--- wireless-testing.orig/net/mac80211/wep.c	2010-10-08 14:50:35.000000000 +0200
+++ wireless-testing/net/mac80211/wep.c	2010-10-08 14:51:41.000000000 +0200
@@ -222,7 +222,7 @@ static int ieee80211_wep_decrypt(struct
 				 struct ieee80211_key *key)
 {
 	u32 klen;
-	u8 *rc4key;
+	u8 rc4key[3 + WLAN_KEY_LEN_WEP104];
 	u8 keyidx;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	unsigned int hdrlen;
@@ -245,10 +245,6 @@ static int ieee80211_wep_decrypt(struct
 
 	klen = 3 + key->conf.keylen;
 
-	rc4key = kmalloc(klen, GFP_ATOMIC);
-	if (!rc4key)
-		return -1;
-
 	/* Prepend 24-bit IV to RC4 key */
 	memcpy(rc4key, skb->data + hdrlen, 3);
 
@@ -260,8 +256,6 @@ static int ieee80211_wep_decrypt(struct
 				       len))
 		ret = -1;
 
-	kfree(rc4key);
-
 	/* Trim ICV */
 	skb_trim(skb, skb->len - WEP_ICV_LEN);
 



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] mac80211: don't kmalloc 16 bytes
  2010-10-10 16:52 [PATCH] mac80211: don't kmalloc 16 bytes Johannes Berg
@ 2010-10-10 17:50 ` Michael Büsch
  2010-10-10 17:52   ` Johannes Berg
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Büsch @ 2010-10-10 17:50 UTC (permalink / raw)
  To: Johannes Berg; +Cc: John Linville, linux-wireless@vger.kernel.org

On Sun, 2010-10-10 at 18:52 +0200, Johannes Berg wrote: 
> From: Johannes Berg <johannes.berg@intel.com>
> 
> Since this small buffer isn't used for DMA,
> we can simply allocate it on the stack, it
> just needs to be 16 bytes of which only 8
> will be used for WEP40 keys.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  net/mac80211/wep.c |    8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> --- wireless-testing.orig/net/mac80211/wep.c	2010-10-08 14:50:35.000000000 +0200
> +++ wireless-testing/net/mac80211/wep.c	2010-10-08 14:51:41.000000000 +0200
> @@ -222,7 +222,7 @@ static int ieee80211_wep_decrypt(struct
>  				 struct ieee80211_key *key)
>  {
>  	u32 klen;
> -	u8 *rc4key;
> +	u8 rc4key[3 + WLAN_KEY_LEN_WEP104];
>  	u8 keyidx;
>  	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
>  	unsigned int hdrlen;
> @@ -245,10 +245,6 @@ static int ieee80211_wep_decrypt(struct
>  
>  	klen = 3 + key->conf.keylen;

What about
if (WARN_ON(klen > sizeof(rc4key)))
return -1;
to harden this a bit for accidental stack overflows?

> 
> -	rc4key = kmalloc(klen, GFP_ATOMIC);
> -	if (!rc4key)
> -		return -1;
> -
>  	/* Prepend 24-bit IV to RC4 key */
>  	memcpy(rc4key, skb->data + hdrlen, 3);
>  
> @@ -260,8 +256,6 @@ static int ieee80211_wep_decrypt(struct
>  				       len))
>  		ret = -1;
>  
> -	kfree(rc4key);
> -
>  	/* Trim ICV */
>  	skb_trim(skb, skb->len - WEP_ICV_LEN);
>  
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Greetings Michael.


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] mac80211: don't kmalloc 16 bytes
  2010-10-10 17:50 ` Michael Büsch
@ 2010-10-10 17:52   ` Johannes Berg
  0 siblings, 0 replies; 3+ messages in thread
From: Johannes Berg @ 2010-10-10 17:52 UTC (permalink / raw)
  To: Michael Büsch; +Cc: John Linville, linux-wireless@vger.kernel.org

On Sun, 2010-10-10 at 19:50 +0200, Michael Büsch wrote:
> On Sun, 2010-10-10 at 18:52 +0200, Johannes Berg wrote: 
> > From: Johannes Berg <johannes.berg@intel.com>
> > 
> > Since this small buffer isn't used for DMA,
> > we can simply allocate it on the stack, it
> > just needs to be 16 bytes of which only 8
> > will be used for WEP40 keys.
> > 
> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> > ---
> >  net/mac80211/wep.c |    8 +-------
> >  1 file changed, 1 insertion(+), 7 deletions(-)
> > 
> > --- wireless-testing.orig/net/mac80211/wep.c	2010-10-08 14:50:35.000000000 +0200
> > +++ wireless-testing/net/mac80211/wep.c	2010-10-08 14:51:41.000000000 +0200
> > @@ -222,7 +222,7 @@ static int ieee80211_wep_decrypt(struct
> >  				 struct ieee80211_key *key)
> >  {
> >  	u32 klen;
> > -	u8 *rc4key;
> > +	u8 rc4key[3 + WLAN_KEY_LEN_WEP104];
> >  	u8 keyidx;
> >  	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> >  	unsigned int hdrlen;
> > @@ -245,10 +245,6 @@ static int ieee80211_wep_decrypt(struct
> >  
> >  	klen = 3 + key->conf.keylen;
> 
> What about
> if (WARN_ON(klen > sizeof(rc4key)))
> return -1;
> to harden this a bit for accidental stack overflows?

Not sure, it doesn't really seem worth it -- if somebody really wanted
to extend mac80211 to support WEP256 he'd also have to find and change
the other places that already contain similar code. Not that it's really
a hotpath though (since 11n doesn't support WEP).

johannes


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-10-10 17:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-10 16:52 [PATCH] mac80211: don't kmalloc 16 bytes Johannes Berg
2010-10-10 17:50 ` Michael Büsch
2010-10-10 17:52   ` Johannes Berg

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).