mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-09 03:29:43 -10:00
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.
This commit is contained in:
1 parent
8de244ccaa
commit
9e840aa0bb
1 file changed
+13
-2
@@ -650,6 +650,19 @@ static void iohidmanager_hid_device_add(IOHIDDeviceRef device, iohidmanager_hid_
|
||||
started (by deterministic method). if so do not re-add the pad */
|
||||
uint32_t device_location_id = iohidmanager_hid_device_get_location_id(device);
|
||||
|
||||
/* Hoist the !hid check above the hid->slots[i] dereference
|
||||
* below. hid_driver_get_data() can return NULL if a device-
|
||||
* match callback fires before iohidmanager_hid_init's
|
||||
* assignment or after iohidmanager_hid_free's teardown.
|
||||
* The existing 'if (!hid) goto error' guard at line ~669
|
||||
* was dead code because the for-loop directly below this
|
||||
* comment reads hid->slots[i].data unconditionally -
|
||||
* NULL-deref on NULL hid happened before we ever reached
|
||||
* the calloc. Fix by bailing out now before any hid
|
||||
* dereference. */
|
||||
if (!hid)
|
||||
return;
|
||||
|
||||
for (i = 0; i < MAX_USERS; i++)
|
||||
{
|
||||
struct iohidmanager_hid_adapter *a = (struct iohidmanager_hid_adapter*)hid->slots[i].data;
|
||||
@@ -666,8 +679,6 @@ static void iohidmanager_hid_device_add(IOHIDDeviceRef device, iohidmanager_hid_
|
||||
|
||||
if (!(adapter = (struct iohidmanager_hid_adapter*)calloc(1, sizeof(*adapter))))
|
||||
return;
|
||||
if (!hid)
|
||||
goto error;
|
||||
|
||||
adapter->handle = device;
|
||||
adapter->locationId = device_location_id;
|
||||
|
||||
Reference in new issue
Block a user