All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch] wl3501_cs: min_t() cast truncates high bits
@ 2011-09-26  6:30 ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2011-09-26  6:30 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: John W. Linville, linux-wireless, kernel-janitors

wrqu->encoding.length comes from the network administrator.  It's
size u16.  We want to limit "tocopy" to the smallest value of either
"len_keys", "wrqu->encoding.length" or 100.  But because .length
gets cast from u16 to u8 we might use a random, smaller value than
the was desired.  It's probably not very serious, but we may as well
fix it.

Btw, this is from code auditing and not from testing.  I don't know
if this affects anyone in real life.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/net/wireless/wl3501_cs.c b/drivers/net/wireless/wl3501_cs.c
index 6bc7c92..98fbf54 100644
--- a/drivers/net/wireless/wl3501_cs.c
+++ b/drivers/net/wireless/wl3501_cs.c
@@ -1781,7 +1781,7 @@ static int wl3501_get_encode(struct net_device *dev,
 				  keys, len_keys);
 	if (rc)
 		goto out;
-	tocopy = min_t(u8, len_keys, wrqu->encoding.length);
+	tocopy = min_t(u16, len_keys, wrqu->encoding.length);
 	tocopy = min_t(u8, tocopy, 100);
 	wrqu->encoding.length = tocopy;
 	memcpy(extra, keys, tocopy);

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [patch] wl3501_cs: min_t() cast truncates high bits
@ 2011-09-26  6:30 ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2011-09-26  6:30 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: John W. Linville, linux-wireless, kernel-janitors

wrqu->encoding.length comes from the network administrator.  It's
size u16.  We want to limit "tocopy" to the smallest value of either
"len_keys", "wrqu->encoding.length" or 100.  But because .length
gets cast from u16 to u8 we might use a random, smaller value than
the was desired.  It's probably not very serious, but we may as well
fix it.

Btw, this is from code auditing and not from testing.  I don't know
if this affects anyone in real life.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/net/wireless/wl3501_cs.c b/drivers/net/wireless/wl3501_cs.c
index 6bc7c92..98fbf54 100644
--- a/drivers/net/wireless/wl3501_cs.c
+++ b/drivers/net/wireless/wl3501_cs.c
@@ -1781,7 +1781,7 @@ static int wl3501_get_encode(struct net_device *dev,
 				  keys, len_keys);
 	if (rc)
 		goto out;
-	tocopy = min_t(u8, len_keys, wrqu->encoding.length);
+	tocopy = min_t(u16, len_keys, wrqu->encoding.length);
 	tocopy = min_t(u8, tocopy, 100);
 	wrqu->encoding.length = tocopy;
 	memcpy(extra, keys, tocopy);

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [patch] wl3501_cs: min_t() cast truncates high bits
  2011-09-26  6:30 ` Dan Carpenter
@ 2011-09-26  8:24   ` Johannes Berg
  -1 siblings, 0 replies; 4+ messages in thread
From: Johannes Berg @ 2011-09-26  8:24 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Arnaldo Carvalho de Melo, John W. Linville, linux-wireless,
	kernel-janitors

On Mon, 2011-09-26 at 09:30 +0300, Dan Carpenter wrote:
> wrqu->encoding.length comes from the network administrator.  It's
> size u16.  We want to limit "tocopy" to the smallest value of either
> "len_keys", "wrqu->encoding.length" or 100.  But because .length
> gets cast from u16 to u8 we might use a random, smaller value than
> the was desired.  It's probably not very serious, but we may as well
> fix it.

Nice catch.

> Btw, this is from code auditing and not from testing.  I don't know
> if this affects anyone in real life.

FWIW, it doesn't, the max key length that makes sense is 32 anyway, and
since this is on an output path I doubt the min_t() will ever have done
anything but returned "tocopy" :-)

johannes

> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/net/wireless/wl3501_cs.c b/drivers/net/wireless/wl3501_cs.c
> index 6bc7c92..98fbf54 100644
> --- a/drivers/net/wireless/wl3501_cs.c
> +++ b/drivers/net/wireless/wl3501_cs.c
> @@ -1781,7 +1781,7 @@ static int wl3501_get_encode(struct net_device *dev,
>  				  keys, len_keys);
>  	if (rc)
>  		goto out;
> -	tocopy = min_t(u8, len_keys, wrqu->encoding.length);
> +	tocopy = min_t(u16, len_keys, wrqu->encoding.length);
>  	tocopy = min_t(u8, tocopy, 100);
>  	wrqu->encoding.length = tocopy;
>  	memcpy(extra, keys, tocopy);
> --
> 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
> 



^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [patch] wl3501_cs: min_t() cast truncates high bits
@ 2011-09-26  8:24   ` Johannes Berg
  0 siblings, 0 replies; 4+ messages in thread
From: Johannes Berg @ 2011-09-26  8:24 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Arnaldo Carvalho de Melo, John W. Linville, linux-wireless,
	kernel-janitors

On Mon, 2011-09-26 at 09:30 +0300, Dan Carpenter wrote:
> wrqu->encoding.length comes from the network administrator.  It's
> size u16.  We want to limit "tocopy" to the smallest value of either
> "len_keys", "wrqu->encoding.length" or 100.  But because .length
> gets cast from u16 to u8 we might use a random, smaller value than
> the was desired.  It's probably not very serious, but we may as well
> fix it.

Nice catch.

> Btw, this is from code auditing and not from testing.  I don't know
> if this affects anyone in real life.

FWIW, it doesn't, the max key length that makes sense is 32 anyway, and
since this is on an output path I doubt the min_t() will ever have done
anything but returned "tocopy" :-)

johannes

> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/net/wireless/wl3501_cs.c b/drivers/net/wireless/wl3501_cs.c
> index 6bc7c92..98fbf54 100644
> --- a/drivers/net/wireless/wl3501_cs.c
> +++ b/drivers/net/wireless/wl3501_cs.c
> @@ -1781,7 +1781,7 @@ static int wl3501_get_encode(struct net_device *dev,
>  				  keys, len_keys);
>  	if (rc)
>  		goto out;
> -	tocopy = min_t(u8, len_keys, wrqu->encoding.length);
> +	tocopy = min_t(u16, len_keys, wrqu->encoding.length);
>  	tocopy = min_t(u8, tocopy, 100);
>  	wrqu->encoding.length = tocopy;
>  	memcpy(extra, keys, tocopy);
> --
> 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
> 



^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-09-26  8:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-26  6:30 [patch] wl3501_cs: min_t() cast truncates high bits Dan Carpenter
2011-09-26  6:30 ` Dan Carpenter
2011-09-26  8:24 ` Johannes Berg
2011-09-26  8:24   ` Johannes Berg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.