[patch 2/2] thinkpad_acpi: off by one in adaptive_keyboard_hotkey_notify_hotkey()

[patch 2/2] thinkpad_acpi: off by one in adaptive_keyboard_hotkey_notify_hotkey()

[Dan Carpenter]

This should be >= instead of > because otherwise we read one element
past the end of the hotkey_keycode_map[] array.

The hotkey_keycode_map[] array has TPACPI_HOTKEY_MAP_LEN elements.

Fixes: 6a68d8557084 ('thinkpad_acpi: Add support for more adaptive kbd buttons')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 024861d..7769575 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -3656,8 +3656,9 @@ static bool adaptive_keyboard_hotkey_notify_hotkey(unsigned int scancode)
 		return true;
 
 	default:
-		if (scancode < FIRST_ADAPTIVE_KEY || scancode > FIRST_ADAPTIVE_KEY +
-				TPACPI_HOTKEY_MAP_LEN - ADAPTIVE_KEY_OFFSET) {
+		if (scancode < FIRST_ADAPTIVE_KEY ||
+		    scancode >= FIRST_ADAPTIVE_KEY + TPACPI_HOTKEY_MAP_LEN -
+				ADAPTIVE_KEY_OFFSET) {
 			pr_info("Unhandled adaptive keyboard key: 0x%x\n",
 					scancode);
 			return false;

Re: [patch 2/2] thinkpad_acpi: off by one in adaptive_keyboard_hotkey_notify_hotkey()

[Bastien Nocera]

On Wed, 2015-03-11 at 12:36 +0300, Dan Carpenter wrote:
> This should be >= instead of > because otherwise we read one element 
> past the end of the hotkey_keycode_map[] array.
> 
> The hotkey_keycode_map[] array has TPACPI_HOTKEY_MAP_LEN elements.
> 
> Fixes: 6a68d8557084 ('thinkpad_acpi: Add support for more adaptive 
> kbd buttons')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-By: Bastien Nocera <hadess@hadess.net>

> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index 024861d..7769575 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -3656,8 +3656,9 @@ static bool 
> adaptive_keyboard_hotkey_notify_hotkey(unsigned int scancode)
>                 return true;
>  
>         default:
> -               if (scancode < FIRST_ADAPTIVE_KEY || scancode > 
> FIRST_ADAPTIVE_KEY +
> -                               TPACPI_HOTKEY_MAP_LEN - 
> ADAPTIVE_KEY_OFFSET) {
> +               if (scancode < FIRST_ADAPTIVE_KEY ||
> +                                               scancode >= 
> FIRST_ADAPTIVE_KEY + TPACPI_HOTKEY_MAP_LEN -
> +                               ADAPTIVE_KEY_OFFSET) {
>                         pr_info("Unhandled adaptive keyboard key: 
> 0x%x\n",
>                                         scancode);
>                         return false;

Re: [patch 2/2] thinkpad_acpi: off by one in adaptive_keyboard_hotkey_notify_hotkey()

[Henrique de Moraes Holschuh]

On Wed, Mar 11, 2015, at 06:36, Dan Carpenter wrote:
> +               if (scancode < FIRST_ADAPTIVE_KEY ||
> +                   scancode >= FIRST_ADAPTIVE_KEY +
> TPACPI_HOTKEY_MAP_LEN -
> +                               ADAPTIVE_KEY_OFFSET) {

Meh, I'd prefer that last linebreak to not be there, the 80-char rule is
meant to be ignored when it causes the code to be less readable.

That said,

Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

Thanks for the fix!

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [patch 2/2] thinkpad_acpi: off by one in adaptive_keyboard_hotkey_notify_hotkey()

[Darren Hart]

On Wed, Mar 11, 2015 at 12:36:07PM +0300, Dan Carpenter wrote:
> This should be >= instead of > because otherwise we read one element
> past the end of the hotkey_keycode_map[] array.
> 
> The hotkey_keycode_map[] array has TPACPI_HOTKEY_MAP_LEN elements.
> 
> Fixes: 6a68d8557084 ('thinkpad_acpi: Add support for more adaptive kbd buttons')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Queued, thanks.

-- 
Darren Hart
Intel Open Source Technology Center

Re: [ibm-acpi-devel] [patch 2/2] thinkpad_acpi: off by one in adaptive_keyboard_hotkey_notify_hotkey

[Henrique de Moraes Holschuh]

On Wed, Mar 11, 2015, at 06:36, Dan Carpenter wrote:
> This should be >= instead of > because otherwise we read one element
> past the end of the hotkey_keycode_map[] array.
> 
> The hotkey_keycode_map[] array has TPACPI_HOTKEY_MAP_LEN elements.
> 
> Fixes: 6a68d8557084 ('thinkpad_acpi: Add support for more adaptive kbd
> buttons')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>


-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh