kernel-janitors.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch 001/001] input: fix read past array bounds
@ 2008-10-05  6:12 Michal Roszkowski
  2008-10-05 16:08 ` walter harms
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Michal Roszkowski @ 2008-10-05  6:12 UTC (permalink / raw)
  To: kernel-janitors


---

Trivial fix for read past end of key_map[] when keycode = NR_KEYS.

--- linux-2.6.26.5/drivers/char/keyboard.c.orig	2008-10-05  
15:51:09.000000000 +1030
+++ linux-2.6.26.5/drivers/char/keyboard.c	2008-10-05  
15:52:17.000000000 +1030
@@ -1247,7 +1247,7 @@ static void kbd_keycode(unsigned int key
		return;
	}

-	if (keycode > NR_KEYS)
+	if (keycode >= NR_KEYS)
		if (keycode >= KEY_BRL_DOT1 && keycode <= KEY_BRL_DOT8)
			keysym = K(KT_BRL, keycode - KEY_BRL_DOT1 + 1);
		else


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

* Re: [patch 001/001] input: fix read past array bounds
  2008-10-05  6:12 [patch 001/001] input: fix read past array bounds Michal Roszkowski
@ 2008-10-05 16:08 ` walter harms
  2008-10-05 22:44 ` Michal Roszkowski
  2008-10-06  7:15 ` walter harms
  2 siblings, 0 replies; 4+ messages in thread
From: walter harms @ 2008-10-05 16:08 UTC (permalink / raw)
  To: kernel-janitors

nice catch,
just to improve readablility .... and reduce the change of an other error ...



 if (keycode < ARRAY_SIZE(key_map) )	
	keysym = key_map[keycode];
 else {
	      if (keycode >= KEY_BRL_DOT1 && keycode <= KEY_BRL_DOT8)
                        keysym = K(KT_BRL, keycode - KEY_BRL_DOT1 + 1);
               else
                       return;
	}



re,
 wh

Michal Roszkowski schrieb:
> 
> ---
> 
> Trivial fix for read past end of key_map[] when keycode = NR_KEYS.
> 
> --- linux-2.6.26.5/drivers/char/keyboard.c.orig    2008-10-05
> 15:51:09.000000000 +1030
> +++ linux-2.6.26.5/drivers/char/keyboard.c    2008-10-05
> 15:52:17.000000000 +1030
> @@ -1247,7 +1247,7 @@ static void kbd_keycode(unsigned int key
>         return;
>     }
> 
> -    if (keycode > NR_KEYS)
> +    if (keycode >= NR_KEYS)
>         if (keycode >= KEY_BRL_DOT1 && keycode <= KEY_BRL_DOT8)
>             keysym = K(KT_BRL, keycode - KEY_BRL_DOT1 + 1);
>         else
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe
> kernel-janitors" 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 001/001] input: fix read past array bounds
  2008-10-05  6:12 [patch 001/001] input: fix read past array bounds Michal Roszkowski
  2008-10-05 16:08 ` walter harms
@ 2008-10-05 22:44 ` Michal Roszkowski
  2008-10-06  7:15 ` walter harms
  2 siblings, 0 replies; 4+ messages in thread
From: Michal Roszkowski @ 2008-10-05 22:44 UTC (permalink / raw)
  To: kernel-janitors

Thanks Walter,
I can't use ARRAY_SIZE because key_map is declared as an unsigned  
short *. I noticed that in vt_ioctl.c the assumption is made that all  
key maps are the same size as plain_map[], so if we were to make the  
same (safe) assumption, we could use the conditional keycode <  
ARRAY_SIZE(plain_map), but I think that that would make the code more  
confusing than the simple test against NR_KEYS.

But here's the patch containing the stylistic suggestions:

--- linux-2.6.26.5/drivers/char/keyboard.c.orig	2008-10-06  
07:19:47.000000000 +1030
+++ linux-2.6.26.5/drivers/char/keyboard.c	2008-10-06  
08:34:48.000000000 +1030
@@ -1247,13 +1247,14 @@ static void kbd_keycode(unsigned int key
  		return;
  	}

-	if (keycode > NR_KEYS)
+	if (keycode < NR_KEYS)
+		keysym = key_map[keycode];
+	else {
  		if (keycode >= KEY_BRL_DOT1 && keycode <= KEY_BRL_DOT8)
  			keysym = K(KT_BRL, keycode - KEY_BRL_DOT1 + 1);
  		else
  			return;
-	else
-		keysym = key_map[keycode];
+	}

  	type = KTYP(keysym);


On 06/10/2008, at 2:38 AM, walter harms wrote:

> nice catch,
> just to improve readablility .... and reduce the change of an other  
> error ...
>
>
>
> if (keycode < ARRAY_SIZE(key_map) )	
> 	keysym = key_map[keycode];
> else {
> 	      if (keycode >= KEY_BRL_DOT1 && keycode <= KEY_BRL_DOT8)
>                        keysym = K(KT_BRL, keycode - KEY_BRL_DOT1 + 1);
>               else
>                       return;
> 	}
>
>
>
> re,
> wh
>
> Michal Roszkowski schrieb:
>>
>> ---
>>
>> Trivial fix for read past end of key_map[] when keycode = NR_KEYS.
>>
>> --- linux-2.6.26.5/drivers/char/keyboard.c.orig    2008-10-05
>> 15:51:09.000000000 +1030
>> +++ linux-2.6.26.5/drivers/char/keyboard.c    2008-10-05
>> 15:52:17.000000000 +1030
>> @@ -1247,7 +1247,7 @@ static void kbd_keycode(unsigned int key
>>        return;
>>    }
>>
>> -    if (keycode > NR_KEYS)
>> +    if (keycode >= NR_KEYS)
>>        if (keycode >= KEY_BRL_DOT1 && keycode <= KEY_BRL_DOT8)
>>            keysym = K(KT_BRL, keycode - KEY_BRL_DOT1 + 1);
>>        else
>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe
>> kernel-janitors" 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 001/001] input: fix read past array bounds
  2008-10-05  6:12 [patch 001/001] input: fix read past array bounds Michal Roszkowski
  2008-10-05 16:08 ` walter harms
  2008-10-05 22:44 ` Michal Roszkowski
@ 2008-10-06  7:15 ` walter harms
  2 siblings, 0 replies; 4+ messages in thread
From: walter harms @ 2008-10-06  7:15 UTC (permalink / raw)
  To: kernel-janitors

thx for checking that.
i did not notice these key_map -> keymaps difference.
If understand the code (key_map = key_maps[shift_final];)
that you could simply use
key_maps[shift_final][keycode]
NTL that would be a totaly different patch.

re,
 wh


Michal Roszkowski schrieb:
> Thanks Walter,
> I can't use ARRAY_SIZE because key_map is declared as an unsigned short
> *. I noticed that in vt_ioctl.c the assumption is made that all key maps
> are the same size as plain_map[], so if we were to make the same (safe)
> assumption, we could use the conditional keycode <
> ARRAY_SIZE(plain_map), but I think that that would make the code more
> confusing than the simple test against NR_KEYS.
> 
> But here's the patch containing the stylistic suggestions:
> 
> --- linux-2.6.26.5/drivers/char/keyboard.c.orig    2008-10-06
> 07:19:47.000000000 +1030
> +++ linux-2.6.26.5/drivers/char/keyboard.c    2008-10-06
> 08:34:48.000000000 +1030
> @@ -1247,13 +1247,14 @@ static void kbd_keycode(unsigned int key
>          return;
>      }
> 
> -    if (keycode > NR_KEYS)
> +    if (keycode < NR_KEYS)
> +        keysym = key_map[keycode];
> +    else {
>          if (keycode >= KEY_BRL_DOT1 && keycode <= KEY_BRL_DOT8)
>              keysym = K(KT_BRL, keycode - KEY_BRL_DOT1 + 1);
>          else
>              return;
> -    else
> -        keysym = key_map[keycode];
> +    }
> 
>      type = KTYP(keysym);
> 
> 
> On 06/10/2008, at 2:38 AM, walter harms wrote:
> 
>> nice catch,
>> just to improve readablility .... and reduce the change of an other
>> error ...
>>
>>
>>
>> if (keycode < ARRAY_SIZE(key_map) )   
>>     keysym = key_map[keycode];
>> else {
>>           if (keycode >= KEY_BRL_DOT1 && keycode <= KEY_BRL_DOT8)
>>                        keysym = K(KT_BRL, keycode - KEY_BRL_DOT1 + 1);
>>               else
>>                       return;
>>     }
>>
>>
>>
>> re,
>> wh
>>
>> Michal Roszkowski schrieb:
>>>
>>> ---
>>>
>>> Trivial fix for read past end of key_map[] when keycode = NR_KEYS.
>>>
>>> --- linux-2.6.26.5/drivers/char/keyboard.c.orig    2008-10-05
>>> 15:51:09.000000000 +1030
>>> +++ linux-2.6.26.5/drivers/char/keyboard.c    2008-10-05
>>> 15:52:17.000000000 +1030
>>> @@ -1247,7 +1247,7 @@ static void kbd_keycode(unsigned int key
>>>        return;
>>>    }
>>>
>>> -    if (keycode > NR_KEYS)
>>> +    if (keycode >= NR_KEYS)
>>>        if (keycode >= KEY_BRL_DOT1 && keycode <= KEY_BRL_DOT8)
>>>            keysym = K(KT_BRL, keycode - KEY_BRL_DOT1 + 1);
>>>        else
>>>
>>> -- 
>>> To unsubscribe from this list: send the line "unsubscribe
>>> kernel-janitors" 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:[~2008-10-06  7:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-05  6:12 [patch 001/001] input: fix read past array bounds Michal Roszkowski
2008-10-05 16:08 ` walter harms
2008-10-05 22:44 ` Michal Roszkowski
2008-10-06  7:15 ` walter harms

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