[PATCH] Input: joydev - avoid an undefined shift in the legacy 0x read

[PATCH] Input: joydev - avoid an undefined shift in the legacy 0x read

[Guanglei Zhu]

The legacy 0.x read path shifts the axis values by the calibration
constants that JS_SET_CAL installs:

	data.x = (joydev->abs[0] / 256 + 128) >> joydev->glue.JS_CORR.x;
	data.y = (joydev->abs[1] / 256 + 128) >> joydev->glue.JS_CORR.y;

JS_CORR is copied straight from userspace as three s32 values and
never validated, so the shift count can be negative or wider than
the type, which is undefined behavior: the shift compiles to
whatever the architecture does with an out-of-range count, and the
compiler is free to assume it cannot happen.

Clamp the shift count to the width of the shifted value.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
 drivers/input/joydev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
index 32459fd8a7c1..4740099599bf 100644
--- a/drivers/input/joydev.c
+++ b/drivers/input/joydev.c
@@ -353,8 +353,8 @@ static ssize_t joydev_0x_read(struct joydev_client *client,
 	for (data.buttons = i = 0; i < 32 && i < joydev->nkey; i++)
 		data.buttons |=
 			test_bit(joydev->keypam[i], input->key) ? (1 << i) : 0;
-	data.x = (joydev->abs[0] / 256 + 128) >> joydev->glue.JS_CORR.x;
-	data.y = (joydev->abs[1] / 256 + 128) >> joydev->glue.JS_CORR.y;
+	data.x = (joydev->abs[0] / 256 + 128) >> clamp(joydev->glue.JS_CORR.x, 0, 31);
+	data.y = (joydev->abs[1] / 256 + 128) >> clamp(joydev->glue.JS_CORR.y, 0, 31);
 
 	/*
 	 * Reset reader's event queue
-- 
2.50.1

[PATCH] Input: joydev - avoid signed overflow in the broken-axis correction

[Guanglei Zhu]

The JS_CORR_BROKEN correction multiplies two user-controlled 32-bit
values before shifting and clamping the result:

	value = value > corr->coef[0] ? (value < corr->coef[1] ? 0 :
			((corr->coef[3] * (value - corr->coef[1])) >> 14)) :
			((corr->coef[2] * (value - corr->coef[0])) >> 14);

The coefficients come from JSIOCSCORR without any range check, so
the product can overflow the type, which is undefined behavior: the
compiler is allowed to assume it does not happen and optimize
accordingly.

Do the multiplication in 64 bits, which is bit-identical for every
in-range product.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
 drivers/input/joydev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
index 32459fd8a7c1..947d7671a59f 100644
--- a/drivers/input/joydev.c
+++ b/drivers/input/joydev.c
@@ -74,8 +74,8 @@ static int joydev_correct(int value, struct js_corr *corr)
 
 	case JS_CORR_BROKEN:
 		value = value > corr->coef[0] ? (value < corr->coef[1] ? 0 :
-			((corr->coef[3] * (value - corr->coef[1])) >> 14)) :
-			((corr->coef[2] * (value - corr->coef[0])) >> 14);
+			(((s64)corr->coef[3] * (value - corr->coef[1])) >> 14)) :
+			(((s64)corr->coef[2] * (value - corr->coef[0])) >> 14);
 		break;
 
 	default:
-- 
2.50.1

[PATCH] Input: adi - bound the two-stream packet merge

[Guanglei Zhu]

When a second device shares the wire, adi_move_bits() appends the
second packet to the first one at an offset taken from the
10-bit length field of the ID packet:

	for (i = 1; i <= adi[1].ret; i++)
		adi[0].data[((length - 1) >> 1) + i + 1] = adi[1].data[i];

	adi[0].ret += adi[1].ret;

Nothing compares the merged packet against the size of the buffer:
adi[0].data is the last member of struct adi, and struct adi_port
holds two of them back to back, so an oversized length field makes
the loop write over the second struct adi - its device pointer, its
length and its packet - and the inflated ret later walks the reads
past the buffer as well. The connect path also calls
adi_move_bits() before the length has been validated at all.

Reject a merge that does not fit in the buffer.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
 drivers/input/joystick/adi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/input/joystick/adi.c b/drivers/input/joystick/adi.c
index e07b71978509..03535e58d6a6 100644
--- a/drivers/input/joystick/adi.c
+++ b/drivers/input/joystick/adi.c
@@ -168,6 +168,9 @@ static void adi_move_bits(struct adi_port *port, int length)
 
 	if (adi[0].ret <= 0 || adi[1].ret <= 0) return;
 	if (adi[0].data[0] & 0x20 || ~adi[1].data[0] & 0x20) return;
+	if (adi[0].ret + adi[1].ret > ADI_MAX_LENGTH ||
+	    ((length - 1) >> 1) + adi[1].ret + 1 >= ADI_MAX_LENGTH)
+		return;
 
 	for (i = 1; i <= adi[1].ret; i++)
 		adi[0].data[((length - 1) >> 1) + i + 1] = adi[1].data[i];
-- 
2.50.1

[PATCH] Input: db9 - enlarge the Saturn multitap packet buffer

[Guanglei Zhu]

The Saturn multitap path fills the packet buffer block by block:

	for (i = 0; i < 60; i += 10) {
		data[i] = db9_saturn_read_analog(port, type, powered);
		if (data[i] != 0xff)
			/* read each pad */
			for (j = 0; j < (data[i] & 0x0f); j++)
				data[i + j + 1] = db9_saturn_read_analog(port, type, powered);
	}

Each block is ten bytes apart, and the low nibble of the block
header - taken from the device - allows up to fifteen more bytes,
so the last block can write up to data[65]. The buffer, however,
is sixty bytes, and the six bytes past it belong to the rest of
the stack frame.

Give the buffer the size the protocol layout actually requires.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
 drivers/input/joystick/db9.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/input/joystick/db9.c b/drivers/input/joystick/db9.c
index e5d9b71aab99..73d1e7a296bb 100644
--- a/drivers/input/joystick/db9.c
+++ b/drivers/input/joystick/db9.c
@@ -253,7 +253,8 @@ static unsigned char db9_saturn_read_packet(struct parport *port, unsigned char
 /*
  * db9_saturn_report() analyzes packet and reports.
  */
-static int db9_saturn_report(unsigned char id, unsigned char data[60], struct input_dev *devs[], int n, int max_pads)
+static int db9_saturn_report(unsigned char id, unsigned char data[66],
+			      struct input_dev *devs[], int n, int max_pads)
 {
 	struct input_dev *dev;
 	int tmp, i, j;
@@ -321,7 +322,7 @@ static int db9_saturn_report(unsigned char id, unsigned char data[60], struct in
 
 static int db9_saturn(int mode, struct parport *port, struct input_dev *devs[])
 {
-	unsigned char id, data[60];
+	unsigned char id, data[66];
 	int type, n, max_pads;
 	int tmp, i;
 
-- 
2.50.1

[PATCH] Input: lpc32xx-keys - reject matrices bigger than the hardware has

[Guanglei Zhu]

The scanner state array holds the last state of eight matrix
columns, and the interrupt handler walks the matrix size that the
device tree declared:

	u8 lastkeystates[8];
	...
	for (i = 0; i < kscandat->matrix_sz; i++)
		lpc32xx_mod_states(kscandat, i);

The probe path only checks that rows and columns match; the
hardware supports square matrices from 1x1 up to 8x8, but nothing
stops a device tree from declaring more, which makes the state
array index run past the field into the rest of the driver data -
including the keymap pointer the same code dereferences.

Reject matrices larger than the hardware supports.

Fixes: 69690bec400e ("Input: add support for key scan interface of the LPC32xx SoC")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
 drivers/input/keyboard/lpc32xx-keys.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/input/keyboard/lpc32xx-keys.c b/drivers/input/keyboard/lpc32xx-keys.c
index 285d5c32fe70..1d0b082e7881 100644
--- a/drivers/input/keyboard/lpc32xx-keys.c
+++ b/drivers/input/keyboard/lpc32xx-keys.c
@@ -142,6 +142,11 @@ static int lpc32xx_parse_dt(struct device *dev,
 		return -EINVAL;
 	}
 
+	if (rows > 8) {
+		dev_err(dev, "rows and columns must be at most 8!\n");
+		return -EINVAL;
+	}
+
 	kscandat->matrix_sz = rows;
 	kscandat->row_shift = get_count_order(columns);
 
-- 
2.50.1

[PATCH] Input: mt6779-keypad - check the reported row and column

[Guanglei Zhu]

The interrupt handler turns register bits into a matrix position
and indexes the keycode array with it:

		keypad->calc_row_col(key, &row, &col);

		scancode = MATRIX_SCAN_CODE(row, col, row_shift);
		...
		input_report_key(keypad->input_dev, keycode[scancode], pressed);

The keycode array is sized after the rows and columns the device
tree declared, but neither the probe nor the handler checks that a
reported position actually fits those dimensions. A register glitch
or a matrix smaller than the hardware's fixed layout indexes past
the array and reports whatever sits there as a key code.

Skip positions outside the declared matrix.

Fixes: f28af984e771 ("Input: mt6779-keypad - add MediaTek keypad driver")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
 drivers/input/keyboard/mt6779-keypad.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/input/keyboard/mt6779-keypad.c b/drivers/input/keyboard/mt6779-keypad.c
index e5eb025c5e99..effd154f5bf9 100644
--- a/drivers/input/keyboard/mt6779-keypad.c
+++ b/drivers/input/keyboard/mt6779-keypad.c
@@ -73,6 +73,9 @@ static irqreturn_t mt6779_keypad_irq_handler(int irq, void *dev_id)
 		key = bit_nr / 32 * 16 + bit_nr % 32;
 		keypad->calc_row_col(key, &row, &col);
 
+		if (row >= keypad->n_rows || col >= keypad->n_cols)
+			continue;
+
 		scancode = MATRIX_SCAN_CODE(row, col, row_shift);
 		/* 1: not pressed, 0: pressed */
 		pressed = !test_bit(bit_nr, new_state);
-- 
2.50.1

[PATCH] Input: tegra-kbc - check the pin numbers from the device tree

[Guanglei Zhu]

The row and column pin lists are read straight from the device tree
and used to index the fixed pin configuration array:

	num_rows = of_property_read_variable_u32_array(np, "nvidia,kbc-row-pins",
				rows_cfg, 1, KBC_MAX_GPIO);
	...
	for (i = 0; i < num_rows; i++) {
		kbc->pin_cfg[rows_cfg[i]].type = PIN_CFG_ROW;
		kbc->pin_cfg[rows_cfg[i]].num = i;
	}

The helper only limits how many entries it reads, not what they
contain, and the pin_cfg array holds KBC_MAX_GPIO entries, so a pin
number from the device tree indexes past the array into the rest
of the driver data. The later configuration check validates the
row and column counts stored this way, not the index itself, so it
cannot undo the write.

Reject pin numbers the array does not have.

Fixes: 88390243a993 ("Input: tegra-kbc - add support for rows/columns configuration from dt")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
 drivers/input/keyboard/tegra-kbc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/input/keyboard/tegra-kbc.c b/drivers/input/keyboard/tegra-kbc.c
index e6177f2bdf8d..f086ccf1a075 100644
--- a/drivers/input/keyboard/tegra-kbc.c
+++ b/drivers/input/keyboard/tegra-kbc.c
@@ -522,6 +522,10 @@ static int tegra_kbc_parse_dt(struct tegra_kbc *kbc)
 	}
 
 	for (i = 0; i < num_rows; i++) {
+		if (rows_cfg[i] >= KBC_MAX_GPIO) {
+			dev_err(kbc->dev, "invalid row pin %u\n", rows_cfg[i]);
+			return -EINVAL;
+		}
 		kbc->pin_cfg[rows_cfg[i]].type = PIN_CFG_ROW;
 		kbc->pin_cfg[rows_cfg[i]].num = i;
 	}
@@ -538,6 +542,10 @@ static int tegra_kbc_parse_dt(struct tegra_kbc *kbc)
 	}
 
 	for (i = 0; i < num_cols; i++) {
+		if (cols_cfg[i] >= KBC_MAX_GPIO) {
+			dev_err(kbc->dev, "invalid col pin %u\n", cols_cfg[i]);
+			return -EINVAL;
+		}
 		kbc->pin_cfg[cols_cfg[i]].type = PIN_CFG_COL;
 		kbc->pin_cfg[cols_cfg[i]].num = i;
 	}
-- 
2.50.1

[PATCH] Input: cyapa_gen3 - clamp the reported finger count to the array

[Guanglei Zhu]

The finger count is taken from the top nibble of a status byte the
trackpad firmware fills in, and used to walk the touch array:

	num_fingers = (data->finger_btn >> 4) & 0x0f;
	for (i = 0; i < num_fingers; i++) {
		const struct cyapa_touch *touch = &data->touches[i];

The nibble allows up to fifteen fingers, but the array holds the
five touches the chip is documented to report, so a firmware that
counts beyond that makes the driver report coordinates read from
the stack past the register block.

Clamp the finger count to the size of the array.

Fixes: 9f1cd857518f ("Input: cyapa - re-design driver to support multi-trackpad in one driver")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
 drivers/input/mouse/cyapa_gen3.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/input/mouse/cyapa_gen3.c b/drivers/input/mouse/cyapa_gen3.c
index fc3fb954523b..fda807a4c141 100644
--- a/drivers/input/mouse/cyapa_gen3.c
+++ b/drivers/input/mouse/cyapa_gen3.c
@@ -1149,7 +1149,8 @@ static int cyapa_gen3_event_process(struct cyapa *cyapa,
 	int num_fingers;
 	int i;
 
-	num_fingers = (data->finger_btn >> 4) & 0x0f;
+	num_fingers = min((data->finger_btn >> 4) & 0x0f,
+			  ARRAY_SIZE(data->touches));
 	for (i = 0; i < num_fingers; i++) {
 		const struct cyapa_touch *touch = &data->touches[i];
 		/* Note: touch->id range is 1 to 15; slots are 0 to 14. */
-- 
2.50.1

Re: [PATCH] Input: joydev - avoid signed overflow in the broken-axis correction

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Input: joydev - 32-bit overflow in joydev_correct() subtraction
- [Medium] Input: joydev - s64 truncation before clamp() in joydev_correct()
--

commit 1426ece8bea8b467a9629c0559c54b6693b90b43
Author: Guanglei Zhu <zhugl3@xiaopeng.com>

Input: joydev - avoid signed overflow in the broken-axis correction

This commit casts the coefficient operands in the JS_CORR_BROKEN calculation
in joydev_correct() to s64 to avoid 32-bit signed integer overflow during
multiplication when user-controlled coefficients are set via JSIOCSCORR.

> diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
> index 32459fd8a7c18..947d7671a59f6 100644
> --- a/drivers/input/joydev.c
> +++ b/drivers/input/joydev.c
> @@ -74,8 +74,8 @@ static int joydev_correct(int value, struct js_corr *corr)
>  
>  	case JS_CORR_BROKEN:
>  		value = value > corr->coef[0] ? (value < corr->coef[1] ? 0 :
> -			((corr->coef[3] * (value - corr->coef[1])) >> 14)) :
> -			((corr->coef[2] * (value - corr->coef[0])) >> 14);
> +			(((s64)corr->coef[3] * (value - corr->coef[1])) >> 14)) :
> +			(((s64)corr->coef[2] * (value - corr->coef[0])) >> 14);

[Severity: Medium]
Can the parenthesized subtractions (value - corr->coef[1]) and
(value - corr->coef[0]) in joydev_correct() in drivers/input/joydev.c still
overflow 32-bit signed arithmetic before being promoted to s64 for the
multiplication?

When joydev_ioctl_common() processes JSIOCSCORR or joydev_event() handles an
EV_ABS event and calls joydev_correct(), both value (int) and corr->coef[i]
(__s32) are 32-bit signed integers. In the first branch, where
value > corr->coef[0] and value >= corr->coef[1], if value >= 0 and
corr->coef[0] and corr->coef[1] are large negative values such as INT_MIN,
evaluating value - corr->coef[1] overflows a 32-bit signed int before
promoting to s64. Likewise, in the second branch where value <= corr->coef[0],
if value < 0 and corr->coef[0] is a large positive value such as INT_MAX,
value - corr->coef[0] underflows a 32-bit signed int.

Would casting value to s64 inside the subtraction, such as
(s64)value - corr->coef[1] and (s64)value - corr->coef[0], avoid this
overflow as well?

[Severity: Medium]
What happens in joydev_correct() in drivers/input/joydev.c when the shifted
64-bit product exceeds the 32-bit signed int range before reaching clamp()?

Because two 32-bit signed values multiplied in s64 can have a magnitude up to
roughly 2^62, the result after >> 14 can still have a magnitude up to roughly
2^48. Assigning that s64 expression back into the 32-bit int variable value:

drivers/input/joydev.c:joydev_correct() {
	...
	case JS_CORR_BROKEN:
		value = value > corr->coef[0] ? (value < corr->coef[1] ? 0 :
			(((s64)corr->coef[3] * (value - corr->coef[1])) >> 14)) :
			(((s64)corr->coef[2] * (value - corr->coef[0])) >> 14);
		break;

	default:
		return 0;
	}

	return clamp(value, -32767, 32767);
}

truncates the upper bits before clamp(value, -32767, 32767) is evaluated,
causing large positive or negative results to wrap around rather than saturate
to 32767 or -32767. Should value or the intermediate result be kept as s64
until clamp_val() or clamp_t(s64, ...) is applied?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103084135.426629-2-zhugl3@xiaopeng.com?part=1

[PATCH v2] Input: joydev - avoid signed overflow in the broken-axis correction

[Guanglei Zhu]

The JS_CORR_BROKEN correction multiplies two user-controlled 32-bit
values before shifting and clamping the result:

	value = value > corr->coef[0] ? (value < corr->coef[1] ? 0 :
			((corr->coef[3] * (value - corr->coef[1])) >> 14)) :
			((corr->coef[2] * (value - corr->coef[0])) >> 14);

The coefficients come from JSIOCSCORR without any range check, so
three things can go wrong: the subtractions can overflow 32 bits
(value = 0 against coef[1] = INT_MIN), the products can overflow the
type, and a product that survives the multiplication in 64 bits can
still wrap when the shifted result is assigned back to the int
variable, which then slips through clamp() as a small value instead
of saturating.

Compute the whole correction in 64 bits, including the subtractions,
and clamp with clamp_t(s64, ...) before narrowing, which is
bit-identical for every in-range input.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
v2: per review, extend the 64-bit computation to cover the subtractions
    ((s64)value - coef, which could overflow 32 bits before the
    promotion in v1) and keep the shifted product in s64 through
    clamp_t(s64, ...) so an out-of-range result saturates instead of
    wrapping during the narrowing to int.

 drivers/input/joydev.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
index 32459fd..18aa9ce 100644
--- a/drivers/input/joydev.c
+++ b/drivers/input/joydev.c
@@ -72,11 +72,15 @@ static int joydev_correct(int value, struct js_corr *corr)
 	case JS_CORR_NONE:
 		break;
 
-	case JS_CORR_BROKEN:
-		value = value > corr->coef[0] ? (value < corr->coef[1] ? 0 :
-			((corr->coef[3] * (value - corr->coef[1])) >> 14)) :
-			((corr->coef[2] * (value - corr->coef[0])) >> 14);
+	case JS_CORR_BROKEN: {
+		s64 tmp = value > corr->coef[0] ?
+			(value < corr->coef[1] ? 0 :
+			 (s64)corr->coef[3] * ((s64)value - corr->coef[1])) :
+			(s64)corr->coef[2] * ((s64)value - corr->coef[0]);
+
+		value = clamp_t(s64, tmp >> 14, -32767, 32767);
 		break;
+	}
 
 	default:
 		return 0;
-- 
2.43.0

Re: [PATCH] Input: adi - bound the two-stream packet merge

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Input: adi: off-by-one check and uninvalidated streams in adi_move_bits()
--

commit 72de23854de8740c131cf08cbda4e0025c231b6c
Author: Guanglei Zhu <zhugl3@xiaopeng.com>

Input: adi - bound the two-stream packet merge

This commit adds bounds checks in adi_move_bits() before merging the
second ADI stream into adi[0].data to prevent out-of-bounds reads and
writes when the packet length is oversized.

> diff --git a/drivers/input/joystick/adi.c b/drivers/input/joystick/adi.c
> index e07b71978509b..03535e58d6a60 100644
> --- a/drivers/input/joystick/adi.c
> +++ b/drivers/input/joystick/adi.c
> @@ -168,6 +168,9 @@ static void adi_move_bits(struct adi_port *port, int length)
>  
>  	if (adi[0].ret <= 0 || adi[1].ret <= 0) return;
>  	if (adi[0].data[0] & 0x20 || ~adi[1].data[0] & 0x20) return;
> +	if (adi[0].ret + adi[1].ret > ADI_MAX_LENGTH ||
> +	    ((length - 1) >> 1) + adi[1].ret + 1 >= ADI_MAX_LENGTH)
> +		return;

[Severity: Medium]
Should the first check here use >= ADI_MAX_LENGTH instead of > ADI_MAX_LENGTH,
and should adi[0].ret and adi[1].ret be invalidated before returning?

In struct adi, data is declared as unsigned char data[ADI_MAX_LENGTH], with
index 0 storing the start bit and payload bits stored at 1-based indices
1..ret. In adi_get_bits(), the index is incremented before checking against
adi->ret:

	if ((adi->idx += count) > adi->ret) return 0;
	for (i = 0; i < count; i++)
		bits |= ((adi->data[adi->idx - i] >> 5) & 1) << i;

Because adi[0].ret + adi[1].ret > ADI_MAX_LENGTH allows adi[0].ret to reach
ADI_MAX_LENGTH (256), if adi_decode() consumes 256 bits, adi_get_bits() will
reach adi->idx == adi->ret == 256 and read adi[0].data[256] one byte past the
end of adi[0].data[].

In addition, once adi_move_bits() has verified that the packet is in 2-stream
mode (!(adi[0].data[0] & 0x20) && (adi[1].data[0] & 0x20)), returning early
without setting adi[0].ret = adi[1].ret = -1 leaves both unmerged half-streams
marked valid (ret > 0). Wouldn't that cause adi_connect() and adi_read() to
decode the unmerged half-streams in adi[0] and adi[1] as if they were valid
standalone packets instead of rejecting the malformed 2-stream read?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103084135.426629-3-zhugl3@xiaopeng.com?part=1

[PATCH v2] Input: adi - bound the two-stream packet merge

[Guanglei Zhu]

When a second device shares the wire, adi_move_bits() appends the
second packet to the first one at an offset taken from the
10-bit length field of the ID packet:

	for (i = 1; i <= adi[1].ret; i++)
		adi[0].data[((length - 1) >> 1) + i + 1] = adi[1].data[i];

	adi[0].ret += adi[1].ret;

Nothing compares the merged packet against the size of the buffer:
adi[0].data is the last member of struct adi, and struct adi_port
holds two of them back to back, so an oversized length field makes
the loop write over the second struct adi - its device pointer, its
length and its packet - and the inflated ret later walks the reads
past the buffer as well. The connect path also calls
adi_move_bits() before the length has been validated at all.

Reject a merge that does not fit in the buffer. Use >= for the
combined length so adi_get_bits() can never walk to data[ret] with
ret == ADI_MAX_LENGTH, and invalidate both half-streams on the
rejection so the unmerged packets are not decoded as standalone
reads.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
v2: per review, use >= so a combined ret of ADI_MAX_LENGTH is rejected
    as well (adi_get_bits() allows idx == ret, and data[idx] with
    idx == ADI_MAX_LENGTH reads one byte past the array), and set
    adi[0].ret = adi[1].ret = -1 on the rejection path so the
    unmerged half-streams are not decoded as standalone packets.

 drivers/input/joystick/adi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/input/joystick/adi.c b/drivers/input/joystick/adi.c
index e07b719..41262b7 100644
--- a/drivers/input/joystick/adi.c
+++ b/drivers/input/joystick/adi.c
@@ -168,6 +168,11 @@ static void adi_move_bits(struct adi_port *port, int length)
 
 	if (adi[0].ret <= 0 || adi[1].ret <= 0) return;
 	if (adi[0].data[0] & 0x20 || ~adi[1].data[0] & 0x20) return;
+	if (adi[0].ret + adi[1].ret >= ADI_MAX_LENGTH ||
+	    ((length - 1) >> 1) + adi[1].ret + 1 >= ADI_MAX_LENGTH) {
+		adi[0].ret = adi[1].ret = -1;
+		return;
+	}
 
 	for (i = 1; i <= adi[1].ret; i++)
 		adi[0].data[((length - 1) >> 1) + i + 1] = adi[1].data[i];
-- 
2.43.0

Re: [PATCH] Input: joydev - avoid an undefined shift in the legacy 0x read

[Dmitry Torokhov]

Hi Guanglei,

On Tue, Nov 03, 2026 at 04:41:28PM +0800, Guanglei Zhu wrote:
> The legacy 0.x read path shifts the axis values by the calibration
> constants that JS_SET_CAL installs:
> 
> 	data.x = (joydev->abs[0] / 256 + 128) >> joydev->glue.JS_CORR.x;
> 	data.y = (joydev->abs[1] / 256 + 128) >> joydev->glue.JS_CORR.y;
> 
> JS_CORR is copied straight from userspace as three s32 values and
> never validated, so the shift count can be negative or wider than
> the type, which is undefined behavior: the shift compiles to
> whatever the architecture does with an out-of-range count, and the
> compiler is free to assume it cannot happen.
> 
> Clamp the shift count to the width of the shifted value.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org

Why do all these need to be in stable?

> Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
> ---
>  drivers/input/joydev.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
> index 32459fd8a7c1..4740099599bf 100644
> --- a/drivers/input/joydev.c
> +++ b/drivers/input/joydev.c
> @@ -353,8 +353,8 @@ static ssize_t joydev_0x_read(struct joydev_client *client,
>  	for (data.buttons = i = 0; i < 32 && i < joydev->nkey; i++)
>  		data.buttons |=
>  			test_bit(joydev->keypam[i], input->key) ? (1 << i) : 0;
> -	data.x = (joydev->abs[0] / 256 + 128) >> joydev->glue.JS_CORR.x;
> -	data.y = (joydev->abs[1] / 256 + 128) >> joydev->glue.JS_CORR.y;
> +	data.x = (joydev->abs[0] / 256 + 128) >> clamp(joydev->glue.JS_CORR.x, 0, 31);
> +	data.y = (joydev->abs[1] / 256 + 128) >> clamp(joydev->glue.JS_CORR.y, 0, 31);
>  
>  	/*
>  	 * Reset reader's event queue

Thanks.

-- 
Dmitry

Re: [PATCH] Input: lpc32xx-keys - reject matrices bigger than the hardware has

[Dmitry Torokhov]

On Tue, Nov 03, 2026 at 04:41:32PM +0800, Guanglei Zhu wrote:
> The scanner state array holds the last state of eight matrix
> columns, and the interrupt handler walks the matrix size that the
> device tree declared:
> 
> 	u8 lastkeystates[8];
> 	...
> 	for (i = 0; i < kscandat->matrix_sz; i++)
> 		lpc32xx_mod_states(kscandat, i);
> 
> The probe path only checks that rows and columns match; the
> hardware supports square matrices from 1x1 up to 8x8, but nothing
> stops a device tree from declaring more, which makes the state
> array index run past the field into the rest of the driver data -
> including the keymap pointer the same code dereferences.
> 
> Reject matrices larger than the hardware supports.
> 
> Fixes: 69690bec400e ("Input: add support for key scan interface of the LPC32xx SoC")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
> ---
>  drivers/input/keyboard/lpc32xx-keys.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/input/keyboard/lpc32xx-keys.c b/drivers/input/keyboard/lpc32xx-keys.c
> index 285d5c32fe70..1d0b082e7881 100644
> --- a/drivers/input/keyboard/lpc32xx-keys.c
> +++ b/drivers/input/keyboard/lpc32xx-keys.c
> @@ -142,6 +142,11 @@ static int lpc32xx_parse_dt(struct device *dev,
>  		return -EINVAL;
>  	}
>  
> +	if (rows > 8) {

This should be a constant/#define.

> +		dev_err(dev, "rows and columns must be at most 8!\n");
> +		return -EINVAL;
> +	}
> +
>  	kscandat->matrix_sz = rows;
>  	kscandat->row_shift = get_count_order(columns);

Thanks.

-- 
Dmitry