Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] Input: apple_z2 - bound the device-reported finger count
@ 2026-06-14  1:22 Bryam Vargas via B4 Relay
  2026-06-14 12:24 ` Joshua Peisach
  0 siblings, 1 reply; 2+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-06-14  1:22 UTC (permalink / raw)
  To: Sasha Finkelstein, Dmitry Torokhov
  Cc: linux-kernel, Janne Grunau, linux-arm-kernel, linux-input,
	Sven Peter, asahi, Neal Gompa

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>




^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] Input: apple_z2 - bound the device-reported finger count
  2026-06-14  1:22 [PATCH v2] Input: apple_z2 - bound the device-reported finger count Bryam Vargas via B4 Relay
@ 2026-06-14 12:24 ` Joshua Peisach
  0 siblings, 0 replies; 2+ messages in thread
From: Joshua Peisach @ 2026-06-14 12:24 UTC (permalink / raw)
  To: hexlabsecurity, Sasha Finkelstein, Dmitry Torokhov
  Cc: linux-kernel, Janne Grunau, linux-arm-kernel, linux-input,
	Sven Peter, asahi, Neal Gompa

On Sat Jun 13, 2026 at 9:22 PM EDT, 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.
>
> 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,

Reviewed-by: Joshua Peisach <jpeisach@ubuntu.com>


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-06-14 12:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-14  1:22 [PATCH v2] Input: apple_z2 - bound the device-reported finger count Bryam Vargas via B4 Relay
2026-06-14 12:24 ` Joshua Peisach

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox