apple-ibridge: fix heap out-of-bounds write in appleib_add_device() (sub_hdevs[] indexed by raw collection index)

[xeeban]

Scope / safety: memory-safety fix in the T1 iBridge path — apple_ibridge binds only USB_DEVICE_ID_APPLE_IBRIDGE (0x8600), so appleib_add_device() never runs on a T2 machine (the T2 Touch Bar 0x8302/0x8102 is handled by separate drivers). It is also behavior-preserving for any well-formed layout, so it is inert / zero-risk for T2 users.

What

appleib_add_device() indexes the fixed-size sub_hdevs[] array — which has only ARRAY_SIZE(appleib_sub_hid_ids) (= 2) slots — by the raw HID collection index i. On the T1's combined display/ALS interface the report descriptor has 7 collections (the ALS, five nested sensor collections, and the Touch Bar display at index 6), so the display's hid_device * is written to sub_hdevs[6] — 24 bytes past the end of the devm-allocated appleib_hid_dev_info. This is a heap out-of-bounds write on every probe / every boot; the corrupted adjacent object then GPFs on any later teardown of the iBridge (driver unload, USB unbind/re-enumerate, or suspend/resume).

This PR indexes sub_hdevs[] by the matched sub-device-id slot instead (dev_id - appleib_sub_hid_ids, {0,1}, in-bounds by construction).

Root cause

for (i = 0; i < hdev->maxcollection; i++) {        /* iterates ALL collections (7 on T1) */
    usage  = hdev->collection[i].usage;
    dev_id = appleib_find_dev_id_for_usage(usage);
    if (!dev_id)
        hid_warn(hdev, "Unknown collection ... usage %x\n", usage);  /* the 5 nested ones */
    else
        hdev_info->sub_hdevs[i] = appleib_add_sub_dev(hdev_info, dev_id);  /* i can be 6 -> OOB */
}

hdev->maxcollection counts every collection, while sub_hdevs[] is only ever sized/iterated as 2 slots everywhere else (appleib_remove_device(), appleib_forward_int_op(), raw-event), so i is the wrong index here.

Proof

A later teardown GPFs with non-canonical "pointers" that are byte-for-byte the first 16 bytes of the live report descriptor (a hid_device's first member is dev_rdesc):

WARNING: ... lib/list_debug.c ... __list_del_entry_valid_or_report+0x...
Oops: general protection fault, probably for non-canonical address 0x25011503160a2005 [#1] SMP
RIP: 0010:remove_nodes.isra.0+0x...
Call Trace:
  hid_destroy_device+0x68/0x80

The change

• appleib_add_device() — index by the matched slot; warn only for unmatched application collections (the nested sensor collections are expected, not errors); guard against a duplicate collection mapping to an already-filled slot; never store an ERR_PTR in the array; clean up the full array on the error path. Correct indexing also means the display sub-device is now destroyed by appleib_remove_device() (it was orphaned at the OOB index → leak + UAF on parent unbind) and the sub_open[] flag lands on the right slot.
• raw-event forwarder — read sub_hdevs[i] once and NULL/IS_ERR-guard it before hid_input_report().

No functional change on the normal path — the same sub-devices register and bind, the bar lights as before, minus the five bogus Unknown collection warnings per boot. (The old code's extra sub-devices at indices ≥ 2 were never touched by any forwarder/remove/open loop — they were pure orphans/leaks.)

Testing

• Hardware: MacBookPro13,2 (T1), Arch Linux, kernels 6.x and 7.0.10.
• Before: every boot logs 5 Unknown collection warnings; any iBridge teardown (echo 0/1 > .../authorized, modprobe -r apple_touchbar) GPFs in remove_nodes()/hid_destroy_device() or D-state-deadlocks modprobe; recovery is a hard reboot.
• After: warnings gone; the same authorized 0→1 power-cycle and a full modprobe -r/reload run clean (no GPF, no D-state, process stays R), repeatedly; Touch Bar + ALS bind and function across reboots.
• Patched module builds clean against 7.0.10 headers (both modules, no new warnings).

Note

This is a generic memory-safety bug — not suspend-specific; that's just one reliable trigger. Full write-up (crash forensics + adversarial review of the fix): https://github.com/xeeban/macbook-t1-linux/blob/main/touch-bar/IBRIDGE-TEARDOWN-UAF-ANALYSIS.md

Credit to @roadrunner2 (Ronald Tschalär) for the original driver.