[PATCH] HID: apple: validate feature-report field count to prevent NULL pointer dereference

[PATCH] HID: apple: validate feature-report field count to prevent NULL pointer dereference

[Qasim Ijaz]

A malicious HID device with quirk APPLE_MAGIC_BACKLIGHT can trigger a NULL
pointer dereference whilst the power feature-report is toggled and sent to
the device in apple_magic_backlight_report_set(). The power feature-report
is expected to have two data fields, but if the descriptor declares one
field then accessing field[1] and dereferencing it in
apple_magic_backlight_report_set() becomes invalid
since field[1] will be NULL.

An example of a minimal descriptor which can cause the crash is something
like the following where the report with ID 3 (power report) only
references a single 1-byte field. When hid core parses the descriptor it
will encounter the final feature tag, allocate a hid_report (all members
of field[] will be zeroed out), create field structure and populate it,
increasing the maxfield to 1. The subsequent field[1] access and
dereference causes the crash.

  Usage Page (Vendor Defined 0xFF00)
  Usage (0x0F)
  Collection (Application)
    Report ID (1)
    Usage (0x01)
    Logical Minimum (0)
    Logical Maximum (255)
    Report Size (8)
    Report Count (1)
    Feature (Data,Var,Abs)

    Usage (0x02)
    Logical Maximum (32767)
    Report Size (16)
    Report Count (1)
    Feature (Data,Var,Abs)

    Report ID (3)
    Usage (0x03)
    Logical Minimum (0)
    Logical Maximum (1)
    Report Size (8)
    Report Count (1)
    Feature (Data,Var,Abs)
  End Collection

Here we see the KASAN splat when the kernel dereferences the
NULL pointer and crashes:

  [   15.164723] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000006: 0000 [#1] SMP KASAN NOPTI
  [   15.165691] KASAN: null-ptr-deref in range [0x0000000000000030-0x0000000000000037]
  [   15.165691] CPU: 0 UID: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.15.0 #31 PREEMPT(voluntary) 
  [   15.165691] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
  [   15.165691] RIP: 0010:apple_magic_backlight_report_set+0xbf/0x210
  [   15.165691] Call Trace:
  [   15.165691]  <TASK>
  [   15.165691]  apple_probe+0x571/0xa20
  [   15.165691]  hid_device_probe+0x2e2/0x6f0
  [   15.165691]  really_probe+0x1ca/0x5c0
  [   15.165691]  __driver_probe_device+0x24f/0x310
  [   15.165691]  driver_probe_device+0x4a/0xd0
  [   15.165691]  __device_attach_driver+0x169/0x220
  [   15.165691]  bus_for_each_drv+0x118/0x1b0
  [   15.165691]  __device_attach+0x1d5/0x380
  [   15.165691]  device_initial_probe+0x12/0x20
  [   15.165691]  bus_probe_device+0x13d/0x180
  [   15.165691]  device_add+0xd87/0x1510
  [...]

To fix this issue we should validate the number of fields that the
backlight and power reports have and if they do not have the required
number of fields then bail.

Fixes: 394ba612f941 ("HID: apple: Add support for magic keyboard backlight on T2 Macs")
Cc: stable@vger.kernel.org
Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
---
 drivers/hid/hid-apple.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index ed34f5cd5a91..183229ae5f02 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -890,7 +890,8 @@ static int apple_magic_backlight_init(struct hid_device *hdev)
 	backlight->brightness = report_enum->report_id_hash[APPLE_MAGIC_REPORT_ID_BRIGHTNESS];
 	backlight->power = report_enum->report_id_hash[APPLE_MAGIC_REPORT_ID_POWER];
 
-	if (!backlight->brightness || !backlight->power)
+	if (!backlight->brightness || backlight->brightness->maxfield < 2 ||
+	    !backlight->power || backlight->power->maxfield < 2)
 		return -ENODEV;
 
 	backlight->cdev.name = ":white:" LED_FUNCTION_KBD_BACKLIGHT;
-- 
2.39.5

Re: [PATCH] HID: apple: validate feature-report field count to prevent NULL pointer dereference

[Aditya Garg]

On 14/07/25 5:00 am, Qasim Ijaz wrote:
> A malicious HID device with quirk APPLE_MAGIC_BACKLIGHT can trigger a NULL
> pointer dereference whilst the power feature-report is toggled and sent to
> the device in apple_magic_backlight_report_set(). The power feature-report
> is expected to have two data fields, but if the descriptor declares one
> field then accessing field[1] and dereferencing it in
> apple_magic_backlight_report_set() becomes invalid
> since field[1] will be NULL.
> 
> An example of a minimal descriptor which can cause the crash is something
> like the following where the report with ID 3 (power report) only
> references a single 1-byte field. When hid core parses the descriptor it
> will encounter the final feature tag, allocate a hid_report (all members
> of field[] will be zeroed out), create field structure and populate it,
> increasing the maxfield to 1. The subsequent field[1] access and
> dereference causes the crash.
> 
>   Usage Page (Vendor Defined 0xFF00)
>   Usage (0x0F)
>   Collection (Application)
>     Report ID (1)
>     Usage (0x01)
>     Logical Minimum (0)
>     Logical Maximum (255)
>     Report Size (8)
>     Report Count (1)
>     Feature (Data,Var,Abs)
> 
>     Usage (0x02)
>     Logical Maximum (32767)
>     Report Size (16)
>     Report Count (1)
>     Feature (Data,Var,Abs)
> 
>     Report ID (3)
>     Usage (0x03)
>     Logical Minimum (0)
>     Logical Maximum (1)
>     Report Size (8)
>     Report Count (1)
>     Feature (Data,Var,Abs)
>   End Collection
> 
> Here we see the KASAN splat when the kernel dereferences the
> NULL pointer and crashes:
> 
>   [   15.164723] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000006: 0000 [#1] SMP KASAN NOPTI
>   [   15.165691] KASAN: null-ptr-deref in range [0x0000000000000030-0x0000000000000037]
>   [   15.165691] CPU: 0 UID: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.15.0 #31 PREEMPT(voluntary) 
>   [   15.165691] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
>   [   15.165691] RIP: 0010:apple_magic_backlight_report_set+0xbf/0x210
>   [   15.165691] Call Trace:
>   [   15.165691]  <TASK>
>   [   15.165691]  apple_probe+0x571/0xa20
>   [   15.165691]  hid_device_probe+0x2e2/0x6f0
>   [   15.165691]  really_probe+0x1ca/0x5c0
>   [   15.165691]  __driver_probe_device+0x24f/0x310
>   [   15.165691]  driver_probe_device+0x4a/0xd0
>   [   15.165691]  __device_attach_driver+0x169/0x220
>   [   15.165691]  bus_for_each_drv+0x118/0x1b0
>   [   15.165691]  __device_attach+0x1d5/0x380
>   [   15.165691]  device_initial_probe+0x12/0x20
>   [   15.165691]  bus_probe_device+0x13d/0x180
>   [   15.165691]  device_add+0xd87/0x1510
>   [...]
> 
> To fix this issue we should validate the number of fields that the
> backlight and power reports have and if they do not have the required
> number of fields then bail.
> 
> Fixes: 394ba612f941 ("HID: apple: Add support for magic keyboard backlight on T2 Macs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
> ---

Tested-by: Aditya Garg <gargaditya08@live.com>

Re: [PATCH] HID: apple: validate feature-report field count to prevent NULL pointer dereference

[Orlando Chamberlain]

Hi Qasim,

> On 14 Jul 2025, at 9:31 am, Qasim Ijaz <qasdev00@gmail.com> wrote:
> ...
>  [   15.164723] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000006: 0000 [#1] SMP KASAN NOPTI
>  [   15.165691] KASAN: null-ptr-deref in range [0x0000000000000030-0x0000000000000037]
>  [   15.165691] CPU: 0 UID: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.15.0 #31 PREEMPT(voluntary)
>  [   15.165691] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
>  [   15.165691] RIP: 0010:apple_magic_backlight_report_set+0xbf/0x210
>  [   15.165691] Call Trace:
>  [   15.165691]  <TASK>
>  [   15.165691]  apple_probe+0x571/0xa20
>  [   15.165691]  hid_device_probe+0x2e2/0x6f0
>  [   15.165691]  really_probe+0x1ca/0x5c0
>  [   15.165691]  __driver_probe_device+0x24f/0x310
>  [   15.165691]  driver_probe_device+0x4a/0xd0
>  [   15.165691]  __device_attach_driver+0x169/0x220
>  [   15.165691]  bus_for_each_drv+0x118/0x1b0
>  [   15.165691]  __device_attach+0x1d5/0x380
>  [   15.165691]  device_initial_probe+0x12/0x20
>  [   15.165691]  bus_probe_device+0x13d/0x180
>  [   15.165691]  device_add+0xd87/0x1510
>  [...]
> 
> To fix this issue we should validate the number of fields that the
> backlight and power reports have and if they do not have the required
> number of fields then bail.
> 
> Fixes: 394ba612f941 ("HID: apple: Add support for magic keyboard backlight on T2 Macs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>

I haven't had a chance to test this on my laptop but Aditya has the same Macbook model anyway. As long as this fixes the null deref you got with the spoofed hid device in qemu, this seems fine.

Reviewed-by: Orlando Chamberlain <orlandoch.dev@gmail.com>
> ---
> drivers/hid/hid-apple.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> index ed34f5cd5a91..183229ae5f02 100644
> --- a/drivers/hid/hid-apple.c
> +++ b/drivers/hid/hid-apple.c
> @@ -890,7 +890,8 @@ static int apple_magic_backlight_init(struct hid_device *hdev)
>    backlight->brightness = report_enum->report_id_hash[APPLE_MAGIC_REPORT_ID_BRIGHTNESS];
>    backlight->power = report_enum->report_id_hash[APPLE_MAGIC_REPORT_ID_POWER];
> 
> -    if (!backlight->brightness || !backlight->power)
> +    if (!backlight->brightness || backlight->brightness->maxfield < 2 ||
> +        !backlight->power || backlight->power->maxfield < 2)
>        return -ENODEV;
> 
>    backlight->cdev.name = ":white:" LED_FUNCTION_KBD_BACKLIGHT;
> --
> 2.39.5
> 

Re: (subset) [PATCH] HID: apple: validate feature-report field count to prevent NULL pointer dereference

[Benjamin Tissoires]

On Mon, 14 Jul 2025 00:30:08 +0100, Qasim Ijaz wrote:
> A malicious HID device with quirk APPLE_MAGIC_BACKLIGHT can trigger a NULL
> pointer dereference whilst the power feature-report is toggled and sent to
> the device in apple_magic_backlight_report_set(). The power feature-report
> is expected to have two data fields, but if the descriptor declares one
> field then accessing field[1] and dereferencing it in
> apple_magic_backlight_report_set() becomes invalid
> since field[1] will be NULL.
> 
> [...]

Applied to hid/hid.git (for-6.17/apple), thanks!

[1/1] HID: apple: validate feature-report field count to prevent NULL pointer dereference
      https://git.kernel.org/hid/hid/c/1bb3363da862

Cheers,
-- 
Benjamin Tissoires <bentiss@kernel.org>