* Re: [PATCH] HID: usbhid: skip interrupt IN polling for devices with no input reports
From: Denis Benato @ 2026-06-06 12:42 UTC (permalink / raw)
To: Antheas Kapenekakis, Ahmed Yaseen
Cc: Jiri Kosina, Benjamin Tissoires, Ilpo Järvinen,
Kerim Kabirov, GameBurrow, linux-usb, linux-input, linux-kernel
In-Reply-To: <CAGwozwFWbW=v2B7ruS4OUGuPhjSayw4Qxj3K+bCzwmgQu158Ng@mail.gmail.com>
On 6/5/26 14:02, Antheas Kapenekakis wrote:
> On Fri, 5 Jun 2026 at 13:40, Ahmed Yaseen <yaseen@ghoul.dev> wrote:
>> usbhid starts polling a device's interrupt IN endpoint on open
>> (usbhid_open() -> hid_start_in()). If the report descriptor declares no
>> input reports there is nothing to read there, so the poll is useless,
>> and on some composite devices it is also harmful.
> If it did have input reports, would starting the polling still cause
> issues? Because if it would, the issue is in the polling itself.
So far we haven't found an asus device that has more than one interface
that supports reading data out of if.
> Given the creativity of manufacturers when implementing hid protocols,
> I find it certain that they do use the in endpoint even without input
> reports. E.g., for feature reports. This could cause regressions.
While I mostly agree with this it is also true that the general direction
for the kernel (especially lately) has been to not do out-of-spec things
at least by default.
If things really regress it's expected to do so only an very few specific
devices with a buggy firmware, and we can think of something different
for those (hopefully very few ones).
Perhaps someone concerned with security might be interested in what
we have because it doesn't look very normal.
Note that below I have written a few ideas that maybe are worth
looking into.
>> The ASUS ROG N-Key keyboards expose a second, input-less interface used
>> only for RGB control via feature reports. Opening its hidraw node (any
>> hidraw reader does, including SDL/Steam Input or a plain cat) starts the
> cating a hidraw causing issues would be expected, so let's focus on the former.
Simply opening an hidraw should not trigger a delayed disconnect of that device,
I don't know why you would expect this to happen nor why you would
consider it acceptable. It's a bug.
Focusing on userspace software exposing the bug is not a realistic option
because over the time we found a good chunk of software doing that:
- logitech control software (forgot the name)
- open razer software
- sdl
- asusctl (obviously it opens the device albeit in the future I will change this)
and likely more given the fact not all software was identified.
> Asusctl has a bug where if you add the quirk that separates the event
> nodes per hid, this bug is reproduced as well. I chucked it to
> complicated threading getting out of control. It is the reason we
> skipped that patch that was in my series.
I found and solved the bug already. Regardless the issue remains:
Even with no asusctl at all, if a user has one logitech mouse
(and its control software) and a razer keyboard (and its control software)
the asus N-Key device will start an endless disconnect-reconnect loop.
Any combination of two or more of those tools will trigger the issue
on some devices (weirdly enough not every model is affected):
this is not good.
> Now, you say SDL/Steam do a spurious read as well, can you identify
> the codepath so we can look into it? What devices are affected? The
> early return fixes a warning on the Z13, but it also feeds through the
> universal lamp interface on the new Xbox Allies. Is this a bug on
> those devices or keyboards? If yes, it could be caused by userspace
> hanging on that node
Sure, and I agree with you that fixing all userspace tools is desirable
but it's also unfeasible to fix them all, if we managed to do that
there will be years before everyone receives a fixed version of every
affected software and even then a core issue would remain:
linux tries to poll something it can't have anything out from.
I am much more oriented on the fact that kernel shouldn't
be doing weird things (at least not by default) so this has to
somehow be stopped regardless of how well userspace behaves.
If you have better ideas on how to fix the kernel we would
like to hear those as well.
Best regards,
Denis
> Antheas
>
>> pointless IN poll and keypress reports on the keyboard interface get
>> dropped for as long as the node stays open: a lost key-down drops a
>> letter, a lost key-up leaves the key stuck. usbmon shows the dropped
>> reports never reach the URB layer.
>>
>> The useless poll itself is long-standing; commit 4ac74ea68f64 ("HID:
>> asus: early return for ROG devices") is what exposes it on these
>> devices by keeping the input-less interface alive instead of ejecting
>> it, so its hidraw node can be opened and the poll started.
>>
>> Skip the poll in usbhid_open() when the device has no input reports.
>> Feature reports and hidraw output keep working over the control and OUT
>> endpoints, so the interface is otherwise unaffected.
I will write my review here to avoid forking the discussion:
I agree with the general idea but perhaps we can avoid
some hid devices to ever get HID_QUIRK_ALWAYS_POLL
and that might be enough to skip the problematic code?
Maybe there is value in doing this with a quirk flag in hid-asus.c
affecting the least amount of devices?
Or maybe just prevent devices with no data possibly coming out
to ever get HID_QUIRK_ALWAYS_POLL?
For how to best do this we will need to hear what Jiri and
Benjamin have to say but if they think the proposed solution
is the correct solution:
Reviewed-by: Denis Benato <denis.benato@linux.dev>
>> Fixes: 4ac74ea68f64 ("HID: asus: early return for ROG devices")
>> Tested-by: Kerim Kabirov <the.privat33r+linux@pm.me>
>> Tested-by: GameBurrow <gameburrow@pm.me>
>> Signed-off-by: Ahmed Yaseen <yaseen@ghoul.dev>
>> ---
>> drivers/hid/usbhid/hid-core.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
>> index 96b0181cf819..90a8b34d9305 100644
>> --- a/drivers/hid/usbhid/hid-core.c
>> +++ b/drivers/hid/usbhid/hid-core.c
>> @@ -688,7 +688,8 @@ static int usbhid_open(struct hid_device *hid)
>>
>> set_bit(HID_OPENED, &usbhid->iofl);
>>
>> - if (hid->quirks & HID_QUIRK_ALWAYS_POLL) {
>> + if ((hid->quirks & HID_QUIRK_ALWAYS_POLL) ||
>> + list_empty(&hid->report_enum[HID_INPUT_REPORT].report_list)) {
>> res = 0;
>> goto Done;
>> }
>> --
>> 2.54.0
>>
>>
>>
^ permalink raw reply
* Re: [PATCH 7/7] iio: pressure: hid-sensor-press: use u32 instead of unsigned
From: sashiko-bot @ 2026-06-06 12:32 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-7-dd4a6820b674@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Unaligned memory access and out-of-bounds read in `press_capture_sample` due to unconditional pointer cast and ignoring `raw_len`.
- [High] Stale software timestamp reused for all subsequent samples if the device lacks hardware timestamps.
--
commit a1f5a54c679fcb699d59de1d79aadcc9a8177858
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: pressure: hid-sensor-press: use u32 instead of unsigned
Prefer 'u32' instead of bare 'unsigned' variable to improve code
clarity and consistency with kernel style.
> diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
> index a039b99d9851d..3e47a10d72a89 100644
> --- a/drivers/iio/pressure/hid-sensor-press.c
> +++ b/drivers/iio/pressure/hid-sensor-press.c
> @@ -155,7 +155,7 @@ static const struct iio_info press_info = {
>
> /* Callback handler to send event after all samples are received and captured */
> static int press_proc_event(struct hid_sensor_hub_device *hsdev,
> - unsigned usage_id,
> + u32 usage_id,
> void *priv)
> {
[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential problem
with how timestamps are handled in press_proc_event().
When a hardware timestamp isn't provided, the driver generates a software
timestamp:
if (atomic_read(&press_state->common_attributes.data_ready)) {
if (!press_state->timestamp)
press_state->timestamp = iio_get_time_ns(indio_dev);
iio_push_to_buffers_with_ts(indio_dev, &press_state->scan,
sizeof(press_state->scan),
press_state->timestamp);
}
Because press_state->timestamp is never cleared back to 0 after pushing the
buffer, will all subsequent samples incorrectly reuse the exact same timestamp
instead of generating a new one?
[ ... ]
> @@ -176,7 +176,7 @@ static int press_proc_event(struct hid_sensor_hub_device *hsdev,
>
> /* Capture samples in local storage */
> static int press_capture_sample(struct hid_sensor_hub_device *hsdev,
> - unsigned usage_id,
> + u32 usage_id,
> size_t raw_len, char *raw_data,
> void *priv)
> {
[Severity: High]
This is a pre-existing issue, but could this cause unaligned accesses or
out-of-bounds reads?
Looking at press_capture_sample(), raw_data is cast directly to a pointer
without using the unaligned access helpers:
case HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE:
press_state->scan.press_data = *(u32 *)raw_data;
ret = 0;
break;
case HID_USAGE_SENSOR_TIME_TIMESTAMP:
press_state->timestamp = hid_sensor_convert_timestamp(
&press_state->common_attributes, *(s64 *)raw_data);
Since raw_data points into a HID report, it might not be aligned, which could
cause undefined behavior on strictly aligned architectures.
Additionally, we're unconditionally reading 4 or 8 bytes here without checking
raw_len. If the HID descriptor specifies a smaller field size, could this read
past the end of the valid data buffer?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com?part=7
^ permalink raw reply
* Re: [PATCH 5/7] iio: orientation: hid-sensor-incl-3d: use u32 instead of unsigned
From: sashiko-bot @ 2026-06-06 12:30 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-5-dd4a6820b674@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Out-of-bounds read and information leak in `incl_3d_capture_sample` due to ignored `raw_len`.
- [High] Endianness mismatch in `incl_3d_capture_sample` due to direct pointer cast.
- [High] Unaligned memory access in `incl_3d_capture_sample`.
--
commit bb48c83224153ec8c20c69c57c293183eb5211a7
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: orientation: hid-sensor-incl-3d: use u32 instead of unsigned
This patch updates the hid-sensor-incl-3d driver to use u32 instead of a
bare unsigned type in callback signatures, improving code clarity and
consistency.
> diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c
> index 4e23a598a3fb9..c7fbff498be7a 100644
> --- a/drivers/iio/orientation/hid-sensor-incl-3d.c
> +++ b/drivers/iio/orientation/hid-sensor-incl-3d.c
[ ... ]
> @@ -212,7 +212,7 @@ static int incl_3d_proc_event(struct hid_sensor_hub_device *hsdev,
>
> /* Capture samples in local storage */
> static int incl_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
> - unsigned usage_id,
> + u32 usage_id,
> size_t raw_len, char *raw_data,
> void *priv)
> {
[Severity: High]
This is a pre-existing issue, but by ignoring raw_len in
incl_3d_capture_sample(), could we read out-of-bounds if a malicious HID
device provides a report descriptor with a smaller field size?
It appears we unconditionally cast char *raw_data to u32 * or s64 * and
dereference it:
drivers/iio/orientation/hid-sensor-incl-3d.c:incl_3d_capture_sample() {
...
case HID_USAGE_SENSOR_ORIENT_TILT_X:
incl_state->scan.incl_val[CHANNEL_SCAN_INDEX_X] = *(u32 *)raw_data;
...
}
If the field is placed at the end of the HID report buffer, this might
read past the allocated buffer and leak kernel heap memory into the
sensor readings.
[Severity: High]
This is a pre-existing issue, but does this direct cast also cause
incorrect sensor values on big-endian architectures?
The USB HID protocol specifies that report data is strictly little-endian,
but casting directly to u32 * and dereferencing it uses the CPU native
byte order.
[Severity: High]
This is a pre-existing issue, but can this dereference trigger alignment
exceptions on architectures that do not support unaligned accesses?
Since the pointer passed as raw_data is iteratively incremented by the
byte sizes of preceding fields, it might not be aligned to a 4-byte
boundary when we cast it to u32 * and dereference it.
Should we be extracting these values safely using helpers like
get_unaligned_le32() instead?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com?part=5
^ permalink raw reply
* Re: [PATCH 1/7] iio: gyro: hid-sensor-gyro-3d: use u32 instead of unsigned
From: sashiko-bot @ 2026-06-06 12:28 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-1-dd4a6820b674@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The capture_sample callback casts the `raw_data` pointer to `u32*` and `s64*` without validating `raw_len` or pointer alignment, potentially causing out-of-bounds reads, kernel memory leaks, and unaligned access faults.
--
commit e250748ae8811d708c463dcef4e525db238816e3
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: gyro: hid-sensor-gyro-3d: use u32 instead of unsigned
This commit replaces bare unsigned variables with the u32 type in the HID
sensor 3D gyro driver to improve code clarity and adhere to kernel style
guidelines.
> diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> index e48c25c87b6d6..bbca2111e79bb 100644
> --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
[ ... ]
> @@ -199,7 +199,7 @@ static int gyro_3d_proc_event(struct hid_sensor_hub_device *hsdev,
>
> /* Capture samples in local storage */
> static int gyro_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
> - unsigned usage_id,
> + u32 usage_id,
> size_t raw_len, char *raw_data,
> void *priv)
> {
[Severity: High]
This is a pre-existing issue, but looking at the rest of this function, it
appears we cast the raw_data pointer directly to u32* and s64* without checking
raw_len or alignment:
gyro_3d_capture_sample() {
...
gyro_state->scan.gyro_val[CHANNEL_SCAN_INDEX_X + offset] =
*(u32 *)raw_data;
...
gyro_state->timestamp =
hid_sensor_convert_timestamp(&gyro_state->common_attributes,
*(s64 *)raw_data);
...
}
Could a buggy or malicious HID device supply a field size smaller than 4 or 8
bytes, leading to an out-of-bounds read that leaks kernel heap data into the
IIO buffer?
Also, since raw_data is only guaranteed to be byte-aligned, could these native
casts trigger unaligned access exceptions on architectures that do not support
unaligned loads? Would it be safer to validate raw_len and use get_unaligned()
or memcpy() here?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 3/7] iio: light: hid-sensor-als: use u32 instead of unsigned
From: sashiko-bot @ 2026-06-06 12:28 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-3-dd4a6820b674@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Unconditional casting and dereferencing of `raw_data` to `u32*` and `s64*` without checking `raw_len` causes out-of-bounds reads, kernel memory disclosure, unaligned access exceptions, and incorrect endianness handling.
--
commit 7ad652d49fe4e7dcab137b42945a51bbeaea25a3
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: light: hid-sensor-als: use u32 instead of unsigned
This commit updates the HID sensor ALS driver to use 'u32' instead of a bare
'unsigned' variable for the usage_id parameter. This improves code clarity
and consistency with kernel style.
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
[ ... ]
> /* Capture samples in local storage */
> static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
> - unsigned usage_id,
> + u32 usage_id,
> size_t raw_len, char *raw_data,
> void *priv)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(priv);
> struct als_state *als_state = iio_priv(indio_dev);
> int ret = -EINVAL;
> u32 sample_data = *(u32 *)raw_data;
[Severity: High]
This isn't a bug introduced by this patch, but does this direct cast cause
out-of-bounds reads or unaligned accesses if raw_len is less than 4 or
raw_data is unaligned?
Since raw_len is derived from the HID report descriptor, a malicious or
malfunctioning device could specify a smaller field size, potentially exposing
adjacent kernel memory. Should this code check raw_len and use something like
get_unaligned_le32() since HID data is little-endian?
>
> switch (usage_id) {
> case HID_USAGE_SENSOR_LIGHT_ILLUM:
[ ... ]
> case HID_USAGE_SENSOR_TIME_TIMESTAMP:
> als_state->timestamp = hid_sensor_convert_timestamp(&als_state->common_attributes,
> *(s64 *)raw_data);
[Severity: High]
This is also a pre-existing issue, but does this face similar risks with
8-byte reads from raw_data?
Could this trigger an out-of-bounds read if raw_len is less than 8, or cause
alignment faults on strict-alignment architectures?
> ret = 0;
> break;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com?part=3
^ permalink raw reply
* Re: [PATCH 6/7] iio: orientation: hid-sensor-rotation: use u32 instead of unsigned
From: sashiko-bot @ 2026-06-06 12:27 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-6-dd4a6820b674@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Out-of-bounds read and information leak due to unverified raw_len in dev_rot_capture_sample().
- [High] Unaligned and endian-unsafe access when parsing HID raw byte streams.
--
commit 1678aae0ba9cfb6e8824b0811f3f770a159c5a16
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: orientation: hid-sensor-rotation: use u32 instead of unsigned
This commit replaces bare unsigned types with u32 for usage_id parameters
across the HID sensor rotation driver to improve code clarity and
consistency with kernel style.
> --- a/drivers/iio/orientation/hid-sensor-rotation.c
> +++ b/drivers/iio/orientation/hid-sensor-rotation.c
[ ... ]
> @@ -209,7 +209,7 @@ static int dev_rot_proc_event(struct hid_sensor_hub_device *hsdev,
>
> /* Capture samples in local storage */
> static int dev_rot_capture_sample(struct hid_sensor_hub_device *hsdev,
> - unsigned usage_id,
> + u32 usage_id,
> size_t raw_len, char *raw_data,
> void *priv)
> {
[Severity: High]
This is a pre-existing issue, but does this function perform unaligned and
endian-unsafe reads?
Looking further down in dev_rot_capture_sample(), when raw_data is accessed:
if (raw_len / 4 == sizeof(s16)) {
rot_state->scan.sampled_vals[0] = ((s16 *)raw_data)[0];
rot_state->scan.sampled_vals[1] = ((s16 *)raw_data)[1];
rot_state->scan.sampled_vals[2] = ((s16 *)raw_data)[2];
rot_state->scan.sampled_vals[3] = ((s16 *)raw_data)[3];
Since raw_data points to a byte array from a tightly packed HID report, it
is not guaranteed to be aligned to a 2-byte boundary. Casting directly to
s16 pointers might cause bus errors or panics on architectures that do not
support unaligned loads.
Also, native CPU casts will read garbage values on big-endian architectures.
Should this use get_unaligned_le16() instead?
[Severity: High]
This isn't a bug introduced by this patch, but can the fallback memcpy
result in an out-of-bounds read?
If raw_len is not exactly 8 bytes, the code unconditionally copies 16 bytes:
} else {
memcpy(&rot_state->scan.sampled_vals, raw_data,
sizeof(rot_state->scan.sampled_vals));
}
If a malformed or malicious HID descriptor specifies a size smaller than
16 bytes, this would read past the end of the raw_data buffer into adjacent
kernel memory.
The copied data could then be pushed to userspace via iio_push_to_buffers(),
potentially creating an information leak.
[Severity: High]
This is a pre-existing issue, but does the timestamp handling also have
bounds checking and alignment issues?
Later in the same function:
} else if (usage_id == HID_USAGE_SENSOR_TIME_TIMESTAMP) {
rot_state->timestamp = hid_sensor_convert_timestamp(
&rot_state->common_attributes,
*(s64 *)raw_data);
The raw_data pointer is cast directly to a 64-bit integer without verifying
that raw_len is at least 8 bytes.
Similar to the unaligned reads above, a direct cast to an s64 pointer on an
unaligned buffer could cause bus errors on certain architectures, and it is
also not endian-safe.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com?part=6
^ permalink raw reply
* Re: [PATCH 4/7] iio: light: hid-sensor-prox: use u32 instead of unsigned
From: sashiko-bot @ 2026-06-06 12:27 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-4-dd4a6820b674@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Direct casting of the raw HID byte stream pointer (`raw_data`) to `u16 *` and `u32 *` causes unaligned memory accesses and misinterprets little-endian HID data on big-endian architectures.
--
commit e810e65c50acb8fcba7f1cd54ed67e25dac1f409
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: light: hid-sensor-prox: use u32 instead of unsigned
This patch updates the usage_id parameter to use u32 instead of a bare
unsigned type. This improves code clarity and consistency with standard
kernel style.
> diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
> index edc9274a2c076..9059f00f0cedf 100644
> --- a/drivers/iio/light/hid-sensor-prox.c
> +++ b/drivers/iio/light/hid-sensor-prox.c
[ ... ]
> @@ -183,7 +183,7 @@ static int prox_proc_event(struct hid_sensor_hub_device *hsdev,
>
> /* Capture samples in local storage */
> static int prox_capture_sample(struct hid_sensor_hub_device *hsdev,
> - unsigned usage_id,
> + u32 usage_id,
> size_t raw_len, char *raw_data,
> void *priv)
> {
[Severity: High]
This is a pre-existing issue, but since you are updating types in this
function, could we also address how the raw_data pointer is accessed?
Further down in prox_capture_sample(), the raw HID byte stream is directly
cast to u16 and u32 pointers:
switch (raw_len) {
case 1:
prox_state->human_presence[chan] = *(u8 *)raw_data * multiplier;
return 0;
case 2:
prox_state->human_presence[chan] = *(u16 *)raw_data * multiplier;
return 0;
case 4:
prox_state->human_presence[chan] = *(u32 *)raw_data * multiplier;
return 0;
}
Can this sequence cause unaligned memory accesses or endianness issues?
Because HID fields are tightly packed and can have arbitrary lengths, the
raw_data pointer is not guaranteed to be 2-byte or 4-byte aligned.
Dereferencing it directly could trigger unaligned access traps on
architectures that do not natively support them.
Additionally, HID reports are transmitted in little-endian format over the
wire. Reading them via a direct cast evaluates them in host byte-order,
which could yield corrupted data on big-endian architectures.
Would it be better to use get_unaligned_le16() and get_unaligned_le32()
here to safely extract the payload?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com?part=4
^ permalink raw reply
* Re: [PATCH 2/7] iio: accel: hid-sensor-accel-3d: use u32 instead of unsigned
From: sashiko-bot @ 2026-06-06 12:27 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-2-dd4a6820b674@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Unsafe memory access via out-of-bounds read and unaligned dereference.
--
commit f0677ef9176d20b908a2aa7a22d8817abe7925a7
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: accel: hid-sensor-accel-3d: use u32 instead of unsigned
This commit updates the hid-sensor-accel-3d driver to use u32 instead of
unsigned for the usage_id variables. This improves code clarity and maintains
consistency with kernel coding styles.
> diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
> --- a/drivers/iio/accel/hid-sensor-accel-3d.c
> +++ b/drivers/iio/accel/hid-sensor-accel-3d.c
[ ... ]
> @@ -247,7 +247,7 @@ static int accel_3d_proc_event(struct hid_sensor_hub_device *hsdev,
>
> /* Capture samples in local storage */
> static int accel_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
> - unsigned usage_id,
> + u32 usage_id,
> size_t raw_len, char *raw_data,
> void *priv)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(priv);
> struct accel_3d_state *accel_state = iio_priv(indio_dev);
> int offset;
> int ret = -EINVAL;
>
> switch (usage_id) {
> case HID_USAGE_SENSOR_ACCEL_X_AXIS:
> case HID_USAGE_SENSOR_ACCEL_Y_AXIS:
> case HID_USAGE_SENSOR_ACCEL_Z_AXIS:
> offset = usage_id - HID_USAGE_SENSOR_ACCEL_X_AXIS;
> accel_state->scan.accel_val[CHANNEL_SCAN_INDEX_X + offset] =
> *(u32 *)raw_data;
[Severity: High]
This is a pre-existing issue, but I noticed a potential problem with how
raw_data is accessed here. Does this direct cast to a 32-bit integer pointer
assume that raw_len is always at least 4 bytes and that raw_data is always
4-byte aligned?
If a device provides a smaller report, could this result in an out-of-bounds
read? Also, since raw_data comes from a HID report buffer, could this trigger
an alignment fault on architectures with strict alignment requirements if the
data is unaligned?
> ret = 0;
> break;
> case HID_USAGE_SENSOR_TIME_TIMESTAMP:
> accel_state->timestamp =
> hid_sensor_convert_timestamp(
> &accel_state->common_attributes,
> *(int64_t *)raw_data);
[Severity: High]
This is also a pre-existing issue, but similar to the above, does this direct
cast to a 64-bit integer pointer require raw_len to be at least 8 bytes and
raw_data to be 8-byte aligned?
Would it be safer to use get_unaligned() and check raw_len before
dereferencing these pointers to avoid potential slab-out-of-bounds panics
or alignment faults?
> ret = 0;
> break;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com?part=2
^ permalink raw reply
* [PATCH 7/7] iio: pressure: hid-sensor-press: use u32 instead of unsigned
From: Sanjay Chitroda @ 2026-06-06 12:17 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Prefer 'u32' instead of bare 'unsigned' variable to improve code
clarity and consistency with kernel style.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/pressure/hid-sensor-press.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
index a039b99d9851..3e47a10d72a8 100644
--- a/drivers/iio/pressure/hid-sensor-press.c
+++ b/drivers/iio/pressure/hid-sensor-press.c
@@ -155,7 +155,7 @@ static const struct iio_info press_info = {
/* Callback handler to send event after all samples are received and captured */
static int press_proc_event(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
@@ -176,7 +176,7 @@ static int press_proc_event(struct hid_sensor_hub_device *hsdev,
/* Capture samples in local storage */
static int press_capture_sample(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
size_t raw_len, char *raw_data,
void *priv)
{
@@ -204,7 +204,7 @@ static int press_capture_sample(struct hid_sensor_hub_device *hsdev,
static int press_parse_report(struct platform_device *pdev,
struct hid_sensor_hub_device *hsdev,
struct iio_chan_spec *channels,
- unsigned usage_id,
+ u32 usage_id,
struct press_state *st)
{
int ret;
--
2.34.1
^ permalink raw reply related
* [PATCH 6/7] iio: orientation: hid-sensor-rotation: use u32 instead of unsigned
From: Sanjay Chitroda @ 2026-06-06 12:17 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Prefer 'u32' instead of bare 'unsigned' variable to improve code
clarity and consistency with kernel style.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/orientation/hid-sensor-rotation.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
index 4a11e4555099..20563d8efaf6 100644
--- a/drivers/iio/orientation/hid-sensor-rotation.c
+++ b/drivers/iio/orientation/hid-sensor-rotation.c
@@ -176,7 +176,7 @@ static const struct iio_info dev_rot_info = {
/* Callback handler to send event after all samples are received and captured */
static int dev_rot_proc_event(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
@@ -209,7 +209,7 @@ static int dev_rot_proc_event(struct hid_sensor_hub_device *hsdev,
/* Capture samples in local storage */
static int dev_rot_capture_sample(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
size_t raw_len, char *raw_data,
void *priv)
{
@@ -240,7 +240,7 @@ static int dev_rot_capture_sample(struct hid_sensor_hub_device *hsdev,
/* Parse report which is specific to an usage id*/
static int dev_rot_parse_report(struct platform_device *pdev,
struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
struct dev_rot_state *st)
{
int ret;
--
2.34.1
^ permalink raw reply related
* [PATCH 5/7] iio: orientation: hid-sensor-incl-3d: use u32 instead of unsigned
From: Sanjay Chitroda @ 2026-06-06 12:17 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Prefer 'u32' instead of bare 'unsigned' variable to improve code
clarity and consistency with kernel style.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/orientation/hid-sensor-incl-3d.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c
index 4e23a598a3fb..c7fbff498be7 100644
--- a/drivers/iio/orientation/hid-sensor-incl-3d.c
+++ b/drivers/iio/orientation/hid-sensor-incl-3d.c
@@ -189,7 +189,7 @@ static const struct iio_info incl_3d_info = {
/* Callback handler to send event after all samples are received and captured */
static int incl_3d_proc_event(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
@@ -212,7 +212,7 @@ static int incl_3d_proc_event(struct hid_sensor_hub_device *hsdev,
/* Capture samples in local storage */
static int incl_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
size_t raw_len, char *raw_data,
void *priv)
{
@@ -247,7 +247,7 @@ static int incl_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
static int incl_3d_parse_report(struct platform_device *pdev,
struct hid_sensor_hub_device *hsdev,
struct iio_chan_spec *channels,
- unsigned usage_id,
+ u32 usage_id,
struct incl_3d_state *st)
{
int ret;
--
2.34.1
^ permalink raw reply related
* [PATCH 4/7] iio: light: hid-sensor-prox: use u32 instead of unsigned
From: Sanjay Chitroda @ 2026-06-06 12:17 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Prefer 'u32' instead of bare 'unsigned' variable to improve code
clarity and consistency with kernel style.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/light/hid-sensor-prox.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
index edc9274a2c07..9059f00f0ced 100644
--- a/drivers/iio/light/hid-sensor-prox.c
+++ b/drivers/iio/light/hid-sensor-prox.c
@@ -166,7 +166,7 @@ static const struct iio_info prox_info = {
/* Callback handler to send event after all samples are received and captured */
static int prox_proc_event(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
@@ -183,7 +183,7 @@ static int prox_proc_event(struct hid_sensor_hub_device *hsdev,
/* Capture samples in local storage */
static int prox_capture_sample(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
size_t raw_len, char *raw_data,
void *priv)
{
--
2.34.1
^ permalink raw reply related
* [PATCH 3/7] iio: light: hid-sensor-als: use u32 instead of unsigned
From: Sanjay Chitroda @ 2026-06-06 12:17 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Prefer 'u32' instead of bare 'unsigned' variable to improve code
clarity and consistency with kernel style.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/light/hid-sensor-als.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index d72e260b8266..ae2fda8dc500 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -241,7 +241,7 @@ static const struct iio_info als_info = {
/* Callback handler to send event after all samples are received and captured */
static int als_proc_event(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
@@ -263,7 +263,7 @@ static int als_proc_event(struct hid_sensor_hub_device *hsdev,
/* Capture samples in local storage */
static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
size_t raw_len, char *raw_data,
void *priv)
{
@@ -305,7 +305,7 @@ static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
/* Parse report which is specific to an usage id*/
static int als_parse_report(struct platform_device *pdev,
struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
struct als_state *st)
{
struct iio_chan_spec *channels;
--
2.34.1
^ permalink raw reply related
* [PATCH 2/7] iio: accel: hid-sensor-accel-3d: use u32 instead of unsigned
From: Sanjay Chitroda @ 2026-06-06 12:17 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Prefer 'u32' instead of bare 'unsigned' variable to improve code
clarity and consistency with kernel style.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/accel/hid-sensor-accel-3d.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
index 2bf05ab5235e..42c4259bf209 100644
--- a/drivers/iio/accel/hid-sensor-accel-3d.c
+++ b/drivers/iio/accel/hid-sensor-accel-3d.c
@@ -223,7 +223,7 @@ static void hid_sensor_push_data(struct iio_dev *indio_dev, void *data,
/* Callback handler to send event after all samples are received and captured */
static int accel_3d_proc_event(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
@@ -247,7 +247,7 @@ static int accel_3d_proc_event(struct hid_sensor_hub_device *hsdev,
/* Capture samples in local storage */
static int accel_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
size_t raw_len, char *raw_data,
void *priv)
{
@@ -283,7 +283,7 @@ static int accel_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
static int accel_3d_parse_report(struct platform_device *pdev,
struct hid_sensor_hub_device *hsdev,
struct iio_chan_spec *channels,
- unsigned usage_id,
+ u32 usage_id,
struct accel_3d_state *st)
{
int ret;
--
2.34.1
^ permalink raw reply related
* [PATCH 1/7] iio: gyro: hid-sensor-gyro-3d: use u32 instead of unsigned
From: Sanjay Chitroda @ 2026-06-06 12:17 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260606-6-june-hid-iio-correct-usage-id-v1-0-dd4a6820b674@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Prefer 'u32' instead of bare 'unsigned' variable to improve code
clarity and consistency with kernel style.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
changes in v4:
- Use 'u32' instead of 'unsigned int' as driver is already uses that and respective prototype has same data type with input from David and Andy
- v3 link -> https://lore.kernel.org/all/20260509101040.791404-5-sanjayembedded@gmail.com/
---
drivers/iio/gyro/hid-sensor-gyro-3d.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
index e48c25c87b6d..bbca2111e79b 100644
--- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
+++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
@@ -177,7 +177,7 @@ static const struct iio_info gyro_3d_info = {
/* Callback handler to send event after all samples are received and captured */
static int gyro_3d_proc_event(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
@@ -199,7 +199,7 @@ static int gyro_3d_proc_event(struct hid_sensor_hub_device *hsdev,
/* Capture samples in local storage */
static int gyro_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
- unsigned usage_id,
+ u32 usage_id,
size_t raw_len, char *raw_data,
void *priv)
{
@@ -234,7 +234,7 @@ static int gyro_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
static int gyro_3d_parse_report(struct platform_device *pdev,
struct hid_sensor_hub_device *hsdev,
struct iio_chan_spec *channels,
- unsigned usage_id,
+ u32 usage_id,
struct gyro_3d_state *st)
{
int ret;
--
2.34.1
^ permalink raw reply related
* [PATCH 0/7] HID: iio: basic clean up for usage_id
From: Sanjay Chitroda @ 2026-06-06 12:17 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
Hi all,
This series updates all HID IIO drivers to use 'u32' instead of
bare 'unsigned' for the usage_id parameter.
This improves type clarity and ensures consistency with kernel
coding style, as HID usage IDs are defined as 32-bit values.
No functional changes are introduced.
Testing:
- Compiled with W=1 for each patch in the series
---
Sanjay Chitroda (7):
iio: gyro: hid-sensor-gyro-3d: use u32 instead of unsigned
iio: accel: hid-sensor-accel-3d: use u32 instead of unsigned
iio: light: hid-sensor-als: use u32 instead of unsigned
iio: light: hid-sensor-prox: use u32 instead of unsigned
iio: orientation: hid-sensor-incl-3d: use u32 instead of unsigned
iio: orientation: hid-sensor-rotation: use u32 instead of unsigned
iio: pressure: hid-sensor-press: use u32 instead of unsigned
drivers/iio/accel/hid-sensor-accel-3d.c | 6 +++---
drivers/iio/gyro/hid-sensor-gyro-3d.c | 6 +++---
drivers/iio/light/hid-sensor-als.c | 6 +++---
drivers/iio/light/hid-sensor-prox.c | 4 ++--
drivers/iio/orientation/hid-sensor-incl-3d.c | 6 +++---
drivers/iio/orientation/hid-sensor-rotation.c | 6 +++---
drivers/iio/pressure/hid-sensor-press.c | 6 +++---
7 files changed, 20 insertions(+), 20 deletions(-)
---
base-commit: ae696dfa47c30016cd429b9db5e70b259b8f509e
change-id: 20260606-6-june-hid-iio-correct-usage-id-57ce92cb102b
Best regards,
--
Sanjay Chitroda <sanjayembeddedse@gmail.com>
^ permalink raw reply
* Re: [PATCH 3/9] iio: gyro: hid-sensor-gyro-3d: Fix race between callback registration and device exposure
From: sashiko-bot @ 2026-06-06 12:13 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260606-5-june-hid-iio-race-fixes-v1-3-27a848c5758f@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Lack of synchronization in sensor_hub_remove_callback() leads to Use-After-Free (UAF) of function pointers and device state.
--
commit aa52ffd9a4b9c66e6a2d69f6946e1ab533f472af
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: gyro: hid-sensor-gyro-3d: Fix race between callback registration and device exposure
This commit reorders callback registration and IIO device registration
in the probe and remove paths to prevent race conditions during
initialization and teardown.
> diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
[ ... ]
> @@ -337,13 +331,19 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
> &gyro_state->callbacks);
> if (ret < 0) {
> dev_err(&pdev->dev, "callback reg failed\n");
> - goto error_iio_unreg;
> + goto error_remove_trigger;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "device register failed\n");
> + goto error_remove_callback;
> }
>
> return ret;
>
> -error_iio_unreg:
> - iio_device_unregister(indio_dev);
> +error_remove_callback:
> + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
[Severity: High]
Does this new error path introduce a use-after-free window if an event
arrives concurrently?
Looking at sensor_hub_remove_callback(), it removes the callback from the
active list but does not appear to synchronize with or wait for concurrently
executing event handlers.
If iio_device_register() fails, probe() calls sensor_hub_remove_callback()
and returns immediately. The devres subsystem will then free the indio_dev
and gyro_state memory.
Could this sequence occur:
CPU1
hid_gyro_3d_probe()
error_remove_callback:
sensor_hub_remove_callback()
return ret;
CPU2 sees this in sensor_hub_raw_event():
if (callback->capture_sample) {
callback->capture_sample(...);
Since callback points to the devm-allocated gyro_state->callbacks, would
this lead to dereferencing freed function pointers and state data?
> error_remove_trigger:
> hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
> return ret;
[ ... ]
> @@ -356,8 +356,8 @@ static void hid_gyro_3d_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
>
> - sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
> iio_device_unregister(indio_dev);
> + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
[Severity: High]
Does this teardown sequence suffer from the same lack of synchronization?
If sensor_hub_remove_callback() does not wait for ongoing events to complete,
could a concurrent sensor_hub_raw_event() still be executing and access
gyro_state->callbacks after hid_gyro_3d_remove() finishes and devres cleans
up the memory?
> hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-5-june-hid-iio-race-fixes-v1-0-27a848c5758f@gmail.com?part=3
^ permalink raw reply
* Re: [PATCH 9/9] iio: accel: hid-sensor-accel-3d: Fix race between callback registration and device exposure
From: sashiko-bot @ 2026-06-06 12:13 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260606-5-june-hid-iio-race-fixes-v1-9-27a848c5758f@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The patch introduces a Use-After-Free / NULL Pointer Dereference in the `probe` error path by registering the sensor hub callback before IIO device registration.
Pre-existing issues:
- [High] Reordering IIO device unregistration before callback removal exacerbates a Use-After-Free vulnerability during driver teardown.
- [High] Reordering IIO device unregistration before callback removal widens a severe data race leading to linked list corruption.
--
commit 538b7bb72b2f4da6c8de1d51d2852e714b2a2bcc
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: accel: hid-sensor-accel-3d: Fix race between callback registration and device exposure
The driver registers the IIO device before completing sensor hub
callback registration and unregisters callbacks while the IIO device
is still exposed during teardown.
> diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
> index 2bf05ab5235ef..c000e001c0015 100644
> --- a/drivers/iio/accel/hid-sensor-accel-3d.c
> +++ b/drivers/iio/accel/hid-sensor-accel-3d.c
[ ... ]
> @@ -399,13 +393,19 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
> &accel_state->callbacks);
> if (ret < 0) {
> dev_err(&pdev->dev, "callback reg failed\n");
> - goto error_iio_unreg;
> + goto error_remove_trigger;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "device register failed\n");
> + goto error_remove_callback;
[Severity: High]
Does this new error path expose asynchronous event handlers to a
use-after-free or NULL pointer dereference?
When sensor_hub_register_callback() succeeds, it allows the hardware to
immediately dispatch events to accel_3d_proc_event(). If iio_device_register()
then fails, the code jumps to error_remove_callback.
While sensor_hub_remove_callback() removes the callback from the list, it
does not appear to synchronize with or wait for concurrently running handlers.
When probe() returns, devres frees the indio_dev structure and the driver
core clears drvdata.
Any concurrently executing accel_3d_proc_event() might then fetch the cleared
drvdata (causing a NULL pointer dereference) or access the freed indio_dev
(causing a use-after-free).
> }
>
> return ret;
>
> -error_iio_unreg:
> - iio_device_unregister(indio_dev);
> +error_remove_callback:
> + sensor_hub_remove_callback(hsdev, hsdev->usage);
> error_remove_trigger:
> hid_sensor_remove_trigger(indio_dev, &accel_state->common_attributes);
> return ret;
> @@ -418,8 +418,8 @@ static void hid_accel_3d_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct accel_3d_state *accel_state = iio_priv(indio_dev);
>
> - sensor_hub_remove_callback(hsdev, hsdev->usage);
> iio_device_unregister(indio_dev);
[Severity: High]
This is a pre-existing issue, but does moving iio_device_unregister() before
sensor_hub_remove_callback() widen a data race leading to linked list
corruption?
By calling iio_device_unregister() first, the sensor callback remains fully
active and eligible to receive new events during IIO teardown.
Hardware events trigger accel_3d_proc_event(), which calls
iio_push_to_buffers(). This traverses the buffer_list without holding locks
using list_for_each_entry(). Concurrently, iio_device_unregister() calls
iio_disable_all_buffers(), which modifies the same buffer_list.
Can this concurrent list modification corrupt the traversal pointer,
potentially causing an infinite loop or a panic?
> + sensor_hub_remove_callback(hsdev, hsdev->usage);
[Severity: High]
This is a pre-existing issue, but does this reordering exacerbate a
use-after-free vulnerability during driver teardown?
sensor_hub_remove_callback() unlinks the callback from the internal list
without synchronizing with currently executing event handlers. When the
remove function returns, devres will immediately free indio_dev.
Previously, the slow iio_device_unregister() call was executed after callback
removal, providing an implicit grace period for in-flight events to finish.
By moving it before callback removal, this delay is eliminated, which might
cause any concurrent event handler to dereference the freed indio_dev
structure more reliably.
> hid_sensor_remove_trigger(indio_dev, &accel_state->common_attributes);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-5-june-hid-iio-race-fixes-v1-0-27a848c5758f@gmail.com?part=9
^ permalink raw reply
* Re: [PATCH 6/9] iio: light: hid-sensor-prox: Fix race between callback registration and device exposure
From: sashiko-bot @ 2026-06-06 12:13 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260606-5-june-hid-iio-race-fixes-v1-6-27a848c5758f@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Critical] Reordering `iio_device_unregister()` before `sensor_hub_remove_callback()` introduces a critical data race that can lead to an infinite loop, list corruption, or Use-After-Free during device teardown.
Pre-existing issues:
- [High] Lack of synchronization in `sensor_hub_remove_callback()` fails to flush in-flight events, causing a Use-After-Free of devres-managed memory during probe failure.
- [High] Incorrect parsing of little-endian HID report data using host-endian pointer casts.
--
commit 60bbe69d1f4ccbb5130ebd8324841d9eca79e70a
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: light: hid-sensor-prox: Fix race between callback registration and device exposure
The driver registers the IIO device before completing sensor hub
callback registration and unregisters callbacks while the IIO device
is still exposed during teardown.
This creates race windows in both probe and remove paths, which can
lead to NULL pointer dereferences or use-after-free.
Fix this by correct ordering of callback registration and
IIO device registration in probe and remove paths.
> diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
> index edc9274a2c076..3df7f28d3b445 100644
> --- a/drivers/iio/light/hid-sensor-prox.c
> +++ b/drivers/iio/light/hid-sensor-prox.c
[ ... ]
> @@ -325,13 +319,19 @@ static int hid_prox_probe(struct platform_device *pdev)
> &prox_state->callbacks);
> if (ret < 0) {
> dev_err(&pdev->dev, "callback reg failed\n");
> - goto error_iio_unreg;
> + goto error_remove_trigger;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "device register failed\n");
> + goto error_remove_callback;
> }
[Severity: High]
This is a pre-existing issue, but this patch newly exposes it in the probe
error path by moving callback registration before iio_device_register().
When hid_prox_probe() fails here and completes, devres automatically frees the
indio_dev and prox_state structures.
sensor_hub_remove_callback() acquires pdata->dyn_callback_lock to remove the
callback from the list, but it does not wait for currently executing callbacks
in sensor_hub_raw_event() (which run under pdata->lock) to complete.
Could a still-executing event handler on another CPU dereference the freed
memory, leading to a Use-After-Free?
>
> return ret;
>
> -error_iio_unreg:
> - iio_device_unregister(indio_dev);
> +error_remove_callback:
> + sensor_hub_remove_callback(hsdev, hsdev->usage);
> error_remove_trigger:
[ ... ]
> @@ -344,8 +344,8 @@ static void hid_prox_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct prox_state *prox_state = iio_priv(indio_dev);
>
> - sensor_hub_remove_callback(hsdev, hsdev->usage);
> iio_device_unregister(indio_dev);
> + sensor_hub_remove_callback(hsdev, hsdev->usage);
[Severity: Critical]
Does reordering the teardown sequence so that sensor_hub_remove_callback()
runs after iio_device_unregister() leave the sensor hub callback fully
active while the IIO device is being dismantled?
iio_device_unregister() calls iio_disable_all_buffers(), which iterates over
the buffer_list using list_del_init() and frees internal state. Concurrently,
incoming hardware events trigger prox_proc_event(). While
iio_disable_all_buffers() eventually decrements data_ready to 0,
prox_proc_event() checks data_ready without any locking.
If prox_proc_event() observes data_ready > 0 and calls iio_push_to_buffers()
concurrently with buffer teardown, will it perform a lockless list traversal
over the actively modifying buffer_list?
drivers/iio/industrialio-buffer.c:iio_push_to_buffers() {
...
list_for_each_entry(buf, &iio_dev_opaque->buffer_list, buffer_list) {
...
}
Could this concurrent modification cause the iterator's next pointer to become
self-referential, resulting in an infinite loop, or access freed memory
causing a Use-After-Free?
> hid_sensor_remove_trigger(indio_dev, &prox_state->common_attributes);
> }
[Severity: High]
This isn't a bug introduced by this patch, but looking at how raw data
is handled in prox_capture_sample() from drivers/iio/light/hid-sensor-prox.c:
switch (raw_len) {
case 1:
prox_state->human_presence[chan] = *(u8 *)raw_data * multiplier;
return 0;
case 2:
prox_state->human_presence[chan] = *(u16 *)raw_data * multiplier;
return 0;
case 4:
prox_state->human_presence[chan] = *(u32 *)raw_data * multiplier;
return 0;
}
The raw data received from the HID sensor hub is in little-endian format as
per the HID specification.
When raw_data is directly cast to u16 * or u32 * and dereferenced on a Big
Endian architecture, will this host-endian read result in incorrectly swapped
bytes, producing corrupted sensor readings?
Furthermore, since raw_data points directly into the unaligned HID report byte
array, can this cast trigger an unaligned memory access exception (kernel oops)
on strict architectures? Should this use get_unaligned_le16() or
get_unaligned_le32() instead to correctly parse the little-endian data stream?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-5-june-hid-iio-race-fixes-v1-0-27a848c5758f@gmail.com?part=6
^ permalink raw reply
* Re: [PATCH 1/9] iio: orientation: hid-sensor-rotation: Fix race between callback registration and device exposure
From: sashiko-bot @ 2026-06-06 12:11 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260606-5-june-hid-iio-race-fixes-v1-1-27a848c5758f@gmail.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Newly introduced Use-After-Free in the probe error path caused by registering the callback before IIO device registration.
- [High] Newly introduced data race leading to list corruption in the remove path by unregistering the IIO device while the sensor hub callback is still active.
Pre-existing issues:
- [High] Pre-existing Use-After-Free due to a lack of synchronization in sensor hub callback removal.
- [High] Pre-existing Out-of-Bounds Read in dev_rot_capture_sample when reading malicious HID reports.
--
commit 53e3688342562957aa2297744b7cc0b03bf921af
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Subject: iio: orientation: hid-sensor-rotation: Fix race between callback registration and device exposure
The driver registers the IIO device before completing sensor hub callback
registration and unregisters callbacks while the IIO device is still exposed
during teardown. This creates race windows in both probe and remove paths.
This patch attempts to fix this by correct ordering of callback registration.
Fixes: fc18dddc0625 ("iio: hid-sensors: Added device rotation support")
> diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
> index 4a11e45550991..fd9beb93cefb1 100644
> --- a/drivers/iio/orientation/hid-sensor-rotation.c
> +++ b/drivers/iio/orientation/hid-sensor-rotation.c
> @@ -332,12 +332,6 @@ static int hid_dev_rot_probe(struct platform_device *pdev)
> return ret;
> }
>
> - ret = iio_device_register(indio_dev);
> - if (ret) {
> - dev_err(&pdev->dev, "device register failed\n");
> - goto error_remove_trigger;
> - }
> -
> rot_state->callbacks.send_event = dev_rot_proc_event;
> rot_state->callbacks.capture_sample = dev_rot_capture_sample;
[Severity: High]
This is a pre-existing issue, but can dev_rot_capture_sample() read
out-of-bounds when handling malicious HID reports?
If raw_len does not correspond to an array of four 16-bit integers, the
function unconditionally copies 16 bytes:
drivers/iio/orientation/hid-sensor-rotation.c:dev_rot_capture_sample() {
...
} else {
memcpy(&rot_state->scan.sampled_vals, raw_data,
sizeof(rot_state->scan.sampled_vals));
}
...
}
If a malicious USB device provides a report descriptor with a field size
smaller than 16 bytes, will this memcpy() read out-of-bounds without verifying
that raw_len is at least 16?
> rot_state->callbacks.pdev = pdev;
> @@ -345,13 +339,19 @@ static int hid_dev_rot_probe(struct platform_device *pdev)
> &rot_state->callbacks);
> if (ret) {
> dev_err(&pdev->dev, "callback reg failed\n");
> - goto error_iio_unreg;
> + goto error_remove_trigger;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "device register failed\n");
> + goto error_remove_callback;
> }
[Severity: High]
Is there a use-after-free introduced in this new error path?
By registering the callback before the IIO device, if iio_device_register()
fails, the error path removes the callback and returns, triggering devres
cleanup.
Because sensor_hub_remove_callback() lacks execution synchronization, an event
dispatched immediately after callback registration can run concurrently. When
hid_dev_rot_probe() returns, the rot_state memory is freed while the event
handler is still executing.
[ ... ]
> @@ -364,8 +364,8 @@ static void hid_dev_rot_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct dev_rot_state *rot_state = iio_priv(indio_dev);
[Severity: High]
This isn't a bug introduced by this patch, but does hid_dev_rot_remove()
complete teardown without synchronization, allowing devres to free memory
while callbacks are still executing?
When sensor_hub_remove_callback() removes the callback entry from the list:
drivers/hid/hid-sensor-hub.c:sensor_hub_remove_callback() {
...
list_del(&callback->list);
kfree(callback);
...
}
It does not wait for any currently executing callback, such as
dev_rot_proc_event(), to finish. When hid_dev_rot_remove() returns, devres
automatically frees indio_dev and rot_state.
If dev_rot_proc_event() is running concurrently on another CPU:
drivers/iio/orientation/hid-sensor-rotation.c:dev_rot_proc_event() {
...
struct iio_dev *indio_dev = platform_get_drvdata(priv);
struct dev_rot_state *rot_state = iio_priv(indio_dev);
...
}
Could it dereference the freed indio_dev pointer?
>
> - sensor_hub_remove_callback(hsdev, hsdev->usage);
> iio_device_unregister(indio_dev);
> + sensor_hub_remove_callback(hsdev, hsdev->usage);
[Severity: High]
Will moving sensor_hub_remove_callback() after iio_device_unregister()
dismantle the buffer list concurrently with active events?
Leaving the callback active during teardown means an incoming sensor event can
trigger dev_rot_proc_event(), which calls iio_push_to_buffers() while
iio_device_unregister() is disabling buffers.
If iio_push_to_buffers() locklessly iterates over the buffer list:
drivers/iio/industrialio-buffer.c:iio_push_to_buffers() {
...
list_for_each_entry(buf, &iio_dev_opaque->buffer_list, buffer_list) {
...
}
And iio_device_unregister() concurrently uses list_del_init() to remove
buffers from the list, might this cause list corruption?
> hid_sensor_remove_trigger(indio_dev, &rot_state->common_attributes);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-5-june-hid-iio-race-fixes-v1-0-27a848c5758f@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 7/9] iio: light: hid-sensor-als: Fix race between callback registration and device exposure
From: sashiko-bot @ 2026-06-06 12:11 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260606-5-june-hid-iio-race-fixes-v1-7-27a848c5758f@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Critical] Reversing the teardown order in `hid_als_remove()` leaves the data producer active during IIO device unregistration, leading to buffer list corruption and a kernel panic.
- [High] Registering the HID sensor hub callback before `iio_device_register()` introduces a Use-After-Free vulnerability on the probe error path.
Pre-existing issues:
- [High] `sensor_hub_remove_callback()` does not wait for executing callbacks to finish, causing a Use-After-Free when the driver unbinds.
--
commit 7a8f6d5a76d506427e21235e877b3b2cd1616826
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: light: hid-sensor-als: Fix race between callback registration and device exposure
This commit modifies the initialization and teardown sequences in the
hid-sensor-als driver. It registers the sensor hub callback before exposing
the IIO device in the probe path, and reverses the order in the remove
path.
> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> index d72e260b82668..93d6038486908 100644
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
> @@ -406,25 +406,25 @@ static int hid_als_probe(struct platform_device *pdev)
> return ret;
> }
>
> - ret = iio_device_register(indio_dev);
> - if (ret) {
> - dev_err(&pdev->dev, "device register failed\n");
> - goto error_remove_trigger;
> - }
> -
> als_state->callbacks.send_event = als_proc_event;
> als_state->callbacks.capture_sample = als_capture_sample;
> als_state->callbacks.pdev = pdev;
> ret = sensor_hub_register_callback(hsdev, hsdev->usage, &als_state->callbacks);
> if (ret < 0) {
> dev_err(&pdev->dev, "callback reg failed\n");
> - goto error_iio_unreg;
> + goto error_remove_trigger;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "device register failed\n");
> + goto error_remove_callback;
[Severity: High]
Does registering the HID sensor hub callback before iio_device_register()
introduce a use-after-free vulnerability on the probe error path?
If iio_device_register() fails, the error path calls
sensor_hub_remove_callback() and the probe function returns, triggering
devres to free the allocated indio_dev and als_state memory.
However, sensor_hub_remove_callback() only unlinks the callback from the
list; it does not synchronize or wait for concurrently executing events.
If an event arrived from the hardware between registration and failure,
als_capture_sample() could be executing on another CPU.
That function will continue to run and write to the devres-managed memory
after it has been freed:
drivers/iio/light/hid-sensor-als.c:als_capture_sample() {
...
struct iio_dev *indio_dev = platform_get_drvdata(priv);
struct als_state *als_state = iio_priv(indio_dev);
switch (usage_id) {
case HID_USAGE_SENSOR_LIGHT_ILLUM:
als_state->scan.illum[CHANNEL_SCAN_INDEX_ILLUM] = sample_data;
...
}
> }
>
> return ret;
>
> -error_iio_unreg:
> - iio_device_unregister(indio_dev);
> +error_remove_callback:
> + sensor_hub_remove_callback(hsdev, hsdev->usage);
> error_remove_trigger:
> hid_sensor_remove_trigger(indio_dev, &als_state->common_attributes);
> return ret;
> @@ -437,8 +437,8 @@ static void hid_als_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct als_state *als_state = iio_priv(indio_dev);
>
> - sensor_hub_remove_callback(hsdev, hsdev->usage);
> iio_device_unregister(indio_dev);
> + sensor_hub_remove_callback(hsdev, hsdev->usage);
[Severity: Critical]
Does reversing the teardown order here leave the data producer active
during IIO device unregistration?
iio_device_unregister() cleans up buffers by calling
iio_disable_all_buffers(), which unlinks buffers from
indio_dev->buffer_list using list_del_init():
drivers/iio/industrialio-buffer.c:iio_buffer_deactivate() {
...
list_del_init(&buffer->buffer_list);
...
}
Because the HID event callback remains registered and active, incoming
sensor events can concurrently trigger iio_push_to_buffers_with_ts(). This
function locklessly iterates over buffer_list:
drivers/iio/industrialio-buffer.c:iio_push_to_buffers() {
...
list_for_each_entry(buf, &iio_dev_opaque->buffer_list, buffer_list) {
ret = iio_push_to_buffer(buf, data);
...
}
Concurrently traversing a linked list while it is being actively dismantled
via list_del_init() without synchronization causes the reader to follow
self-referential pointers, which can result in an infinite loop, soft
lockup, or kernel panic.
[Severity: High]
This is a pre-existing issue, but does sensor_hub_remove_callback() wait
for executing callbacks to finish?
When the driver is removed, it removes the callback using
sensor_hub_remove_callback(). This function removes the callback from the
list under dyn_callback_lock but fails to synchronize with or wait for
currently running callbacks:
drivers/hid/hid-sensor-hub.c:sensor_hub_remove_callback() {
...
list_for_each_entry(callback, &pdata->dyn_callback_list, list)
if (callback->usage_id == usage_id &&
callback->hsdev == hsdev) {
list_del(&callback->list);
kfree(callback);
break;
}
spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
...
}
If an event is already in-flight, als_capture_sample() can continue
executing after hid_als_remove() completes and devres frees the indio_dev.
The concurrent execution will then dereference the freed als_state
structure, causing a use-after-free.
> hid_sensor_remove_trigger(indio_dev, &als_state->common_attributes);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-5-june-hid-iio-race-fixes-v1-0-27a848c5758f@gmail.com?part=7
^ permalink raw reply
* Re: [PATCH 8/9] iio: magnetometer: hid-sensor-magn-3d: Fix race between callback registration and device exposure
From: sashiko-bot @ 2026-06-06 12:10 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260606-5-june-hid-iio-race-fixes-v1-8-27a848c5758f@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Critical] Reordering `iio_device_unregister()` before `sensor_hub_remove_callback()` in the remove path introduces a critical list corruption race.
- [High] Reordering callback registration before IIO device registration introduces a Use-After-Free (UAF) window on the probe error path.
Pre-existing issues:
- [High] Pre-existing Use-After-Free vulnerability because `sensor_hub_remove_callback()` unlinks the callback without synchronizing with currently executing asynchronous HID events.
--
commit b4969e9293deed992cb095faf4c58a2ab65c8d1e
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: magnetometer: hid-sensor-magn-3d: Fix race between callback registration and device exposure
This patch attempts to fix a race condition by reordering the callback
registration and IIO device registration in the probe and remove paths.
It registers the sensor hub callback before exposing the IIO device, and
unregisters the IIO device before removing the callbacks.
> diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
> index 23884825eb00f..acae77273bd5e 100644
> --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
> +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
[ ... ]
> @@ -526,13 +520,19 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
> &magn_state->callbacks);
> if (ret < 0) {
> dev_err(&pdev->dev, "callback reg failed\n");
> - goto error_iio_unreg;
> + goto error_remove_trigger;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "device register failed\n");
> + goto error_remove_callback;
> }
>
> return ret;
>
> -error_iio_unreg:
> - iio_device_unregister(indio_dev);
> +error_remove_callback:
> + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_COMPASS_3D);
[Severity: High]
Can this sequence cause a use-after-free if a callback is currently
executing?
Since sensor_hub_remove_callback() only unlinks the callback without
synchronizing or waiting for currently executing ones to finish, could
magn_3d_proc_event() still be running?
When hid_magn_3d_probe() returns an error, the devres subsystem will
immediately free indio_dev and magn_state. If a concurrent callback was
triggered right before iio_device_register() failed, it would then
access the newly freed memory.
> error_remove_trigger:
> hid_sensor_remove_trigger(indio_dev, &magn_state->magn_flux_attributes);
> return ret;
> @@ -545,8 +545,8 @@ static void hid_magn_3d_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct magn_3d_state *magn_state = iio_priv(indio_dev);
>
> - sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_COMPASS_3D);
> iio_device_unregister(indio_dev);
[Severity: Critical]
Does reordering iio_device_unregister() before removing the callback
expose a list corruption race?
iio_device_unregister() calls iio_disable_all_buffers(), which modifies
the buffer_list using list_del_init() without lock or RCU protection.
Since the sensor callback is still active, incoming hardware events can
continue to fire. If magn_3d_proc_event() calls
iio_push_to_buffers_with_timestamp() and iterates over buffer_list
locklessly at the same time list_del_init() is executing, could this
cause the next pointer to loop on itself and result in a hard lockup?
> + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_COMPASS_3D);
[Severity: High]
This isn't a bug introduced by this patch, but does the lack of
synchronization in sensor_hub_remove_callback() leave a lingering
use-after-free window here?
sensor_hub_remove_callback() removes the callback from the list but
returns without waiting for any currently executing sensor_hub_raw_event()
to finish.
After sensor_hub_remove_callback() completes, hid_magn_3d_remove()
returns and devres will free indio_dev and magn_state. If a callback is
still executing on another CPU, it might access this freed memory.
> hid_sensor_remove_trigger(indio_dev, &magn_state->magn_flux_attributes);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-5-june-hid-iio-race-fixes-v1-0-27a848c5758f@gmail.com?part=8
^ permalink raw reply
* Re: [PATCH 2/9] iio: orientation: hid-sensor-incl-3d: Fix race between callback registration and device exposure
From: sashiko-bot @ 2026-06-06 12:10 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260606-5-june-hid-iio-race-fixes-v1-2-27a848c5758f@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The patch introduces a Use-After-Free (UAF) vulnerability in the probe error path by registering the sensor hub callback before completing IIO device registration.
- [High] The patch inverts the teardown order in the remove path, leaving the HID callback active during IIO buffer teardown, which leads to lockless list corruption or Use-After-Free.
Pre-existing issues:
- [High] The driver suffers from a pre-existing Use-After-Free vulnerability during device removal because `sensor_hub_remove_callback()` does not synchronize with currently executing HID callbacks.
--
commit 7127797d9af9cf3f6a23d56c71b46a308c88b097
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: orientation: hid-sensor-incl-3d: Fix race between callback registration and device exposure
This commit modifies the probe and remove paths in the HID 3D inclinometer
driver to adjust the registration order between the IIO device and the sensor
hub callbacks. It attempts to fix a race condition by registering callbacks
before the IIO device in the probe function, and unregistering the IIO device
before removing callbacks in the remove function.
> diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c
> --- a/drivers/iio/orientation/hid-sensor-incl-3d.c
> +++ b/drivers/iio/orientation/hid-sensor-incl-3d.c
[ ... ]
> @@ -370,13 +364,19 @@ static int hid_incl_3d_probe(struct platform_device *pdev)
> &incl_state->callbacks);
> if (ret) {
> dev_err(&pdev->dev, "callback reg failed\n");
> - goto error_iio_unreg;
> + goto error_remove_trigger;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "device register failed\n");
> + goto error_remove_callback;
> }
>
> return 0;
>
> -error_iio_unreg:
> - iio_device_unregister(indio_dev);
> +error_remove_callback:
> + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_INCLINOMETER_3D);
[Severity: High]
Does this error path introduce a use-after-free regression?
By registering the callback before IIO device registration completes, the HID
event callback is added to the active list and can dispatch events
asynchronously immediately.
If iio_device_register() fails, the error path calls
sensor_hub_remove_callback(). However, this function only deletes the
callback from a lock-protected list and does not synchronize with
concurrently executing callbacks:
drivers/hid/hid-sensor-hub.c:sensor_hub_raw_event() {
...
if (callback && collection && callback->send_event)
callback->send_event(hsdev, collection->usage, callback->pdev);
...
}
When the probe function returns, the devres subsystem will free the indio_dev
memory which contains the callbacks structure.
If a concurrent HID event was dispatched just before the callback was
removed, could it locklessly dereference the freed callback->send_event
function pointer?
> error_remove_trigger:
> hid_sensor_remove_trigger(indio_dev, &incl_state->common_attributes);
> return ret;
[ ... ]
> @@ -389,8 +389,8 @@ static void hid_incl_3d_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct incl_3d_state *incl_state = iio_priv(indio_dev);
>
> - sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_INCLINOMETER_3D);
> iio_device_unregister(indio_dev);
> + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_INCLINOMETER_3D);
[Severity: High]
Might this inverted teardown order cause list corruption or a use-after-free?
By leaving the HID callback active during IIO device unregistration, the
hardware event source remains active while dismantling structures.
When iio_device_unregister() disables buffers, it removes them from
indio_dev->buffer_list using list_del_init() without holding locks that
protect the push path.
If other sensors keep the shared HID hub active, events for this sensor can
arrive concurrently. The event handler calls iio_push_to_buffers(), which
iterates locklessly:
drivers/iio/industrialio-buffer.c:iio_push_to_buffers() {
...
list_for_each_entry(buf, &iio_dev_opaque->buffer_list, buffer_list) {
ret = iio_push_to_buffer(buf, data);
...
}
If these race, could the concurrent modification result in list corruption or
dereferencing freed buffers?
[Severity: High]
This is a pre-existing issue, but does sensor_hub_remove_callback() properly
synchronize with executing HID callbacks before device removal completes?
Looking at the implementation, sensor_hub_remove_callback() removes the
callback from a lock-protected list and returns immediately:
drivers/hid/hid-sensor-hub.c:sensor_hub_remove_callback() {
...
list_del(&callback->list);
kfree(callback);
break;
...
}
It does not wait for any actively running callbacks in sensor_hub_raw_event()
to finish. Once hid_incl_3d_remove() completes, devres frees the indio_dev
memory containing the incl_state and its callbacks.
If the HID event thread is still executing the callback locklessly, might it
dereference a freed function pointer and access freed memory?
> hid_sensor_remove_trigger(indio_dev, &incl_state->common_attributes);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-5-june-hid-iio-race-fixes-v1-0-27a848c5758f@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH 5/9] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()
From: sashiko-bot @ 2026-06-06 12:09 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260606-5-june-hid-iio-race-fixes-v1-5-27a848c5758f@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Explicitly calling `iio_device_unregister()` before stopping the hardware data source causes a hard lockup.
Pre-existing issues:
- [High] The driver uses a single global static `struct hid_sensor_hub_callbacks temperature_callbacks` which is modified during `probe()`. If multiple temperature sensors are probed, the `pdev` pointer within this global struct is overwritten by the most recently probed device. This causes events from all sensors to be routed to the last probed sensor's IIO device, leading to data corruption. Furthermore, if a sensor is removed, the remaining sensors will dereference a dangling `pdev` pointer when handling events, causing a Use-After-Free.
- [High] The sensor hub remove callback lacks synchronization with active event processing, leading to use-after-free on module unload or device unbind.
--
commit 40b43e4e8d671ec0d491c5e0ebefd5ffcf97e7ad
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()
This commit switches from devm_iio_device_register() to the non-devm
iio_device_register() to avoid mixing devm APIs with goto-based unwinding.
It adds an explicit iio_device_unregister() call in the teardown path to
ensure deterministic cleanup.
> diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
> index 9f628a8e5cfbe..34bff7e9f3a33 100644
> --- a/drivers/iio/temperature/hid-sensor-temperature.c
> +++ b/drivers/iio/temperature/hid-sensor-temperature.c
> @@ -244,7 +244,7 @@ static int hid_temperature_probe(struct platform_device *pdev)
> if (ret)
> goto error_remove_trigger;
>
> - ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> + ret = iio_device_register(indio_dev);
[Severity: High]
This isn't a bug introduced by this patch, but does the driver use a
single global static struct hid_sensor_hub_callbacks temperature_callbacks
which is modified during probe()?
If multiple temperature sensors are probed, the pdev pointer within this
global struct is overwritten by the most recently probed device:
drivers/iio/temperature/hid-sensor-temperature.c:hid_temperature_probe() {
...
temperature_callbacks.pdev = pdev;
ret = sensor_hub_register_callback(hsdev,
HID_USAGE_SENSOR_TEMPERATURE,
&temperature_callbacks);
...
}
This causes events from all sensors to be routed to the last probed sensor's
IIO device, leading to data corruption.
Furthermore, if a sensor is removed, the remaining sensors will dereference
a dangling pdev pointer when handling events, causing a use-after-free.
> if (ret)
> goto error_remove_callback;
>
[ ... ]
> @@ -264,6 +264,7 @@ static void hid_temperature_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct temperature_state *temp_st = iio_priv(indio_dev);
>
> + iio_device_unregister(indio_dev);
[Severity: High]
Does explicitly calling iio_device_unregister() before stopping the
hardware data source cause a hard lockup?
The modified teardown order unregisters the IIO device before the sensor
hub callback is removed. This function deletes the buffer from the IIO core
buffer list.
Concurrently, incoming hardware interrupts can invoke
temperature_proc_event() which locklessly iterates over the same buffer
list using list_for_each_entry().
If list_del_init() executes concurrently during the iteration, the lockless
traversal can enter an infinite loop. Because the event handler executes
with IRQs disabled, this could cause a hard lockup.
> sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE);
[Severity: High]
This is a pre-existing issue, but does sensor_hub_remove_callback() lack
synchronization with active event processing?
When sensor_hub_remove_callback() unlinks the callback from
dyn_callback_list, it returns immediately without waiting for concurrent
executions of sensor_hub_raw_event() to finish.
Since the raw event handler drops dyn_callback_lock before executing the
callbacks, remove() can complete and trigger devres cleanup while the raw
event handler is still actively dereferencing the IIO device on another
CPU, potentially leading to a use-after-free.
> hid_sensor_remove_trigger(indio_dev, &temp_st->common_attributes);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-5-june-hid-iio-race-fixes-v1-0-27a848c5758f@gmail.com?part=5
^ permalink raw reply
* Re: [PATCH 4/9] iio: pressure: hid-sensor-press: Fix race between callback registration and device exposure
From: sashiko-bot @ 2026-06-06 11:50 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260606-5-june-hid-iio-race-fixes-v1-4-27a848c5758f@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Critical] Reordering teardown introduces list corruption and UAF via concurrent buffer access.
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the lack of synchronization in sensor hub callback removal leads to Use-After-Free, and this patch exposes the race on the probe error path.
--
commit 82bc619c3ad12fb028f336e0c55f37d3a740bf6e
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: pressure: hid-sensor-press: Fix race between callback registration and device exposure
The driver registers the IIO device before completing sensor hub
callback registration and unregisters callbacks while the IIO device
is still exposed during teardown. This patch reorders callback
registration and IIO device registration in probe and remove paths.
> diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
> index a039b99d9851d..09795473948a9 100644
> --- a/drivers/iio/pressure/hid-sensor-press.c
> +++ b/drivers/iio/pressure/hid-sensor-press.c
[ ... ]
> @@ -304,13 +298,19 @@ static int hid_press_probe(struct platform_device *pdev)
> &press_state->callbacks);
> if (ret < 0) {
> dev_err(&pdev->dev, "callback reg failed\n");
> - goto error_iio_unreg;
> + goto error_remove_trigger;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "device register failed\n");
> + goto error_remove_callback;
> }
>
> return ret;
>
> -error_iio_unreg:
> - iio_device_unregister(indio_dev);
> +error_remove_callback:
> + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_PRESSURE);
[Severity: High]
This is a pre-existing issue, but the lack of synchronization in sensor
hub callback removal might lead to a use-after-free, and this patch
exposes it on the probe error path.
When sensor_hub_remove_callback() deletes the list entry:
drivers/hid/hid-sensor-hub.c:sensor_hub_remove_callback() {
list_del(&callback->list);
kfree(callback);
...
}
it doesn't appear to wait for active readers to finish processing. If
iio_device_register() fails, the driver calls sensor_hub_remove_callback()
and returns, allowing devres to free the press_state memory.
Could a suspended or delayed sensor_hub_raw_event() thread resume and
dereference the freed memory?
> error_remove_trigger:
> hid_sensor_remove_trigger(indio_dev, &press_state->common_attributes);
> return ret;
> @@ -323,8 +323,8 @@ static void hid_press_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct press_state *press_state = iio_priv(indio_dev);
>
> - sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_PRESSURE);
> iio_device_unregister(indio_dev);
> + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_PRESSURE);
[Severity: Critical]
Does this reversed teardown order introduce list corruption or a
use-after-free through concurrent buffer access?
By calling iio_device_unregister() before removing the sensor hub
callback, incoming HID events can still trigger press_proc_event().
This locklessly pushes to buffers:
drivers/iio/pressure/hid-sensor-press.c:press_proc_event()
iio_push_to_buffers_with_ts()
iio_push_to_buffers()
list_for_each_entry(buf, &iio_dev_opaque->buffer_list, buffer_list)
While this runs, iio_device_unregister() actively removes buffers
from the list:
iio_device_unregister()
iio_disable_all_buffers()
list_del_init()
Could iterating over the buffer list while it is being modified by
list_del_init() cause the lockless reader to follow the self-referential
next pointer, leading to an infinite loop or use-after-free?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606-5-june-hid-iio-race-fixes-v1-0-27a848c5758f@gmail.com?part=4
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox