[PATCH 0/2] Input: adp5588-keys Please correct and add

[PATCH 0/2] Input: adp5588-keys Please correct and add

[Denis Arefev]

1. Add check on return code
2. Prevent buffer overflow

Denis Arefev (2):
  Input: adp5588-keys Add check on return code
  Input: adp5588-keys Prevent buffer overflow

 drivers/input/keyboard/adp5588-keys.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH 1/2] Input: adp5588-keys Add check on return code

[Denis Arefev]

Function 'adp5588_read()' can return a negative value, which after
calculations will be used as an index to access the array
'kpad->keycode'.

Add a check for the return value.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Denis Arefev <arefev@swemel.ru>
---
 drivers/input/keyboard/adp5588-keys.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
index dc734974ce06..13136f863270 100644
--- a/drivers/input/keyboard/adp5588-keys.c
+++ b/drivers/input/keyboard/adp5588-keys.c
@@ -519,9 +519,14 @@ static void adp5588_report_events(struct adp5588_kpad *kpad, int ev_cnt)
 	int i;
 
 	for (i = 0; i < ev_cnt; i++) {
-		int key = adp5588_read(kpad->client, KEY_EVENTA + i);
-		int key_val = key & KEY_EV_MASK;
-		int key_press = key & KEY_EV_PRESSED;
+		int key, key_val, key_press;
+
+		key = adp5588_read(kpad->client, KEY_EVENTA + i);
+		if (key < 0)
+			continue;
+
+		key_val = key & KEY_EV_MASK;
+		key_press = key & KEY_EV_PRESSED;
 
 		if (key_val >= GPI_PIN_BASE && key_val <= GPI_PIN_END) {
 			/* gpio line used as IRQ source */
-- 
2.43.0

[PATCH 2/2] Input: adp5588-keys Prevent buffer overflow

[Denis Arefev]

If the value of 'key_val' is less than 1 or greater than 80,
a buffer overflow may occur.

Add a check for valid values 'key_val'.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Denis Arefev <arefev@swemel.ru>
---
 drivers/input/keyboard/adp5588-keys.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
index 13136f863270..91f00d6e2413 100644
--- a/drivers/input/keyboard/adp5588-keys.c
+++ b/drivers/input/keyboard/adp5588-keys.c
@@ -164,6 +164,9 @@
 #define KEY_EV_PRESSED		BIT(7)
 #define KEY_EV_MASK		GENMASK(6, 0)
 
+#define KEY_EVENT_MIN		1
+#define KEY_EVENT_MAX		80
+
 #define KP_SEL(x)		(BIT(x) - 1)	/* 2^x-1 */
 
 #define KEYP_MAX_EVENT		10
@@ -531,7 +534,7 @@ static void adp5588_report_events(struct adp5588_kpad *kpad, int ev_cnt)
 		if (key_val >= GPI_PIN_BASE && key_val <= GPI_PIN_END) {
 			/* gpio line used as IRQ source */
 			adp5588_gpio_irq_handle(kpad, key_val, key_press);
-		} else {
+		} else if (key_val >= KEY_EVENT_MIN && key_val <= KEY_EVENT_MAX) {
 			int row = (key_val - 1) / ADP5588_COLS_MAX;
 			int col =  (key_val - 1) % ADP5588_COLS_MAX;
 			int code = MATRIX_SCAN_CODE(row, col, kpad->row_shift);
@@ -542,6 +545,8 @@ static void adp5588_report_events(struct adp5588_kpad *kpad, int ev_cnt)
 
 			input_report_key(kpad->input,
 					 kpad->keycode[code], key_press);
+		} else {
+			dev_err_ratelimited(&kpad->client->dev, "invalid report key value %d", key);
 		}
 	}
 }
-- 
2.43.0

Re: [PATCH 0/2] Input: adp5588-keys Please correct and add

[Nuno Sá]

On Fri, 2025-05-16 at 16:43 +0300, Denis Arefev wrote:
> 1. Add check on return code
> 2. Prevent buffer overflow
> 
> Denis Arefev (2):
>   Input: adp5588-keys Add check on return code
>   Input: adp5588-keys Prevent buffer overflow
> 
>  drivers/input/keyboard/adp5588-keys.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)

Hi,

Thanks for the patch. However, not sure if this is really worth it... This is
driver is in the process of being removed:

https://lore.kernel.org/linux-input/04b8a6d68fdc0c0eadf69fbbc6a130ecc6c49360.camel@gmail.com/T/#mad1980e9652161a6a2e36c2aeeb97f900c6e9fc2

Unless we want somehow to backport these patches?

- Nuno Sá

Re: [PATCH 0/2] Input: adp5588-keys Please correct and add

[Denis Arefev]

> On Fri, 2025-05-16 at 16:43 +0300, Denis Arefev wrote:
>> 1. Add check on return code
>> 2. Prevent buffer overflow
>>
>> Denis Arefev (2):
>>   Input: adp5588-keys Add check on return code
>>   Input: adp5588-keys Prevent buffer overflow
>>
>>  drivers/input/keyboard/adp5588-keys.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> Hi,
>
> Thanks for the patch. However, not sure if this is really worth it... This is
> driver is in the process of being removed:
>
> https://lore.kernel.org/linux-input/04b8a6d68fdc0c0eadf69fbbc6a130ecc6c49360.camel@gmail.com/T/#mad1980e9652161a6a2e36c2aeeb97f900c6e9fc2
>
> Unless we want somehow to backport these patches?
>
> - Nuno Sá

Hi Nuno.

It'd be great if the fix patches are applied and then directed
to stable kernels before the code is dropped from upstream.
I've sent v3 with relevant stable tags included. Thanks.

Re: [PATCH 0/2] Input: adp5588-keys Please correct and add

[Nuno Sá]

On Tue, 2025-05-20 at 14:12 +0300, Denis Arefev wrote:
> > On Fri, 2025-05-16 at 16:43 +0300, Denis Arefev wrote:
> > > 1. Add check on return code
> > > 2. Prevent buffer overflow
> > > 
> > > Denis Arefev (2):
> > >   Input: adp5588-keys Add check on return code
> > >   Input: adp5588-keys Prevent buffer overflow
> > > 
> > >  drivers/input/keyboard/adp5588-keys.c | 18 ++++++++++++++----
> > >  1 file changed, 14 insertions(+), 4 deletions(-)
> > 
> > Hi,
> > 
> > Thanks for the patch. However, not sure if this is really worth it... This
> > is
> > driver is in the process of being removed:
> > 
> > https://lore.kernel.org/linux-input/04b8a6d68fdc0c0eadf69fbbc6a130ecc6c49360.camel@gmail.com/T/#mad1980e9652161a6a2e36c2aeeb97f900c6e9fc2
> > 
> > Unless we want somehow to backport these patches?
> > 
> > - Nuno Sá
> 
> Hi Nuno.
> 
> It'd be great if the fix patches are applied and then directed
> to stable kernels before the code is dropped from upstream.
> I've sent v3 with relevant stable tags included. Thanks.

Not up to me to decide that :)

- Nuno Sá