linux-kernel - Re: [PATCH] input:keyboard: "keycode & KEY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Sat, 23 Nov 2013 10:06:23 -0800
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	andrew.liu200917@...il.com
Cc:	michael.hennerich@...log.com,
	device-drivers-devel@...ckfin.uclinux.org,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-media@...r.kernel.org
Subject: Re: [PATCH] input:keyboard: "keycode & KEY_MAX" changes some keycode
 value.

Hi Andrew,

On Sat, Nov 23, 2013 at 09:45:45PM +0800, andrew.liu200917@...il.com wrote:
> From: Andrew Liu <andrew.liu200917@...il.com>
> 
> For exmaple, keycode: KEY_OK(0x160) is changed by "and" operation with
> KEY_MAX(0x2ff) to KEY_KPENTER(96).
> 
> Signed-off-by: Andrew Liu <andrew.liu200917@...il.com>
> ---
>  drivers/input/keyboard/adp5588-keys.c |    8 ++++++--
>  drivers/input/keyboard/adp5589-keys.c |    8 ++++++--
>  drivers/input/keyboard/bf54x-keys.c   |    8 ++++++--
>  drivers/input/misc/pcf8574_keypad.c   |    6 +++++-
>  drivers/media/pci/ttpci/av7110_ir.c   |    1 +
>  5 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
> index dbd2047..1ce559e 100644
> --- a/drivers/input/keyboard/adp5588-keys.c
> +++ b/drivers/input/keyboard/adp5588-keys.c
> @@ -535,8 +535,12 @@ static int adp5588_probe(struct i2c_client *client,
>  	if (pdata->repeat)
>  		__set_bit(EV_REP, input->evbit);
>  
> -	for (i = 0; i < input->keycodemax; i++)
> -		__set_bit(kpad->keycode[i] & KEY_MAX, input->keybit);
> +	for (i = 0; i < input->keycodemax; i++) {
> +		if (kpad->keycode[i] > KEY_MAX)
> +			kpad->keycode[i] = 0;

There is no need to reset keymap entires to 0, not setting keybit is
enough.

> +		else if (kpad->keycode[i] > KEY_RESERVED)
> +			__set_bit(kpad->keycode[i], input->keybit);
> +	}
>  	__clear_bit(KEY_RESERVED, input->keybit);
>  
>  	if (kpad->gpimapsize)
> diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
> index 67d12b3..22ca2a5 100644
> --- a/drivers/input/keyboard/adp5589-keys.c
> +++ b/drivers/input/keyboard/adp5589-keys.c
> @@ -991,8 +991,12 @@ static int adp5589_probe(struct i2c_client *client,
>  	if (pdata->repeat)
>  		__set_bit(EV_REP, input->evbit);
>  
> -	for (i = 0; i < input->keycodemax; i++)
> -		__set_bit(kpad->keycode[i] & KEY_MAX, input->keybit);
> +	for (i = 0; i < input->keycodemax; i++) {
> +		if (kpad->keycode[i] > KEY_MAX)
> +			kpad->keycode[i] = 0;
> +		else if (kpad->keycode[i] > KEY_RESERVED)
> +			__set_bit(kpad->keycode[i], input->keybit);
> +	}
>  	__clear_bit(KEY_RESERVED, input->keybit);
>  
>  	if (kpad->gpimapsize)
> diff --git a/drivers/input/keyboard/bf54x-keys.c b/drivers/input/keyboard/bf54x-keys.c
> index fc88fb4..96f54e7 100644
> --- a/drivers/input/keyboard/bf54x-keys.c
> +++ b/drivers/input/keyboard/bf54x-keys.c
> @@ -288,8 +288,12 @@ static int bfin_kpad_probe(struct platform_device *pdev)
>  	if (pdata->repeat)
>  		__set_bit(EV_REP, input->evbit);
>  
> -	for (i = 0; i < input->keycodemax; i++)
> -		__set_bit(bf54x_kpad->keycode[i] & KEY_MAX, input->keybit);
> +	for (i = 0; i < input->keycodemax; i++) {
> +		if (bf54x_kpad->keycode[i] > KEY_MAX)
> +			bf54x_kpad->keycode[i] = 0;
> +		else if (bf54x_kpad->keycode[i] > KEY_RESERVED)
> +			__set_bit(bf54x_kpad->keycode[i], input->keybit);
> +	}
>  	__clear_bit(KEY_RESERVED, input->keybit);
>  
>  	error = input_register_device(input);
> diff --git a/drivers/input/misc/pcf8574_keypad.c b/drivers/input/misc/pcf8574_keypad.c
> index e373929..48829f3 100644
> --- a/drivers/input/misc/pcf8574_keypad.c
> +++ b/drivers/input/misc/pcf8574_keypad.c
> @@ -114,8 +114,12 @@ static int pcf8574_kp_probe(struct i2c_client *client, const struct i2c_device_i
>  
>  	for (i = 0; i < ARRAY_SIZE(pcf8574_kp_btncode); i++) {
>  		lp->btncode[i] = pcf8574_kp_btncode[i];
> -		__set_bit(lp->btncode[i] & KEY_MAX, idev->keybit);
> +		if (lp->btncode[i] > KEY_MAX)
> +			lp->btncode[i] = 0;
> +		else if (lp->btncode[i] > KEY_RESERVED)
> +			__set_bit(lp->btncode[i], idev->keybit);
>  	}
> +	__clear_bit(KEY_RESERVED, idev->keybit);
>  
>  	sprintf(lp->name, DRV_NAME);
>  	sprintf(lp->phys, "kp_data/input0");
> diff --git a/drivers/media/pci/ttpci/av7110_ir.c b/drivers/media/pci/ttpci/av7110_ir.c
> index 0e763a7..7fdac45 100644
> --- a/drivers/media/pci/ttpci/av7110_ir.c
> +++ b/drivers/media/pci/ttpci/av7110_ir.c
> @@ -201,6 +201,7 @@ static void input_register_keys(struct infrared *ir)
>  		else if (ir->key_map[i] > KEY_RESERVED)
>  			set_bit(ir->key_map[i], ir->input_dev->keybit);
>  	}
> +	__clear_bit(KEY_RESERVED, ir->input_dev->keybit);

Here we only setting bits for keycodes above KEY_RESERVED so we do not
need to clear KEY_RESERVED bit.

>  
>  	ir->input_dev->keycode = ir->key_map;
>  	ir->input_dev->keycodesize = sizeof(ir->key_map[0]);
> -- 
> 1.7.1
> 

Edited and applied, thank you.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/