Commit 9e840aa
committed
input/drivers_hid/iohidmanager_hid: hoist !hid NULL-check above hid->slots dereference
iohidmanager_hid_device_add had a dead-code NULL-guard:
for (i = 0; i < MAX_USERS; i++)
{
struct iohidmanager_hid_adapter *a =
(struct iohidmanager_hid_adapter*)hid->slots[i].data;
...
}
if (!(adapter = (struct iohidmanager_hid_adapter*)calloc(
1, sizeof(*adapter))))
return;
if (!hid)
goto error;
The 'if (!hid) goto error' guard only fires after the
for-loop has already unconditionally dereferenced hid via
hid->slots[i].data. On NULL hid the function segfaults at
the slots lookup; the later guard is unreachable.
=== Reachability ===
iohidmanager_hid_device_matched is the sole caller and fetches
hid via:
iohidmanager_hid_t *hid =
(iohidmanager_hid_t*) hid_driver_get_data();
iohidmanager_hid_device_add(device, hid);
hid_driver_get_data() can return NULL in two known windows:
1. Early init: IOHIDManager matching callbacks can fire
before iohidmanager_hid_init's return stores the hid
pointer in the global driver state. A device plugged
in (or already present) at startup triggers the race.
2. Teardown: iohidmanager_hid_free nulls the global before
tearing down IOKit subscriptions, so a matching-callback
dispatched just before the unsubscribe fires with NULL
hid.
Both windows are small but real - especially on systems where
lots of HID devices enumerate at startup (USB hubs, Bluetooth
adapters), the first window is reliably reachable.
=== Fix ===
Hoist the !hid check to the top of iohidmanager_hid_device_add,
immediately after the local variable declarations and the
device_location_id read. Remove the now-redundant later guard.
On NULL hid we return early without ever touching hid->slots or
allocating an adapter - a cleaner outcome than the previous
segfault and a strictly simpler state to reason about than the
old two-stage check (which would have leaked an allocated
adapter through goto-error on NULL hid if the dereference
crash had somehow not happened first).
=== Adapter freshness on goto error ===
The error: label at line ~946 walks adapter->hats /
adapter->axes / adapter->buttons linked lists. These reads are
safe because adapter is always calloc'd fresh (zero-filled)
before any goto error fires from this function: the four
earlier goto-error sites at the time of writing (IOHIDDeviceOpen
failure, element enumeration failures, etc.) all run after the
calloc has succeeded, so adapter is non-NULL. No change needed
there.
=== Scope ===
Single-file change. Behaviour under valid (non-NULL) hid is
unchanged. Behaviour under NULL hid goes from segfault to
silent skip of the device-add - which matches the behaviour
when the same device has already been registered (the earlier
early-return at the top of the function).
Thread-safety: IOHIDManager callbacks run on the run loop
thread, typically the main thread. No concurrent mutation of
hid itself is introduced by this change - we still rely on
the caller (hid_driver_get_data) to return a consistent
snapshot of the global.1 parent 8de244c commit 9e840aa
1 file changed
Lines changed: 13 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
650 | 650 | | |
651 | 651 | | |
652 | 652 | | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
653 | 666 | | |
654 | 667 | | |
655 | 668 | | |
| |||
666 | 679 | | |
667 | 680 | | |
668 | 681 | | |
669 | | - | |
670 | | - | |
671 | 682 | | |
672 | 683 | | |
673 | 684 | | |
| |||
0 commit comments