[PATCH] Input: apple_z2 - bound the device-reported finger count

[PATCH] Input: apple_z2 - bound the device-reported finger count

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

apple_z2_parse_touches() takes the finger count from the touch
controller's report and loops over that many fixed-size finger records
without ever checking the count against the length of the report:

	nfingers = msg[APPLE_Z2_NUM_FINGERS_OFFSET];
	fingers = (struct apple_z2_finger *)(msg + APPLE_Z2_FINGERS_OFFSET);
	for (i = 0; i < nfingers; i++)
		/* read fingers[i] ... */

msg points into the fixed 4000-byte z2->rx_buf and nfingers is a single
device-supplied byte, so it can be as large as 255.  A malicious,
malfunctioning or counterfeit controller (or an interposer on the SPI
bus) can report a large finger count in a short packet, making the loop
read up to 255 * sizeof(struct apple_z2_finger) bytes starting 24 bytes
into msg -- far past the 4000-byte buffer.  This is a controller-driven
heap out-of-bounds read, and the finger fields that are read (position,
pressure, touch and tool dimensions) are forwarded to userspace as input
events, leaking adjacent kernel memory.

Bound the device-reported count to the number of finger records the
report actually carries.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/all/20260613215358.329921F000E9@smtp.kernel.org/
Fixes: 471a92f8a21a ("Input: apple_z2 - add a driver for Apple Z2 touchscreens")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Reachable on every touch interrupt once the controller is booted
(apple_z2_irq -> apple_z2_read_packet -> apple_z2_parse_touches).

nfingers is a different device field from the packet length handled by
the in-flight "Input: apple_z2 - bound the device-reported packet length"
patch, so the two are orthogonal: that one bounds the spi_read() length
(rx_buf[1..2]); this one bounds the per-report finger count (msg[16]).

The early-return is tightened from NUM_FINGERS_OFFSET (16) to
FINGERS_OFFSET (24) so the subtraction below cannot underflow; since
msg_len == pkt_len - 5 and pkt_len is rounded to a multiple of four, the
only reachable lengths the tighter guard now drops are 19 and 23, both of
which are too short to hold even the finger-array header.

Verified with a faithful in-kernel KASAN litmus (the verbatim 4000-byte
buffer, the struct apple_z2_finger layout and the parse loop),
CONFIG_KASAN=y on x86_64:

  Arm A, nfingers = 255 in a short packet (msg_len 19):
    BUG: KASAN: slab-out-of-bounds in apple_z2_parse_touches
    Read of size 2 ... 1 bytes to the right of allocated 4000-byte region
    ... cache kmalloc-4k of size 4096
  Arm B, with this patch (count clamped to what the packet holds): clean
  Arm C, benign device (3 fingers): clean

  AddressSanitizer (x86_64 and i386): heap-buffer-overflow READ, both ABIs.

Reproducer and full logs available on request.
---
 drivers/input/touchscreen/apple_z2.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/apple_z2.c b/drivers/input/touchscreen/apple_z2.c
index 271ababf0ad5..61f353553e7c 100644
--- a/drivers/input/touchscreen/apple_z2.c
+++ b/drivers/input/touchscreen/apple_z2.c
@@ -88,10 +88,13 @@ static void apple_z2_parse_touches(struct apple_z2 *z2,
 	int slot_valid;
 	struct apple_z2_finger *fingers;
 
-	if (msg_len <= APPLE_Z2_NUM_FINGERS_OFFSET)
+	if (msg_len <= APPLE_Z2_FINGERS_OFFSET)
 		return;
 	nfingers = msg[APPLE_Z2_NUM_FINGERS_OFFSET];
 	fingers = (struct apple_z2_finger *)(msg + APPLE_Z2_FINGERS_OFFSET);
+	/* a malicious controller can claim more fingers than the packet holds */
+	nfingers = min_t(int, nfingers,
+			 (msg_len - APPLE_Z2_FINGERS_OFFSET) / sizeof(*fingers));
 	for (i = 0; i < nfingers; i++) {
 		slot = input_mt_get_slot_by_key(z2->input_dev, fingers[i].finger);
 		if (slot < 0) {

---
base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
change-id: 20260613-b4-disp-f0148c89-dfafdfb84b3f

Best regards,
-- 
Bryam Vargas <hexlabsecurity@proton.me>

Re: [PATCH] Input: apple_z2 - bound the device-reported finger count

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The patch incorrectly discards valid zero-finger reports, resulting in stuck touches.

Pre-existing issues:
- [Critical] Unbounded device-reported packet length leads to a massive heap buffer overflow during `spi_read()` and completely bypasses the patch's intended fix for out-of-bounds reads.
--

commit 3a1e80d03b931df0a0db172ea0ca1a4ea3349e47
Author: Bryam Vargas <hexlabsecurity@proton.me>

Input: apple_z2 - bound the device-reported finger count

This commit attempts to fix a heap out-of-bounds read by bounding the number
of reported fingers against the message length.

Link: https://lore.kernel.org/all/20260613215358.329921F000E9@smtp.kernel.org/

> diff --git a/drivers/input/touchscreen/apple_z2.c b/drivers/input/touchscreen/apple_z2.c
> index 271ababf0ad55..61f353553e7c5 100644
> --- a/drivers/input/touchscreen/apple_z2.c
> +++ b/drivers/input/touchscreen/apple_z2.c
> @@ -88,10 +88,13 @@ static void apple_z2_parse_touches(struct apple_z2 *z2,
>  	int slot_valid;
>  	struct apple_z2_finger *fingers;
>  
> -	if (msg_len <= APPLE_Z2_NUM_FINGERS_OFFSET)
> +	if (msg_len <= APPLE_Z2_FINGERS_OFFSET)
>  		return;

[Severity: High]
Will this new check discard valid zero-finger reports?

With SPI length rounding, an empty report indicating all fingers have been
lifted usually lacks the finger array, meaning its payload size can fall
between 17 and 24 bytes.

If a 0-finger packet is dropped here, the input subsystem won't be notified
via input_mt_sync_frame() and input_sync(). Could this result in touches
remaining stuck on the screen?

>  	nfingers = msg[APPLE_Z2_NUM_FINGERS_OFFSET];
>  	fingers = (struct apple_z2_finger *)(msg + APPLE_Z2_FINGERS_OFFSET);
> +	/* a malicious controller can claim more fingers than the packet holds */
> +	nfingers = min_t(int, nfingers,
> +			 (msg_len - APPLE_Z2_FINGERS_OFFSET) / sizeof(*fingers));

[Severity: Critical]
This isn't a bug introduced by this patch, but does deriving msg_len from
the untrusted packet length bypass this new boundary check entirely?

In apple_z2_read_packet(), pkt_len is derived directly from an
unauthenticated SPI read:

apple_z2_read_packet() {
        ...
        pkt_len = (get_unaligned_le16(z2->rx_buf + 1) + 8) & 0xfffffffc;
        error = spi_read(z2->spidev, z2->rx_buf, pkt_len);
        ...
}

Since z2->rx_buf is a fixed 4000-byte allocation, a malicious controller
could send a large length field (up to 65540) to cause a heap buffer
overflow during spi_read().

Furthermore, because msg_len is derived directly from this untrusted
pkt_len, an artificially large pkt_len bypasses the new clamping logic
above. This allows nfingers to still evaluate to 255.

Does this render the out-of-bounds read fix ineffective against malicious
controllers?

>  	for (i = 0; i < nfingers; i++) {
>  		slot = input_mt_get_slot_by_key(z2->input_dev, fingers[i].finger);
>  		if (slot < 0) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260613-b4-disp-f0148c89-v1-1-868a48b2a187@proton.me?part=1