* Re: [PATCH 03/11] iio: gyro: hid-sensor-gyro-3d: align parenthesis for readability
From: Andy Shevchenko @ 2026-06-16 10:56 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada, linux-iio, linux-kernel,
linux-input
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-3-0cd544286575@gmail.com>
On Tue, Jun 16, 2026 at 03:56:08PM +0530, Sanjay Chitroda wrote:
> Adjust alignment of parentheses to improve readability and
> maintain consistency with kernel coding style
Respect English grammar and punctuation.
...
> /* Channel write_raw handler */
> static int gyro_3d_write_raw(struct iio_dev *indio_dev,
> - struct iio_chan_spec const *chan,
> - int val,
> - int val2,
> - long mask)
> + struct iio_chan_spec const *chan,
> + int val,
> + int val2,
> + long mask)
In cases like this have a chance to (re-)split logically.
static int gyro_3d_write_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int val, int val2, long mask)
...
These comments for the entire series.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 02/11] iio: humidity: hid-sensor-humidity: align parenthesis for readability
From: Andy Shevchenko @ 2026-06-16 10:57 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada, linux-iio, linux-kernel,
linux-input
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-2-0cd544286575@gmail.com>
On Tue, Jun 16, 2026 at 03:56:07PM +0530, Sanjay Chitroda wrote:
> Adjust alignment of parentheses to improve readability and
> maintain consistency with kernel coding style
Same comment about punctuation.
...
> /* Callback handler to send event after all samples are received and captured */
> static int humidity_proc_event(struct hid_sensor_hub_device *hsdev,
> - unsigned int usage_id, void *pdev)
> + unsigned int usage_id, void *pdev)
Shouldn't usage_id be u32 to begin with? Can you check what's going on there?
(It used to be a change to implement that, maybe it was not comprehensive one and
needs a part 2?)
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 08/11] iio: hid-sensor-magn-3d: use ! instead of explicit NULL check
From: Joshua Crofts @ 2026-06-16 11:01 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada, linux-iio, linux-kernel,
linux-input
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-8-0cd544286575@gmail.com>
On Tue, 16 Jun 2026 15:56:13 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
> Replace 'if (indio_dev == NULL)' with 'if (!indio_dev)' in to follow
> the preferred kernel style.
>
> No functional change.
>
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> ---
Couldn't all of these null check patches be squashed into one,
considering these are all the same logical change and affect
HID drivers? Surely this is code churn a bit.
Same goes for the parentheses alignment patches.
--
Kind regards
CJD
^ permalink raw reply
* [PATCH 11/11] iio: hid-sensor-accel-3d: use ! instead of explicit NULL check
From: Sanjay Chitroda @ 2026-06-16 11:01 UTC (permalink / raw)
To: jikos, jic23, srinivas.pandruvada, David Lechner, Nuno Sá,
Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-950d25a05d9a@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Replace 'if (indio_dev == NULL)' with 'if (!indio_dev)' in to follow
the preferred kernel style.
No functional change.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/accel/hid-sensor-accel-3d.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
index 42c4259bf209..0999f7dbf434 100644
--- a/drivers/iio/accel/hid-sensor-accel-3d.c
+++ b/drivers/iio/accel/hid-sensor-accel-3d.c
@@ -329,7 +329,7 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
indio_dev = devm_iio_device_alloc(&pdev->dev,
sizeof(struct accel_3d_state));
- if (indio_dev == NULL)
+ if (!indio_dev)
return -ENOMEM;
platform_set_drvdata(pdev, indio_dev);
--
2.34.1
^ permalink raw reply related
* [PATCH 11/11] iio: hid-sensor-accel-3d: use ! instead of explicit NULL check
From: Sanjay Chitroda @ 2026-06-16 11:11 UTC (permalink / raw)
To: jikos, jic23, srinivas.pandruvada, David Lechner, Nuno Sá,
Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Replace 'if (indio_dev == NULL)' with 'if (!indio_dev)' in to follow
the preferred kernel style.
No functional change.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/accel/hid-sensor-accel-3d.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
index 42c4259bf209..0999f7dbf434 100644
--- a/drivers/iio/accel/hid-sensor-accel-3d.c
+++ b/drivers/iio/accel/hid-sensor-accel-3d.c
@@ -329,7 +329,7 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
indio_dev = devm_iio_device_alloc(&pdev->dev,
sizeof(struct accel_3d_state));
- if (indio_dev == NULL)
+ if (!indio_dev)
return -ENOMEM;
platform_set_drvdata(pdev, indio_dev);
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 04/11] iio: magnetometer: hid-sensor-magn-3d: align parenthesis for readability
From: sashiko-bot @ 2026-06-16 11:19 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-4-0cd544286575@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Blindly reading 32 or 64 bits from `raw_data` without verifying `raw_len` can lead to slab-out-of-bounds reads.
--
commit 13804edb2c46947f67fe0d4d5d050f780422d2f7
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: magnetometer: hid-sensor-magn-3d: align parenthesis for readability
This commit adjusts the alignment of parentheses across multiple functions in
the hid-sensor-magn-3d driver. This improves readability and maintains
consistency with the kernel coding style.
> diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
> index 23884825eb00f..82507297c55bb 100644
> --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
> +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
[ ... ]
> @@ -292,9 +291,9 @@ static int magn_3d_proc_event(struct hid_sensor_hub_device *hsdev,
>
> /* Capture samples in local storage */
> static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
> - u32 usage_id,
> - size_t raw_len, char *raw_data,
> - void *priv)
> + u32 usage_id,
> + size_t raw_len, char *raw_data,
> + void *priv)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(priv);
> struct magn_3d_state *magn_state = iio_priv(indio_dev);
[Severity: High]
This is a pre-existing issue, but since this patch touches the signature of
magn_3d_capture_sample(), could we be at risk of a slab out-of-bounds read
further down in the function?
If a device presents a crafted HID report descriptor that defines the
HID_USAGE_SENSOR_TIME_TIMESTAMP field with a smaller size, it could result in
raw_len being less than 8. Reading 8 bytes from raw_data in this case:
case HID_USAGE_SENSOR_TIME_TIMESTAMP:
magn_state->timestamp =
hid_sensor_convert_timestamp(&magn_state->magn_flux_attributes,
*(s64 *)raw_data);
would read past the end of the buffer.
A similar dereference happens later in the function for the magnetic axes
where 4 bytes are read:
iio_val = magn_state->magn_val_addr[offset];
if (iio_val != NULL)
*iio_val = *((u32 *)raw_data);
Could this also trigger an out-of-bounds read if raw_len is smaller than 4?
Should we verify the size of raw_len before making these casts?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com?part=4
^ permalink raw reply
* Re: [PATCH 01/11] iio: hid-sensors: add missing blank line after declarations
From: sashiko-bot @ 2026-06-16 11:22 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-1-0cd544286575@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but pending asynchronous suspend work is cancelled during device removal, leaving hardware powered on indefinitely.
--
commit fb90a2bc06515600f67795b51872db81c06e23a2
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: hid-sensors: add missing blank line after declarations
Add a blank line after variable declarations to improve
readability and align with kernel coding style.
> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> index 417c4ab8c1b27..c8ccf96f3d035 100644
> --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> @@ -314,7 +314,9 @@ static int __maybe_unused hid_sensor_resume(struct device *dev)
> {
> struct iio_dev *indio_dev = dev_get_drvdata(dev);
> struct hid_sensor_common *attrb = iio_device_get_drvdata(indio_dev);
> +
> schedule_work(&attrb->work);
> +
> return 0;
> }
[Severity: High]
This is a pre-existing issue, but I noticed a potential problem where
pending asynchronous suspend work might be cancelled during device removal,
leaving the hardware powered on indefinitely.
During driver removal, iio_device_unregister() disables active iio buffers,
triggering buffer_predisable(). This calls hid_sensor_power_state() with
state false, which queues an asynchronous suspend:
hid_sensor_power_state() {
...
} else {
atomic_dec(&st->user_requested_state);
pm_runtime_use_autosuspend(&st->pdev->dev);
ret = pm_runtime_put_autosuspend(&st->pdev->dev);
}
...
}
Immediately after, hid_sensor_remove_trigger() is called:
hid_sensor_remove_trigger() {
if (atomic_read(&attrb->runtime_pm_enable))
pm_runtime_disable(&attrb->pdev->dev);
pm_runtime_set_suspended(&attrb->pdev->dev);
...
}
Because pm_runtime_disable() cancels any pending asynchronous idle/suspend
work, is it possible the autosuspend work gets cancelled before execution?
If the work is cancelled, pm_runtime_set_suspended() would only update the
software pm state, leaving the actual physical hardware permanently powered
on. Does the driver need a synchronous suspend in the removal path before
disabling runtime pm?
>
> @@ -322,6 +324,7 @@ static int __maybe_unused hid_sensor_runtime_resume(struct device *dev)
> {
> struct iio_dev *indio_dev = dev_get_drvdata(dev);
> struct hid_sensor_common *attrb = iio_device_get_drvdata(indio_dev);
> +
> return _hid_sensor_power_state(attrb, true);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v2 1/4] HID: multitouch: set INPUT_PROP_PRESSUREPAD based on Digitizer/Button Type
From: Rong Zhang @ 2026-06-16 11:16 UTC (permalink / raw)
To: Peter Hutterer
Cc: devnull+peter.hutterer.who-t.net, bentiss, dmitry.torokhov, jikos,
linux-input, linux-kernel, linux-kselftest, shuah, vadim
In-Reply-To: <ajDO531wxzVZlL4g@quokka>
Hi Peter,
On Tue, 2026-06-16 at 14:24 +1000, Peter Hutterer wrote:
> Hi Rong,
>
> On Tue, Jun 02, 2026 at 01:25:57AM +0800, Rong Zhang wrote:
> >
> > Hi all,
> >
> > Hopefully I'm not too late to show up here.
> >
> > > From: Peter Hutterer <peter.hutterer@who-t.net>
> > >
> > > A Digitizer/Button Type value of 1 indicates the device is a
> > > pressurepad, see
> > > https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-windows-precision-touchpad-collection#device-capabilities-feature-report
> > >
> > > Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
> > > ---
> > > drivers/hid/hid-multitouch.c | 12 +++++++++++-
> > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> > > index 179dc316b4b518d78bdc900d9fd15756c5eba83e..382e6f50c4f7e663af7d028abb8be7cb2e6e7b8e 100644
> > > --- a/drivers/hid/hid-multitouch.c
> > > +++ b/drivers/hid/hid-multitouch.c
> > > @@ -81,6 +81,7 @@ MODULE_LICENSE("GPL");
> > > #define MT_INPUTMODE_TOUCHPAD 0x03
> > >
> > > #define MT_BUTTONTYPE_CLICKPAD 0
> > > +#define MT_BUTTONTYPE_PRESSUREPAD 1
> > >
> > > enum latency_mode {
> > > HID_LATENCY_NORMAL = 0,
> > > @@ -179,6 +180,7 @@ struct mt_device {
> > > __u8 inputmode_value; /* InputMode HID feature value */
> > > __u8 maxcontacts;
> > > bool is_buttonpad; /* is this device a button pad? */
> > > + bool is_pressurepad; /* is this device a pressurepad? */
> > > bool is_haptic_touchpad; /* is this device a haptic touchpad? */
> > > bool serial_maybe; /* need to check for serial protocol */
> > >
> > > @@ -530,8 +532,14 @@ static void mt_feature_mapping(struct hid_device *hdev,
> > > }
> > >
> > > mt_get_feature(hdev, field->report);
> > > - if (field->value[usage->usage_index] == MT_BUTTONTYPE_CLICKPAD)
> > > + switch (field->value[usage->usage_index]) {
> > > + case MT_BUTTONTYPE_CLICKPAD:
> > > td->is_buttonpad = true;
> > > + break;
> > > + case MT_BUTTONTYPE_PRESSUREPAD:
> > > + td->is_pressurepad = true;
> > > + break;
> > > + }
> > >
> > > break;
> > > case 0xff0000c5:
> > > @@ -1393,6 +1401,8 @@ static int mt_touch_input_configured(struct hid_device *hdev,
> > >
> > > if (td->is_buttonpad)
> > > __set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
> > > + if (td->is_pressurepad)
> > > + __set_bit(INPUT_PROP_PRESSUREPAD, input->propbit);
> >
> > I noticed that this leads to dual reporting on my device.
> >
> > Consider previous checks:
> >
> > if (application == HID_DG_TOUCHPAD) {
> > mt_application->mt_flags |= INPUT_MT_POINTER;
> > td->inputmode_value = MT_INPUTMODE_TOUCHPAD;
> > }
> >
> > ...
> >
> > /* check for clickpads */
> > if ((app->mt_flags & INPUT_MT_POINTER) &&
> > (app->buttons_count == 1))
> > td->is_buttonpad = true;
> >
> > ... where `td->is_buttonpad' is set to true when a pressure pad has only
> > one button, i.e., the "touchpad button integrated with digitizer" [1].
> > Most (if not all) pressure pads fall into this category. As a result,
> > the presence of INPUT_PROP_PRESSUREPAD is always accompanied by the
> > presence of INPUT_PROP_BUTTONPAD.
>
> Yes, this is intended, see commit
> ae8966b7b5bd69b86209cc34bcca1ba9f18b68e6 which lists this in the commit
> message:
>
> ```
> This means:
> - clickpad: INPUT_PROP_BUTTONPAD
> - pressurepad: INPUT_PROP_BUTTONPAD + INPUT_PROP_PRESSUREPAD
> - pressurepad with configurable haptics:
> INPUT_PROP_BUTTONPAD + INPUT_PROP_PRESSUREPAD + FF_HAPTIC
> ```
>
> We have to keep setting BUTTONPAD on all pressurepads because otherwise
> we'd break existing userspace which relies on this.
Thanks a lot for your explanation. It makes sense to me.
Thanks,
Rong
>
> Cheers,
> Peter
>
^ permalink raw reply
* Re: [PATCH 02/11] iio: humidity: hid-sensor-humidity: align parenthesis for readability
From: sashiko-bot @ 2026-06-16 11:22 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-2-0cd544286575@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Global state corruption due to shared static `humidity_callbacks` struct
- [High] Out-of-bounds read and unaligned memory access in `humidity_capture_sample()`
- [High] Possible list corruption and use-after-free during device removal
--
commit 340e0cdb97cad589244be8376c8b1f7f79ff105f
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: humidity: hid-sensor-humidity: align parenthesis for readability
Adjust alignment of parentheses to improve readability and
maintain consistency with kernel coding style
> diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
> index be2338d5f4070..6fed019f4099f 100644
> --- a/drivers/iio/humidity/hid-sensor-humidity.c
> +++ b/drivers/iio/humidity/hid-sensor-humidity.c
[ ... ]
> @@ -141,8 +141,8 @@ static int humidity_proc_event(struct hid_sensor_hub_device *hsdev,
>
> /* Capture samples in local storage */
> static int humidity_capture_sample(struct hid_sensor_hub_device *hsdev,
> - unsigned int usage_id, size_t raw_len,
> - char *raw_data, void *pdev)
> + unsigned int usage_id, size_t raw_len,
> + char *raw_data, void *pdev)
[Severity: High]
This is a pre-existing issue, but is there a potential out-of-bounds read
and unaligned memory access here?
The HID sensor hub core invokes humidity_capture_sample() with raw_data
pointing to a field within a raw byte report buffer, and raw_len specifying
its exact byte size:
drivers/iio/humidity/hid-sensor-humidity.c:humidity_capture_sample() {
...
case HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY:
humid_st->scan.humidity_data = *(s32 *)raw_data;
return 0;
...
}
The driver unconditionally casts raw_data to a 32-bit integer pointer and
performs a 4-byte read. If raw_len is less than 4 (e.g., 1 or 2 bytes) and
the field resides at the end of the report buffer, does this result in an
out-of-bounds memory read?
Additionally, since HID reports are byte-packed, raw_data is not guaranteed
to be 32-bit aligned. Could an unaligned 32-bit read trigger a kernel panic
on architectures that strictly enforce alignment (e.g., ARMv5, SPARC)?
> {
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct hid_humidity_state *humid_st = iio_priv(indio_dev);
[ ... ]
> @@ -234,7 +234,7 @@ static int hid_humidity_probe(struct platform_device *pdev)
> atomic_set(&humid_st->common_attributes.data_ready, 0);
>
> ret = hid_sensor_setup_trigger(indio_dev, name,
> - &humid_st->common_attributes);
> + &humid_st->common_attributes);
> if (ret)
> return ret;
>
> @@ -242,7 +242,7 @@ static int hid_humidity_probe(struct platform_device *pdev)
>
> humidity_callbacks.pdev = pdev;
> ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_HUMIDITY,
> - &humidity_callbacks);
[Severity: High]
This isn't a bug introduced by this patch, but could this cause global state
corruption?
The humidity_callbacks struct is defined statically at the file scope. Here,
during hid_humidity_probe(), its pdev field is overwritten with the
instance-specific pdev.
The HID core (sensor_hub_register_callback()) stores the pointer to this
shared struct rather than copying its contents. If multiple humidity sensors
are probed, the global pdev will be overwritten by the last probed instance.
When sensor_hub_raw_event() processes an event, does it access callback->pdev
and erroneously route events from all connected sensors to the IIO device of
the last probed sensor, potentially causing data corruption or a
Use-After-Free if that device is later unbound?
> + &humidity_callbacks);
> if (ret)
> goto error_remove_trigger;
>
[Severity: High]
This is a pre-existing issue, but is there a possible list corruption and
use-after-free during device removal?
In hid_humidity_remove(), iio_device_unregister() is called before
sensor_hub_remove_callback(). This allows the HID sensor hub to continue
dispatching hardware events via the active callback while the IIO device is
being concurrently torn down.
When an event arrives, humidity_proc_event() checks the data_ready atomic
variable. Since this is an unlocked Time-Of-Check to Time-Of-Use race,
could data_ready be read as non-zero right before iio_device_unregister()
disables the trigger and clears it?
humidity_proc_event() then calls iio_push_to_buffers_with_timestamp(),
which iterates over the IIO buffer_list without taking locks.
Concurrently, iio_device_unregister() calls iio_disable_all_buffers()
which calls list_del_init() on the buffers. Does traversing a list
concurrently while elements are being removed and reinitialized cause
infinite loops or Use-After-Free crashes?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com?part=2
^ permalink raw reply
* [PATCH 1/1] HID: core: Fix OOB read in hid_get_report for numbered reports
From: Lee Jones @ 2026-06-16 11:26 UTC (permalink / raw)
To: lee, Jiri Kosina, Benjamin Tissoires, Johan Hovold,
Greg Kroah-Hartman, linux-input, linux-kernel
When a caller passes a size of 0 to hid_report_raw_event() for a
numbered report, the function originally called hid_get_report() before
performing any size validation.
Inside hid_get_report(), if the report is numbered (report_enum->numbered
is true), it unconditionally dereferences data[0] to extract the report ID.
With a size of 0, this results in an out-of-bounds read or kernel panic.
Fix this by moving the numbered report size validation check before the
call to hid_get_report(), ensuring that size is at least 1 before
dereferencing the data pointer.
Fixes: 2c85c61d1332 ("HID: pass the buffer size to hid_report_raw_event")
Signed-off-by: Lee Jones <lee@kernel.org>
---
drivers/hid/hid-core.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 41a79e43c82b..cf123347a2af 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2045,6 +2045,13 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
u8 *cdata = data;
int ret = 0;
+ if (report_enum->numbered && (size < 1 || bufsize < 1)) {
+ hid_warn_ratelimited(hid,
+ "Event data for numbered report is too short (%d vs %zu)\n",
+ size, bufsize);
+ return -EINVAL;
+ }
+
report = hid_get_report(report_enum, data);
if (!report)
return 0;
--
2.54.0.1136.gdb2ca164c4-goog
^ permalink raw reply related
* Re: [PATCH 07/11] iio: temperature: hid-sensor-temperature: use common device for devres
From: sashiko-bot @ 2026-06-16 11:27 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-7-0cd544286575@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free of IIO channels array when in-kernel consumers hold references past driver unbind.
- [High] State corruption due to overwriting a static global callbacks structure during probe.
--
commit 92f73ed0d62d832f9bf6d6b90aa841b134a7d14d
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: temperature: hid-sensor-temperature: use common device for devres
This commit updates hid-sensor-temperature to allocate memory using the
platform device instead of the IIO device. This unifies devres ownership
under the platform device for deterministic resource release.
> diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
> index 9f628a8e5cfbe..31ebfd1a1fe2c 100644
> --- a/drivers/iio/temperature/hid-sensor-temperature.c
> +++ b/drivers/iio/temperature/hid-sensor-temperature.c
> @@ -213,8 +213,8 @@ static int hid_temperature_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - temp_chans = devm_kmemdup(&indio_dev->dev, temperature_channels,
> - sizeof(temperature_channels), GFP_KERNEL);
> + temp_chans = devm_kmemdup(&pdev->dev, temperature_channels,
> + sizeof(temperature_channels), GFP_KERNEL);
[Severity: High]
This isn't a bug introduced by this patch, but does using devm_kmemdup()
for the IIO channels array tie its lifecycle to driver unbind rather than
the IIO device's reference-counted lifecycle?
If an in-kernel consumer holds a reference to the IIO device after the
provider driver unbinds, the consumer might access the freed channels array
through its cached iio_channel pointer (e.g., via
iio_read_channel_ext_info(), which does not check if indio_dev->info was
cleared).
Could this lead to a use-after-free? Would it be safer to make the
channels array part of the iio_priv structure so it lives as long as the
indio_dev itself?
> if (!temp_chans)
> return -ENOMEM;
[Severity: High]
This is a pre-existing issue, but looking further down in
hid_temperature_probe(), is it safe to assign pdev to a global static
callbacks structure?
hid_temperature_probe() {
...
temperature_callbacks.pdev = pdev;
ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE,
&temperature_callbacks);
...
}
If a system contains multiple HID temperature sensors, would probing a
second sensor overwrite the pdev pointer with the new device instance?
When the sensor hub receives an event for the first sensor, it might fire
the callback using the pdev of the second sensor, pushing data into the
wrong device's buffer.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com?part=7
^ permalink raw reply
* Re: [PATCH 05/11] iio: humidity: hid-sensor-humidity: use common device for devres
From: sashiko-bot @ 2026-06-16 11:27 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-5-0cd544286575@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Changing the devm allocation parent from the IIO device to the platform device creates a dangling pointer, as the IIO device can outlive the platform device.
Pre-existing issues:
- [High] The driver shares a single static global `struct hid_sensor_hub_callbacks humidity_callbacks` which is overwritten by each probed device, causing data corruption and Use-After-Free.
--
commit ba18caab7a04132bf9ca0d7432b6f7dcfedfbc00
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: humidity: hid-sensor-humidity: use common device for devres
This patch changes the devres allocation parent for the humidity channels
array. It moves the ownership from the IIO device to the HID platform
device to consolidate the devres ownership tree.
> diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
> index 6fed019f4099f..73120b87d641d 100644
> --- a/drivers/iio/humidity/hid-sensor-humidity.c
> +++ b/drivers/iio/humidity/hid-sensor-humidity.c
> @@ -215,7 +215,7 @@ static int hid_humidity_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - humid_chans = devm_kmemdup(&indio_dev->dev, humidity_channels,
> + humid_chans = devm_kmemdup(&pdev->dev, humidity_channels,
> sizeof(humidity_channels), GFP_KERNEL);
[Severity: High]
Does changing the devm allocation parent to the platform device create a
dangling pointer here?
In the IIO subsystem, the iio_dev object is reference-counted and can outlive
the platform device if userspace holds open file descriptors (for example,
/dev/iio:deviceX). Devres resources attached to pdev->dev are freed
unconditionally when the driver unbinds, but resources attached to
indio_dev->dev live until the iio_dev reference count drops to zero.
By moving the allocation of humidity_channels to pdev->dev, won't
indio_dev->channels point to freed memory immediately after unbind, leading
to a use-after-free if the IIO core or userspace continues to interact with
the still-alive iio_dev?
> if (!humid_chans)
> return -ENOMEM;
[ ... ]
While looking at hid_humidity_probe(), I noticed another detail:
drivers/iio/humidity/hid-sensor-humidity.c:hid_humidity_probe() {
...
humidity_callbacks.pdev = pdev;
ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_HUMIDITY,
&humidity_callbacks);
...
}
[Severity: High]
This is a pre-existing issue, but does assigning the local platform device to
the globally shared humidity_callbacks structure cause state corruption?
If a system has multiple humidity sensors probed, it appears the pdev of the
last probed sensor overwrites the pointer for all previous instances.
When sensor_hub_raw_event() dispatches an event for an earlier sensor, it
calls callback->capture_sample() with the overwritten pdev. Could this
corrupt the state of the newer sensor, or cause a use-after-free if the
newer sensor is unbound while the older one remains active?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com?part=5
^ permalink raw reply
* Re: [PATCH 2/6] Input: mms114 - prefer GPL over GPL v2 for module license
From: Linus Walleij @ 2026-06-16 11:38 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Bryam Vargas, linux-kernel
In-Reply-To: <20260616050912.1531241-2-dmitry.torokhov@gmail.com>
On Tue, Jun 16, 2026 at 7:09 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> As explained in commit bf7fbeeae6db ("module: Cure the MODULE_LICENSE
> "GPL" vs. "GPL v2" bogosity"), "GPL" and "GPL v2" have identical
> semantics in the module loader, but "GPL" is preferred to avoid
> unnecessary confusion and maintain consistency across the kernel.
>
> Change MODULE_LICENSE("GPL v2") to MODULE_LICENSE("GPL").
>
> Assisted-by: Antigravity:gemini-3.5-flash
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 1/1] HID: core: Fix OOB read in hid_get_report for numbered reports
From: Greg Kroah-Hartman @ 2026-06-16 11:50 UTC (permalink / raw)
To: Lee Jones
Cc: Jiri Kosina, Benjamin Tissoires, Johan Hovold, linux-input,
linux-kernel
In-Reply-To: <20260616112700.1990813-1-lee@kernel.org>
On Tue, Jun 16, 2026 at 11:26:56AM +0000, Lee Jones wrote:
> When a caller passes a size of 0 to hid_report_raw_event() for a
> numbered report, the function originally called hid_get_report() before
> performing any size validation.
>
> Inside hid_get_report(), if the report is numbered (report_enum->numbered
> is true), it unconditionally dereferences data[0] to extract the report ID.
> With a size of 0, this results in an out-of-bounds read or kernel panic.
>
> Fix this by moving the numbered report size validation check before the
> call to hid_get_report(), ensuring that size is at least 1 before
> dereferencing the data pointer.
>
> Fixes: 2c85c61d1332 ("HID: pass the buffer size to hid_report_raw_event")
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
> drivers/hid/hid-core.c | 7 +++++++
> 1 file changed, 7 insertions(+)
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- You have marked a patch with a "Fixes:" tag for a commit that is in an
older released kernel, yet you do not have a cc: stable line in the
signed-off-by area at all, which means that the patch will not be
applied to any older kernel releases. To properly fix this, please
follow the documented rules in the
Documentation/process/stable-kernel-rules.rst file for how to resolve
this.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
^ permalink raw reply
* [PATCH 0/3] HID: iio: callback API signature match for usage_id
From: Sanjay Chitroda @ 2026-06-16 13:25 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,
Most of HID IIO driver has correct 'u32' type of usage_id with
https://lore.kernel.org/all/20260610-6-june-hid-iio-correct-usage-id-v2-0-c3c5f0720493@gmail.com/
series which is applied on iio/testing branch.
On top of the same, this series updates remaining HID IIO drivers
to use 'u32' for the usage_id parameter.
Pending list of HID IIO drivers are extracted with command line:
find drivers/iio/ -type f -name "*hid*" | xargs grep -A 5 static | \
grep -E -A 5 "_proc_event\(|_capture_sample\(|_parse_report\(" --color | \
grep usage_id
This matches expected callback API type 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
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
Sanjay Chitroda (3):
iio: hid-sensor-temperature: use u32 instead of unsigned int
iio: hid-sensor-humidity: use u32 instead of unsigned int
iio: hid-sensor-custom-intel-hinge: use u32 instead of unsigned int
drivers/iio/humidity/hid-sensor-humidity.c | 6 +++---
drivers/iio/position/hid-sensor-custom-intel-hinge.c | 6 +++---
drivers/iio/temperature/hid-sensor-temperature.c | 6 +++---
3 files changed, 9 insertions(+), 9 deletions(-)
---
base-commit: a50909aa46dec46de3c73235fc15a7d6f763d996
change-id: 20260616-16-jun-hid-iio-u32-api-type-180e8f0b2c37
Best regards,
--
Sanjay Chitroda <sanjayembeddedse@gmail.com>
^ permalink raw reply
* [PATCH 1/3] iio: hid-sensor-temperature: use u32 instead of unsigned int
From: Sanjay Chitroda @ 2026-06-16 13:25 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: <20260616-16-jun-hid-iio-u32-api-type-v1-0-952c74195f46@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Prefer 'u32' instead of 'unsigned int' for usage_id variable.
This matches expected callback API type and improves code clarity.
No functional change.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/temperature/hid-sensor-temperature.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
index 9f628a8e5cfb..a8d3a15f9c53 100644
--- a/drivers/iio/temperature/hid-sensor-temperature.c
+++ b/drivers/iio/temperature/hid-sensor-temperature.c
@@ -125,7 +125,7 @@ static const struct iio_info temperature_info = {
/* Callback handler to send event after all samples are received and captured */
static int temperature_proc_event(struct hid_sensor_hub_device *hsdev,
- unsigned int usage_id, void *pdev)
+ u32 usage_id, void *pdev)
{
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct temperature_state *temp_st = iio_priv(indio_dev);
@@ -140,7 +140,7 @@ static int temperature_proc_event(struct hid_sensor_hub_device *hsdev,
/* Capture samples in local storage */
static int temperature_capture_sample(struct hid_sensor_hub_device *hsdev,
- unsigned int usage_id, size_t raw_len,
+ u32 usage_id, size_t raw_len,
char *raw_data, void *pdev)
{
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
@@ -159,7 +159,7 @@ static int temperature_capture_sample(struct hid_sensor_hub_device *hsdev,
static int temperature_parse_report(struct platform_device *pdev,
struct hid_sensor_hub_device *hsdev,
struct iio_chan_spec *channels,
- unsigned int usage_id,
+ u32 usage_id,
struct temperature_state *st)
{
int ret;
--
2.34.1
^ permalink raw reply related
* [PATCH 2/3] iio: hid-sensor-humidity: use u32 instead of unsigned int
From: Sanjay Chitroda @ 2026-06-16 13:25 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: <20260616-16-jun-hid-iio-u32-api-type-v1-0-952c74195f46@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Prefer 'u32' instead of 'unsigned int' for usage_id variable.
This matches expected callback API type and improves code clarity.
No functional change.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/humidity/hid-sensor-humidity.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
index be2338d5f407..c376a247b137 100644
--- a/drivers/iio/humidity/hid-sensor-humidity.c
+++ b/drivers/iio/humidity/hid-sensor-humidity.c
@@ -127,7 +127,7 @@ static const struct iio_info humidity_info = {
/* Callback handler to send event after all samples are received and captured */
static int humidity_proc_event(struct hid_sensor_hub_device *hsdev,
- unsigned int usage_id, void *pdev)
+ u32 usage_id, void *pdev)
{
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct hid_humidity_state *humid_st = iio_priv(indio_dev);
@@ -141,7 +141,7 @@ static int humidity_proc_event(struct hid_sensor_hub_device *hsdev,
/* Capture samples in local storage */
static int humidity_capture_sample(struct hid_sensor_hub_device *hsdev,
- unsigned int usage_id, size_t raw_len,
+ u32 usage_id, size_t raw_len,
char *raw_data, void *pdev)
{
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
@@ -161,7 +161,7 @@ static int humidity_capture_sample(struct hid_sensor_hub_device *hsdev,
static int humidity_parse_report(struct platform_device *pdev,
struct hid_sensor_hub_device *hsdev,
struct iio_chan_spec *channels,
- unsigned int usage_id,
+ u32 usage_id,
struct hid_humidity_state *st)
{
int ret;
--
2.34.1
^ permalink raw reply related
* [PATCH 3/3] iio: hid-sensor-custom-intel-hinge: use u32 instead of unsigned int
From: Sanjay Chitroda @ 2026-06-16 13:25 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: <20260616-16-jun-hid-iio-u32-api-type-v1-0-952c74195f46@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Prefer 'u32' instead of 'unsigned int' for usage_id variable.
This matches expected callback API type and improves code clarity.
No functional change.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/position/hid-sensor-custom-intel-hinge.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/position/hid-sensor-custom-intel-hinge.c b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
index a26d391661fd..2139ddb670c4 100644
--- a/drivers/iio/position/hid-sensor-custom-intel-hinge.c
+++ b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
@@ -190,7 +190,7 @@ static const struct iio_info hinge_info = {
* and captured.
*/
static int hinge_proc_event(struct hid_sensor_hub_device *hsdev,
- unsigned int usage_id, void *priv)
+ u32 usage_id, void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
struct hinge_state *st = iio_priv(indio_dev);
@@ -209,7 +209,7 @@ static int hinge_proc_event(struct hid_sensor_hub_device *hsdev,
/* Capture samples in local storage */
static int hinge_capture_sample(struct hid_sensor_hub_device *hsdev,
- unsigned int usage_id, size_t raw_len,
+ u32 usage_id, size_t raw_len,
char *raw_data, void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
@@ -236,7 +236,7 @@ static int hinge_capture_sample(struct hid_sensor_hub_device *hsdev,
static int hinge_parse_report(struct platform_device *pdev,
struct hid_sensor_hub_device *hsdev,
struct iio_chan_spec *channels,
- unsigned int usage_id, struct hinge_state *st)
+ u32 usage_id, struct hinge_state *st)
{
int ret;
int i;
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v3 03/10] dt-bindings: input: microchip,cap11xx: Update datasheet URL and LED reg range
From: Jun Yan @ 2026-06-16 13:34 UTC (permalink / raw)
To: conor
Cc: conor+dt, devicetree, dmitry.torokhov, jerrysteve1101, krzk+dt,
linux-input, linux-kernel, robh
In-Reply-To: <20260615-splinter-subtitle-a88cf08320e8@spud>
> On Mon, Jun 15, 2026 at 10:20:29PM +0800, Jun Yan wrote:
> > - Add datasheet links for all supported CAP11xx variants.
> > - Update LED node regex and replace enum constraints with minimum/maximum
> > for LED reg ranges in preparation for CAP1114 support.
> >=20
> > CAP1114 has 11 LED channels. minimum/maximum constraints are easier to
> > maintain than long enum lists when expanding channel count later.
> >=20
> > Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> > ---
> > .../bindings/input/microchip,cap11xx.yaml | 17 ++++++++++++++---
> > 1 file changed, 14 insertions(+), 3 deletions(-)
> >=20
> > diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.ya=
> ml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > index 7ade03f1b32b..9578c7c206a2 100644
> > --- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > +++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > @@ -10,6 +10,15 @@ description: |
> > The Microchip CAP1xxx Family of RightTouchTM multiple-channel capaciti=
> ve
> > touch controllers and LED drivers. The device communication via I2C on=
> ly.
> > =20
> > + For more product information please see the links below:
> > + CAP1106: https://ww1.microchip.com/downloads/en/DeviceDoc/00001624B.=
> pdf
> > + CAP1126: https://ww1.microchip.com/downloads/en/DeviceDoc/00001623B.=
> pdf
> > + CAP1188: https://ww1.microchip.com/downloads/en/DeviceDoc/00001620C.=
> pdf
> > + CAP1203: https://ww1.microchip.com/downloads/en/DeviceDoc/00001572B.=
> pdf
> > + CAP1206: https://ww1.microchip.com/downloads/en/DeviceDoc/00001567B.=
> pdf
> > + CAP1293: https://ww1.microchip.com/downloads/en/DeviceDoc/00001566B.=
> pdf
> > + CAP1298: https://ww1.microchip.com/downloads/en/DeviceDoc/00001571B.=
> pdf
> > +
> > maintainers:
> > - Rob Herring <robh@kernel.org>
> > =20
> > @@ -124,14 +133,16 @@ properties:
> > The number of entries must correspond to the number of channels.
> > =20
> > patternProperties:
> > - "^led@[0-7]$":
> > + "^led@[0-9a-f]$":
>
> This should be done alongside the cap1114 change, not here I think. The
> constraint relaxation doesn't make sense because the user is arriving in
> a later patch.
> With it moved,
Thanks for your review feedback. I'll move this constraint change into
the cap1114 patch and update it in v4.
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
>
> Although, should it not be led@[0-9a-b] if the max is 11?
>
Agreed. I will fix this in v4.
> pw-bot: changes-requested
>
> Cheers,
> Conor.
>
> > type: object
> > description: CAP11xx LEDs
> > $ref: /schemas/leds/common.yaml#
> > =20
> > properties:
> > reg:
> > - enum: [0, 1, 2, 3, 4, 5, 6, 7]
> > + description: LED channel number
> > + minimum: 0
> > + maximum: 7
> > =20
> > label: true
> > =20
> > @@ -158,7 +169,7 @@ allOf:
> > - microchip,cap1298
> > then:
> > patternProperties:
> > - "^led@[0-7]$": false
> > + "^led@": false
> > =20
> > - if:
> > properties:
> > --=20
> > 2.54.0
> >=20
^ permalink raw reply
* Re: [PATCH v7 9/9] arm64: dts: mediatek: Add MediaTek MT6392 PMIC dtsi
From: Rob Herring @ 2026-06-16 13:39 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: linux-mediatek, Val Packett, Dmitry Torokhov, Krzysztof Kozlowski,
Conor Dooley, Sen Chu, Sean Wang, Macpaul Lin, Lee Jones,
Matthias Brugger, AngeloGioacchino Del Regno, Liam Girdwood,
Mark Brown, Linus Walleij, Louis-Alexis Eyraud, Julien Massot,
Fabien Parent, Akari Tsuyukusa, Chen Zhong, linux-input,
devicetree, linux-kernel, linux-pm, linux-arm-kernel, linux-gpio
In-Reply-To: <20260615071836.362883-10-l.scorcia@gmail.com>
On Mon, Jun 15, 2026 at 09:16:15AM +0200, Luca Leonardo Scorcia wrote:
> From: Val Packett <val@packett.cool>
>
> Add the dts to be included by all boards using the MT6392 PMIC,
> providing support for regulator, keys, pinctrl and RTC.
>
> Signed-off-by: Val Packett <val@packett.cool>
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
> ---
> arch/arm64/boot/dts/mediatek/mt6392.dtsi | 75 ++++++++++++++++++++++++
> 1 file changed, 75 insertions(+)
> create mode 100644 arch/arm64/boot/dts/mediatek/mt6392.dtsi
Nothing is using this so it is a dead file that doesn't get tested.
Rob
^ permalink raw reply
* Re: [PATCH 1/3] iio: hid-sensor-temperature: use u32 instead of unsigned int
From: sashiko-bot @ 2026-06-16 13:39 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260616-16-jun-hid-iio-u32-api-type-v1-1-952c74195f46@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Tear-down Order Inversion / Use-After-Free
--
commit fd38c3c8f2adbbb955807f745b5f193e1fd40a85
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: hid-sensor-temperature: use u32 instead of unsigned int
This commit updates the type of the usage_id variable from unsigned int
to u32 in the callback API. This improves code clarity and matches the
expected callback API type.
> diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
> index 9f628a8e5cfbe..a8d3a15f9c531 100644
> --- a/drivers/iio/temperature/hid-sensor-temperature.c
> +++ b/drivers/iio/temperature/hid-sensor-temperature.c
[ ... ]
> @@ -159,7 +159,7 @@ static int temperature_capture_sample(struct hid_sensor_hub_device *hsdev,
> static int temperature_parse_report(struct platform_device *pdev,
> struct hid_sensor_hub_device *hsdev,
> struct iio_chan_spec *channels,
> - unsigned int usage_id,
> + u32 usage_id,
> struct temperature_state *st)
> {
> int ret;
[Severity: High]
This is a pre-existing issue, but does hid_temperature_remove() tear down
the trigger and callback too early?
Because the driver uses devm_iio_device_register(), the IIO device will not
be unregistered until after the driver's remove function returns.
However, hid_temperature_remove() manually unregisters the trigger and
callbacks before that:
drivers/iio/temperature/hid-sensor-temperature.c:hid_temperature_remove() {
...
sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE);
hid_sensor_remove_trigger(indio_dev, &temp_st->common_attributes);
}
Could this leave a race condition window where the IIO device and its
sysfs/char interfaces are still fully accessible to userspace, but its
backing trigger and callbacks have already been destroyed?
Can concurrent access from userspace during this window lead to a
use-after-free?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-16-jun-hid-iio-u32-api-type-v1-0-952c74195f46@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v7 0/9] Add support for MT6392 PMIC
From: Rob Herring @ 2026-06-16 13:42 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: linux-mediatek, Dmitry Torokhov, Krzysztof Kozlowski,
Conor Dooley, Sen Chu, Sean Wang, Macpaul Lin, Lee Jones,
Matthias Brugger, AngeloGioacchino Del Regno, Liam Girdwood,
Mark Brown, Linus Walleij, Julien Massot, Louis-Alexis Eyraud,
Val Packett, Fabien Parent, Akari Tsuyukusa, Chen Zhong,
linux-input, devicetree, linux-kernel, linux-pm, linux-arm-kernel,
linux-gpio
In-Reply-To: <20260615071836.362883-1-l.scorcia@gmail.com>
On Mon, Jun 15, 2026 at 09:16:06AM +0200, Luca Leonardo Scorcia wrote:
> The MediaTek MT6392 PMIC is usually found on devices powered by
> the MT8516/MT8167 SoC and is yet another MT6323/MT6397 variant.
>
> This series is mostly based around patches submitted a couple
> years ago by Fabien Parent and not merged and from Val Packett's
> submission from Jan 2025 that included extra cleanups, fixes, and a
> new dtsi file similar to ones that exist for other PMICs. Some
> comments weren't addressed and the series was ultimately not merged.
>
> These patches enable four functions: keys, regulator, pinctrl and RTC.
> Mono speaker amp will follow later as I need to work further on the
> audio codec.
>
> I added a handful of device tree improvements to fix some dtbs_check
> errors, added support for the pinctrl device and addressed the comments
> from last year's reviews.
>
> Please note that patch 0006 and 0008 depend on patch 0005 as they need the
> registers.h file, but belong to different driver areas. I'm not sure if
> I'm supposed to squash them even if they belong to different driver
> areas of if it's fine like this. Any advice is welcome.
>
> The series has been tested on Xiaomi Mi Smart Clock X04G and on the
> Lenovo Smart Clock 2 CD-24502F.
>
> Changes in v7:
> - Removed patch 0008 dependency on patch 0003.
> - Reintroduced the regulator driver. In earlier revisions of this series,
> it was proposed to remove the dedicated compatible for the regulator
> device [3]. The driver does not use actually it, but it is not possible
> at this time to remove it from the bindings since it's a required
> property.
>
> Making the regulator-required property conditional was NACKed in [5],
> with the suggestion to create a separate binding altogether for devices
> that do not require the compatible property. I tried implementing this,
> but since the parent device needs to be declared as compatible with
> mt6323, it leads to a warning in dt_binding_check since mt6323 would
> be declared as a compatible in both mt6392 and mt6397.
>
> In the end the only regulator driver from the mt6397 documentation that
> still declares an of_match is mt6397-regulator and it does not seem
> to be necessary, so it should be possible to remove it and make the
> regulator compatible optional for all regulators, but that change would
> probably deserve its own separate patch series.
I don't really follow what the issue is here, but compatible should
never be optional.
Rob
^ permalink raw reply
* Re: [PATCH v7 1/9] dt-bindings: mfd: mt6397: Add MT6392 PMIC
From: Rob Herring (Arm) @ 2026-06-16 13:43 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: Lee Jones, Mark Brown, linux-input, linux-pm, Louis-Alexis Eyraud,
linux-kernel, Julien Massot, Macpaul Lin, Krzysztof Kozlowski,
Liam Girdwood, Linus Walleij, Conor Dooley, Dmitry Torokhov,
Akari Tsuyukusa, AngeloGioacchino Del Regno, Chen Zhong,
Fabien Parent, linux-mediatek, Val Packett, linux-arm-kernel,
Matthias Brugger, devicetree, Sean Wang, Sen Chu, linux-gpio
In-Reply-To: <20260615071836.362883-2-l.scorcia@gmail.com>
On Mon, 15 Jun 2026 09:16:07 +0200, Luca Leonardo Scorcia wrote:
> Describe the MT6392 PMIC and its RTC and regulator devices. This device
> is mostly based on MT6323 with some similarities to MT6397 and is usually
> found on boards using the MT8516/MT8167 SoC.
>
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
> ---
> .../devicetree/bindings/mfd/mediatek,mt6397.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 2/3] iio: hid-sensor-humidity: use u32 instead of unsigned int
From: sashiko-bot @ 2026-06-16 13:50 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260616-16-jun-hid-iio-u32-api-type-v1-2-952c74195f46@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Directly dereferencing the `raw_data` pointer as an `s32` can cause unaligned access faults on architectures that strictly enforce alignment.
- [High] The driver unconditionally dereferences `raw_data` as a 32-bit integer, ignoring the `raw_len` parameter, leading to potential out-of-bounds memory reads and corrupted sensor data.
- [High] The `humidity_callbacks` structure is statically allocated but its `pdev` field is modified during `probe`. If multiple humidity sensors are probed, the last probed device overwrites `pdev`, causing callbacks for all instances to be routed to the last bound device's state.
--
commit 90e5b3ef32381e14224dfd5cf7711e2aa3c588a2
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: hid-sensor-humidity: use u32 instead of unsigned int
This commit updates the usage_id parameter type from unsigned int to u32
in callback functions to match the expected API. It is intended to improve
code clarity with no functional changes.
> diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
> index be2338d5f4070..c376a247b1372 100644
> --- a/drivers/iio/humidity/hid-sensor-humidity.c
> +++ b/drivers/iio/humidity/hid-sensor-humidity.c
[ ... ]
> @@ -141,7 +141,7 @@ static int humidity_proc_event(struct hid_sensor_hub_device *hsdev,
>
> /* Capture samples in local storage */
> static int humidity_capture_sample(struct hid_sensor_hub_device *hsdev,
> - unsigned int usage_id, size_t raw_len,
> + u32 usage_id, size_t raw_len,
> char *raw_data, void *pdev)
> {
[Severity: High]
This is a pre-existing issue, but does casting the raw_data pointer directly to
an s32 pointer cause unaligned access faults on architectures that strictly
enforce alignment?
drivers/iio/humidity/hid-sensor-humidity.c:humidity_capture_sample() {
switch (usage_id) {
case HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY:
humid_st->scan.humidity_data = *(s32 *)raw_data;
...
}
The raw_data pointer points directly into the packed HID report byte stream,
which may be unaligned.
Could casting it directly to an s32 pointer and dereferencing it trigger
alignment faults on architectures that do not support hardware unaligned
accesses?
[Severity: High]
This is also a pre-existing issue, but does this function unconditionally read 4
bytes from raw_data, ignoring the raw_len parameter?
drivers/iio/humidity/hid-sensor-humidity.c:humidity_capture_sample() {
switch (usage_id) {
case HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY:
humid_st->scan.humidity_data = *(s32 *)raw_data;
...
}
By indiscriminately reading 4 bytes, the driver might access out-of-bounds
memory when raw_len is smaller than 4.
Could this trigger memory access faults near page boundaries and leak kernel
heap data to userspace if the upper bits contain adjacent memory garbage?
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> @@ -161,7 +161,7 @@ static int humidity_capture_sample(struct hid_sensor_hub_device *hsdev,
> static int humidity_parse_report(struct platform_device *pdev,
> struct hid_sensor_hub_device *hsdev,
> struct iio_chan_spec *channels,
> - unsigned int usage_id,
> + u32 usage_id,
> struct hid_humidity_state *st)
> {
> int ret;
[Severity: High]
This is a pre-existing issue, but does the shared static humidity_callbacks
structure get overwritten during probe?
drivers/iio/humidity/hid-sensor-humidity.c:
static struct hid_sensor_hub_callbacks humidity_callbacks = {
.send_event = &humidity_proc_event,
.capture_sample = &humidity_capture_sample,
};
drivers/iio/humidity/hid-sensor-humidity.c:hid_humidity_probe() {
...
humidity_callbacks.pdev = pdev;
...
}
If multiple humidity sensors are probed, the last probed device overwrites pdev.
When sensor_hub_raw_event() receives data, it invokes callback->capture_sample
with the overwritten pdev.
Could this cause platform_get_drvdata(pdev) to return the wrong IIO device,
corrupting its state and dropping data for all previously bound sensors?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-16-jun-hid-iio-u32-api-type-v1-0-952c74195f46@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v7 3/9] regulator: dt-bindings: Add MediaTek MT6392 PMIC
From: Rob Herring @ 2026-06-16 13:50 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: linux-mediatek, Dmitry Torokhov, Krzysztof Kozlowski,
Conor Dooley, Sen Chu, Sean Wang, Macpaul Lin, Lee Jones,
Matthias Brugger, AngeloGioacchino Del Regno, Liam Girdwood,
Mark Brown, Linus Walleij, Val Packett, Julien Massot,
Louis-Alexis Eyraud, Fabien Parent, Akari Tsuyukusa, Chen Zhong,
linux-input, devicetree, linux-kernel, linux-pm, linux-arm-kernel,
linux-gpio
In-Reply-To: <20260615071836.362883-4-l.scorcia@gmail.com>
On Mon, Jun 15, 2026 at 09:16:09AM +0200, Luca Leonardo Scorcia wrote:
> Add bindings for the regulators found in the MediaTek MT6392 PMIC,
> usually found in board designs using the MediaTek MT8516/MT8167 SoCs.
>
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
> ---
> .../regulator/mediatek,mt6392-regulator.yaml | 234 ++++++++++++++++++
> .../regulator/mediatek,mt6392-regulator.h | 24 ++
> 2 files changed, 258 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt6392-regulator.yaml
> create mode 100644 include/dt-bindings/regulator/mediatek,mt6392-regulator.h
>
> diff --git a/Documentation/devicetree/bindings/regulator/mediatek,mt6392-regulator.yaml b/Documentation/devicetree/bindings/regulator/mediatek,mt6392-regulator.yaml
> new file mode 100644
> index 000000000000..197041df4ba1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/mediatek,mt6392-regulator.yaml
> @@ -0,0 +1,234 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/mediatek,mt6392-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek MT6392 regulator
> +
> +maintainers:
> + - Luca Leonardo Scorcia <l.scorcia@gmail.com>
> +
> +description:
> + MT6392 is a power management system chip containing three buck converters and
> + 23 LDOs. All voltage regulators provided by the PMIC are described as
> + sub-nodes of this node.
> +
> +properties:
> + compatible:
> + items:
> + - const: mediatek,mt6392-regulator
> +
> + vproc-supply:
> + description: Supply for buck regulator vproc
> + vcore-supply:
> + description: Supply for buck regulator vcore
> + vsys-supply:
> + description: Supply for buck regulator vsys
> + avddldo-supply:
> + description: |
Don't need '|' if no formatting to preserve. Elsewhere too.
> + Supply for AVDD LDOs (vm, vio18, vcn18, vcamd, vcamio). According to the data sheet
> + this is an internal supply derived from vsys.
> + ldo1-supply:
> + description: Supply for LDOs group 1 (vaud28, vxo22, vaud22, vadc18, vcama, vrtc)
> + ldo2-supply:
> + description: Supply for LDOs group 2 (vcn35, vio28, vmc, vmch, vefuse, vdig18)
> + ldo3-supply:
> + description: Supply for LDOs group 3 (vusb, vemc3v3, vcamaf, vgp1, vgp2, vm25)
> +
> +patternProperties:
> + "^v(core|proc|sys)$":
> + description: Buck regulators
> + type: object
> + $ref: regulator.yaml#
> + properties:
> + regulator-allowed-modes:
> + description: |
> + BUCK regulators can set regulator-initial-mode and regulator-allowed-modes to
> + values specified in dt-bindings/regulator/mediatek,mt6392-regulator.h
> + items:
> + enum: [0, 1]
minItems: 1
maxItems: 2
? Because if there are only 2 modes, can't have more entries than that,
right? Though wouldn't 2 entries be the same as no property present
because I would assume the default is all modes. I shouldn't have to
assume though.
> + unevaluatedProperties: false
Place this after $ref. Easier to read than after indented blocks.
> +
> + "^v(adc18|camio|cn18|io18|xo22|m25|aud28|io28|rtc|usb)$":
> + description: LDOs with fixed output and mode setting
> + type: object
> + $ref: regulator.yaml#
> + properties:
> + regulator-allowed-modes:
> + description: |
> + LDO regulators can set regulator-initial-mode and regulator-allowed-modes to
> + values specified in dt-bindings/regulator/mediatek,mt6392-regulator.h
> + items:
> + enum: [0, 1]
> + unevaluatedProperties: false
> +
> + "^v(cama|dig18)$":
> + description: LDOs with fixed output without mode setting
> + type: object
> + $ref: regulator.yaml#
> + unevaluatedProperties: false
> +
> + "^v(aud22|camaf|camd|cn35|efuse|emc3v3|gp1|gp2|m|mc|mch)$":
> + description: LDOs with adjustable output
> + type: object
> + $ref: regulator.yaml#
> + properties:
> + regulator-allowed-modes: false
> + unevaluatedProperties: false
> +
> +required:
> + - compatible
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + regulators {
> + compatible = "mediatek,mt6392-regulator";
Drop the example. Put 1 complete example in the MFD schema rather than
incomplete examples.
Rob
^ 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