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

[PATCH v2] 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>
---
Changes since v1 [1]:
- Keep the early-return at NUM_FINGERS_OFFSET instead of moving it to
  FINGERS_OFFSET, so a short zero-finger ("all lifted") report still
  reaches input_mt_sync_frame()/input_sync() and does not leave touches
  stuck on the screen (caught by the sashiko-bot review of v1 [2]).  A
  packet too short to hold even one finger record clamps nfingers to 0
  instead of being dropped.

[1] https://lore.kernel.org/all/20260613-b4-disp-f0148c89-v1-1-868a48b2a187@proton.me/
[2] https://lore.kernel.org/all/20260614000725.6B8D11F000E9@smtp.kernel.org/

Reachable on every touch interrupt once the controller is booted
(apple_z2_irq -> apple_z2_read_packet -> apple_z2_parse_touches).

nfingers is bounded here by the message length; the message length is in
turn bounded by the companion "Input: apple_z2 - bound the device-reported
packet length" change (in flight), which caps the device-reported pkt_len
to the 4000-byte receive buffer.  The two together close the device-driven
out-of-bounds accesses in apple_z2_parse_touches() / apple_z2_read_packet().

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: a zero-finger report (msg_len 19) reaches the
    sync; a 255-finger claim is 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 | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/input/touchscreen/apple_z2.c b/drivers/input/touchscreen/apple_z2.c
index 271ababf0ad5..39ade83ef0de 100644
--- a/drivers/input/touchscreen/apple_z2.c
+++ b/drivers/input/touchscreen/apple_z2.c
@@ -92,6 +92,12 @@ static void apple_z2_parse_touches(struct apple_z2 *z2,
 		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 */
+	if (msg_len < APPLE_Z2_FINGERS_OFFSET)
+		nfingers = 0;
+	else
+		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-4ebcbd68-ed8a28672ccc

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

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

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Unvalidated device-reported packet length leads to a severe heap buffer overflow in `spi_read`. The patch fails to address this root cause.
- [High] Using the `__free(kfree)` attribute on a variable that can hold an `ERR_PTR` causes a kernel panic during error handling.
- [High] Concurrent execution of `apple_z2_read_packet` causes data races on driver state, SPI buffers, and lockless input-mt helper states.
--

commit 9a7b4f864d6502dba7c8e2bed5aa7ed3358d9e4a
Author: Bryam Vargas <hexlabsecurity@proton.me>

Input: apple_z2 - bound the device-reported finger count

The commit adds bounds checking for the device-reported finger count against
the SPI packet length to prevent an out-of-bounds read in
apple_z2_parse_touches().

Closes: 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..39ade83ef0de7 100644
> --- a/drivers/input/touchscreen/apple_z2.c
> +++ b/drivers/input/touchscreen/apple_z2.c
> @@ -92,6 +92,12 @@ static void apple_z2_parse_touches(struct apple_z2 *z2,
>  		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 */
> +	if (msg_len < APPLE_Z2_FINGERS_OFFSET)
> +		nfingers = 0;
> +	else
> +		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) {

[Severity: High]
This is a pre-existing issue, but does this code overflow the heap allocation
for rx_buf?

In 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);

Because rx_buf is allocated with a fixed size of 4000 bytes, couldn't a
malfunctioning device supply a large packet length and cause spi_read to
write past the end of the allocation?

[Severity: High]
This isn't a bug introduced by this patch, but will this cause a kernel panic
on error?

In apple_z2_upload_firmware():

    const u8 *data __free(kfree) =
        apple_z2_build_cal_blob(z2, address, &size);
    if (IS_ERR(data))
        return PTR_ERR(data);

Since kfree does not check for IS_ERR, doesn't returning here cause the
compiler-inserted kfree(data) to attempt freeing an invalid virtual address?

[Severity: High]
This is a pre-existing issue, but can this sequence lead to data races?

If a hardware interrupt arrives immediately after setting z2->booted to true,
could the threaded IRQ handler execute apple_z2_read_packet concurrently?

CPU1 in apple_z2_upload_firmware():
    z2->booted = true;
    apple_z2_read_packet(z2);

CPU2 in apple_z2_irq():
    ...
    else
        apple_z2_read_packet(z2);

Since apple_z2_read_packet modifies shared state without a lock:

apple_z2_read_packet():
    len_cmd->counter = z2->index_parity + 1;
    ...
    z2->index_parity = !z2->index_parity;

wouldn't this concurrent execution corrupt z2->tx_buf, z2->rx_buf, and
z2->index_parity?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260613-b4-disp-4ebcbd68-v2-1-0161acfbd688@proton.me?part=1

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

[Dmitry Torokhov]

Hi Bryam,

On Sat, Jun 13, 2026 at 08:22:51PM -0500, Bryam Vargas via B4 Relay wrote:
> 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.

As Sashiko mentioned, if we do not trust hardware to send valid data we
should also validate that packet size supplied by the device is
reasonable.

Also I wonder why would we want to report some of fingers in case when
device sends bogus number of contacts? I'd drop such packet (maybe
logging ratelimited or "once" message).

You can ignore Sahiko's comment about __free(kfree) not handling error
pointers.

Thanks.

-- 
Dmitry