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
  2026-06-14 20:56 ` Dmitry Torokhov
  0 siblings, 2 replies; 3+ 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] 3+ 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
  2026-06-14 20:56 ` Dmitry Torokhov
  1 sibling, 0 replies; 3+ 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] 3+ 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
@ 2026-06-14 20:56 ` Dmitry Torokhov
  1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Torokhov @ 2026-06-14 20:56 UTC (permalink / raw)
  To: hexlabsecurity
  Cc: Sasha Finkelstein, linux-kernel, Janne Grunau, linux-arm-kernel,
	linux-input, Sven Peter, asahi, Neal Gompa

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


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

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

Thread overview: 3+ 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
2026-06-14 20:56 ` Dmitry Torokhov

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