* [PATCH 01/11] Input: ims-pcu - release data interface on disconnect
From: Dmitry Torokhov @ 2026-05-23 5:06 UTC (permalink / raw)
To: linux-input
Cc: Thomas Fourier, Seungjin Bae, linux-kernel, stable, Sashiko bot
During probe the driver claims the data interface, but it never releases
it. Release it in disconnect to avoid leaving it permanently claimed.
Fixes: 628329d52474 ("Input: add IMS Passenger Control Unit driver")
Cc: stable@vger.kernel.org
Reported-by: Sashiko bot <sashiko-bot@kernel.org>
Assisted-by: Gemini:gemini-3.1-pro
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/ims-pcu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
index 7a1cb9333f53..57d917387544 100644
--- a/drivers/input/misc/ims-pcu.c
+++ b/drivers/input/misc/ims-pcu.c
@@ -2090,6 +2090,7 @@ static void ims_pcu_disconnect(struct usb_interface *intf)
ims_pcu_destroy_application_mode(pcu);
ims_pcu_buffers_free(pcu);
+ usb_driver_release_interface(&ims_pcu_driver, pcu->data_intf);
kfree(pcu);
}
--
2.54.0.746.g67dd491aae-goog
^ permalink raw reply related
* Re: [PATCH] Input: ims-pcu - fix usb_free_coherent() size in ims_pcu_buffers_alloc()
From: Dmitry Torokhov @ 2026-05-23 5:05 UTC (permalink / raw)
To: Thomas Fourier
Cc: stable, Dan Carpenter, Seungjin Bae, Sanghoon Choi, Kees Cook,
linux-input, linux-kernel
In-Reply-To: <20260522085412.45430-2-fourier.thomas@gmail.com>
On Fri, May 22, 2026 at 10:54:04AM +0200, Thomas Fourier wrote:
> The input buffer size is pcu->max_in_size, but pcu->max_out_size is
> passed to usb_free_coherent().
>
> Change size to match the allocation size.
>
> Fixes: 628329d52474 ("Input: add IMS Passenger Control Unit driver")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: Add support for HID++ Multi-Platform feature (0x4531)
From: Greg KH @ 2026-05-23 4:12 UTC (permalink / raw)
To: dev exalt
Cc: Bastien Nocera, jikos, bentiss, lains, linux-input, linux-kernel,
sari.kreitem, hbarnor
In-Reply-To: <CAJaUH_8Tg=QCB96aFBu_zvJd03EN6zgBvS8RGGvN4UonjTJ7_w@mail.gmail.com>
On Thu, May 21, 2026 at 12:27:05PM +0300, dev exalt wrote:
> Hi,
>
> Following up on this thread regarding HID++ Multi-Platform feature
> (0x4531) support.
> Please let us know if you’d like us to proceed with a V2 patch based
> on the discussed approach.
As-is, a module parameter will not work. All systems have configfs, and
sysfs, so use that for device configurations that need it.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 2/3] Input: adafruit-seesaw - add interrupt support
From: Charles Dias @ 2026-05-22 20:32 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Anshul Dalal, Shuah Khan, Brigham Campbell, linux-input,
linux-kernel, Charles Dias
In-Reply-To: <CAHua+AOOy6NSQJUPHr7-VQp2zv6ggq0PXjkCbboN_Y2gY+zZ+Q@mail.gmail.com>
On Sun, Apr 19, 2026 at 4:24 PM Charles Dias <charles.embedded@gmail.com> wrote:
>
> On Tue, Mar 24, 2026 at 10:59 AM Charles Dias
> <charles.embedded@gmail.com> wrote:
> >
> > On Mon, Mar 23, 2026 at 2:12 AM Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> > >
> > > Hi Charles,
> > >
> > > On Sat, Mar 21, 2026 at 05:24:45PM -0300, charles.embedded@gmail.com wrote:
> > > > @@ -289,6 +341,19 @@ static int seesaw_probe(struct i2c_client *client)
> > > > input_set_max_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_MAX);
> > > > input_set_min_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_MIN);
> > > >
> > > > + if (client->irq) {
> > > > + err = seesaw_register_write_u32(client, SEESAW_GPIO_INTENSET, SEESAW_BUTTON_MASK);
> > > > + if (err)
> > > > + return dev_err_probe(&client->dev, err,
> > > > + "failed to enable hardware interrupts\n");
> > >
> > > Maybe this should be in seesaw_open()?
> > >
> > > Thanks.
> > >
> > > --
> > > Dmitry
> >
> > Hi Dmitry. Thank you for your review!
> >
> > Since this is a one-time setup, I believe it should remain as is
> > within the seesaw_probe() function, similar to other pin
> > configurations.
> >
> > Please let me know if I'm missing any point here.
> >
> > Best regards,
> > Charles Dias
>
> Hi Dmitry,
>
> I just wanted to follow up on my previous response regarding the initialization.
>
> Please let me know if there is anything else you'd like me to address
> or if there are any further changes needed on my end. Thank you!
>
> Best regards,
> Charles Dias
Hi Dmitry,
I would like to check whether my previous reply addressed your concern.
Best regards,
Charles Dias.
^ permalink raw reply
* [PATCH v2 2/2] arm64: dts: glymur-crd: Add reset GPIO to touchscreen node
From: Pradyot Kumar Nayak @ 2026-05-22 19:55 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Abel Vesa, Dmitry Baryshkov, Benjamin Tissoires,
Jiri Kosina
Cc: linux-arm-msm, linux-input, Konrad Dybcio, devicetree,
linux-kernel,
20260519-glymur-mahua-common-nodes-v2-1-9f1b89a3f398,
Pradyot Kumar Nayak
In-Reply-To: <20260523-arm64-dts-glymur-crd-add-reset-gpio-to-touchscreen-v2-0-5c333051e5bb@oss.qualcomm.com>
The touchscreen module on Glymur/Mahua CRDs is different from the one
used on Hamoa CRDs and requires the reset-gpios to be wired to the device.
Without this in place the reset line will remain permanently asserted
during resume leaving the device offline and causing all I2C transactions
to fail with -ENXIO.
Error Logs:
i2c_hid_of 3-0038: failed to change power setting.
i2c_hid_of 3-0038: PM: dpm_run_callback(): i2c_hid_core_pm_resume [i2c_hid] returns -6
i2c_hid_of 3-0038: PM: failed to resume async: error -6
Add the reset GPIO so the driver can deassert the line on resume,
restoring I2C communication with the device.
Fixes: e6bf559f7eb9 ("arm64: dts: qcom: glymur-crd: Enable keyboard, trackpad and touchscreen")
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Pradyot Kumar Nayak <pradyot.nayak@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/glymur-crd.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/glymur-crd.dtsi b/arch/arm64/boot/dts/qcom/glymur-crd.dtsi
index 1de3a49f49e4..2259577b9ae1 100644
--- a/arch/arm64/boot/dts/qcom/glymur-crd.dtsi
+++ b/arch/arm64/boot/dts/qcom/glymur-crd.dtsi
@@ -623,6 +623,8 @@ touchscreen@38 {
vdd-supply = <&vreg_misc_3p3>;
vddl-supply = <&vreg_l15b_e0_1p8>;
+ reset-gpios = <&tlmm 48 GPIO_ACTIVE_LOW>;
+
pinctrl-0 = <&ts0_default>;
pinctrl-names = "default";
};
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/2] dt-bindings: input: hid-over-i2c: Add reset-gpios property
From: Pradyot Kumar Nayak @ 2026-05-22 19:55 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Abel Vesa, Dmitry Baryshkov, Benjamin Tissoires,
Jiri Kosina
Cc: linux-arm-msm, linux-input, Konrad Dybcio, devicetree,
linux-kernel,
20260519-glymur-mahua-common-nodes-v2-1-9f1b89a3f398,
Pradyot Kumar Nayak
In-Reply-To: <20260523-arm64-dts-glymur-crd-add-reset-gpio-to-touchscreen-v2-0-5c333051e5bb@oss.qualcomm.com>
Document reset-gpios property for devices that explicitly require
the reset line to be de-asserted during power up.
Signed-off-by: Pradyot Kumar Nayak <pradyot.nayak@oss.qualcomm.com>
---
Documentation/devicetree/bindings/input/hid-over-i2c.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.yaml b/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
index 138caad96a29..fe0ec4fce673 100644
--- a/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
+++ b/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
@@ -50,6 +50,10 @@ properties:
description: Time required by the device after enabling its regulators
or powering it on, before it is ready for communication.
+ reset-gpios:
+ maxItems: 1
+ description: GPIO line used to reset the device.
+
touchscreen-inverted-x: true
touchscreen-inverted-y: true
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/2] arm64: dts: glymur-crd: Add reset GPIO to touchscreen
From: Pradyot Kumar Nayak @ 2026-05-22 19:55 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Abel Vesa, Dmitry Baryshkov, Benjamin Tissoires,
Jiri Kosina
Cc: linux-arm-msm, linux-input, Konrad Dybcio, devicetree,
linux-kernel,
20260519-glymur-mahua-common-nodes-v2-1-9f1b89a3f398,
Pradyot Kumar Nayak
The touchscreen on Glymur/Mahua CRDs has a dedicated GPIO reset line that must
be explicitly modeled in DT so the i2c-hid driver can deassert it on resume.
Without this, the device stays offline after suspend/resume and all I2C
transactions fail with -ENXIO.
Patch 1 adds reset-gpios to the hid-over-i2c binding.
Patch 2 wires the GPIO in the Glymur CRD DTSI.
prerequisite-message-id: 20260519-glymur-mahua-common-nodes-v2-1-9f1b89a3f398@oss.qualcomm.com
Signed-off-by: Pradyot Kumar Nayak <pradyot.nayak@oss.qualcomm.com>
---
Changes in v2:
- Add dt-bindings patch to document reset-gpios in hid-over-i2c schema
(flagged by dtbs_check on v1)
- No change to the DTS patch itself
- Link to v1: https://lore.kernel.org/r/20260522-arm64-dts-glymur-crd-add-reset-gpio-to-touchscreen-v1-1-c7653924acdc@oss.qualcomm.com
---
Pradyot Kumar Nayak (2):
dt-bindings: input: hid-over-i2c: Add reset-gpios property
arm64: dts: glymur-crd: Add reset GPIO to touchscreen node
Documentation/devicetree/bindings/input/hid-over-i2c.yaml | 4 ++++
arch/arm64/boot/dts/qcom/glymur-crd.dtsi | 2 ++
2 files changed, 6 insertions(+)
---
base-commit: 1d376e58cb4a94f89de0fa9195d583ad6b86eb89
change-id: 20260522-arm64-dts-glymur-crd-add-reset-gpio-to-touchscreen-5892beded29b
Best regards,
--
Pradyot Kumar Nayak <pradyot.nayak@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH v4 0/6] iio: hid-sensor: standardize scan_type initialization
From: Jonathan Cameron @ 2026-05-22 16:51 UTC (permalink / raw)
To: Natália Salvino André
Cc: andy, dlechner, jikos, nuno.sa, srinivas.pandruvada, linux-iio,
linux-input, Zhang, Lixu
In-Reply-To: <20260519234752.23911-1-natalia.andre@ime.usp.br>
On Tue, 19 May 2026 20:40:42 -0300
Natália Salvino André <natalia.andre@ime.usp.br> wrote:
> This series refactors the HID sensor drivers to standardize the
> initialization of the iio_chan_spec scan_type structure using compound
> literals.
>
> This change improves code readability and ensures that all fields of
> the scan_type structure are properly zero-initialized, allowing the
> removal of local helper functions.
>
> Additionally, the channel initialization loops for hid-sensor-accel-3d
> and hid-sensor-gyro-3d were cleaned up to iterate directly over the
> scan indices, eliminating redundant index-offset logic.
+CC Lixu
Please make sure to add to your +CC anyone who has been involve in earlier
versions of the series.
I'm going to queue this now to get some build coverage. Still fine
to add tags though or indeed drop it if any problems.
Applied to the testing branch of iio.git
thanks,
Jonathan
>
> ---
>
> Changes in v4:
>
> - Fixed a bug in the sensor_hub_input_get_attribute_info()
> call where passing the raw loop index 'ch' broke the HID report
> field lookup in hid-sensor-accel-3d. Restored the proper HID
> usage ID offset calculation using HID_USAGE_SENSOR_ACCEL_X_AXIS + ch
> - Cleaned up the channel initialization loop to iterate
> directly over the scan indices in hid-sensor-gyro-3d for consistency
>
> Changes in v3:
>
> - Dropped the global helper function implementation from
> hid-sensor-attributes.c following maintainer feedback.
> - Shifted to local refactoring inside the drivers using compound
> literals and the macros BYTES_TO_BITS() and BITS_PER_TYPE()
> - Refactored the channel initialization loop in hid-sensor-accel-3d to
> iterate directly over the scan enums instead of using 0-based
> index offsets
> - v3 link: https://lore.kernel.org/linux-iio/89a2e6775e3c922a4848a4b4730aab8d32097454.camel@linux.intel.com/T/#t
>
> Changes in v2:
>
> - Modified the helper function to use the BITS_PER_BYTE macro instead of
> magic numbers for bit calculations
> - Updated the scan_type structure field assignment from '.sign'
> to '.format' based on recent subsystem renaming feedback
>
> Natália Salvino André (6):
> iio: accel: HID: hid-sensor-accel-3d: Refactor channel initialization
> iio: gyro: HID: hid-sensor-gyro-3d: Refactor channel initialization
> iio: light: HID: hid-sensor-als: Refactor channel initialization
> iio: light: HID: hid-sensor-prox: Refactor channel initialization
> iio: magnetometer: HID: hid-sensor-magn-3d: Refactor channel
> initialization
> iio: pressure: HID: hid-sensor-press: Refactor channel initialization
>
> drivers/iio/accel/hid-sensor-accel-3d.c | 27 +++++++------------
> drivers/iio/gyro/hid-sensor-gyro-3d.c | 27 +++++++------------
> drivers/iio/light/hid-sensor-als.c | 18 +++++--------
> drivers/iio/light/hid-sensor-prox.c | 19 +++++--------
> drivers/iio/magnetometer/hid-sensor-magn-3d.c | 20 +++++---------
> drivers/iio/pressure/hid-sensor-press.c | 19 +++++--------
> 6 files changed, 42 insertions(+), 88 deletions(-)
>
^ permalink raw reply
* Re: [PATCH] Input: synaptics - add quirk for Daffodil DC253D touchpad
From: sashiko-bot @ 2026-05-22 15:43 UTC (permalink / raw)
To: Ashwin Gundarapu; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <19e5047ead1.ce62bdb39017.6022546617731203250@zohomail.in>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The newly added DMI quirk is bypassed if the touchpad initializes in SMBus (InterTouch) mode.
- [Medium] Using exact string matching with dmi_match() instead of a dmi_system_id table.
- [Low] Spaces are used instead of tabs for indentation.
--
commit 49d6473335088b14bbb30544a1f0fe866d10ddb5
Author: Ashwin Gundarapu <linuxuser509@zohomail.in>
Input: synaptics - add quirk for Daffodil DC253D touchpad
This commit adds a DMI quirk to clear the CLICKPAD capability bit for the
Daffodil DC253D touchpad. It aims to fix an issue where the device is
incorrectly detected as a clickpad, causing the physical right button to be
missing.
Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2153983
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 26071128f43a7..4ca85fb4ab70d 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -534,9 +534,16 @@ static int synaptics_invert_y(int y)
> * Apply quirk(s) if the hardware matches
> */
> static void synaptics_apply_quirks(struct psmouse *psmouse,
[Severity: High]
Does this logic get bypassed if the touchpad initializes in SMBus
(InterTouch) mode?
It looks like synaptics_apply_quirks() is only called from the PS/2 fallback
initialization path (synaptics_init_ps2()). During device initialization in
synaptics_init(), if InterTouch (SMBus) is supported, the setup is routed to
synaptics_setup_intertouch().
This SMBus path directly evaluates SYN_CAP_CLICKPAD(info->ext_cap_0c) to
configure the .buttonpad property for the RMI4 driver without ever running
synaptics_apply_quirks(). This would cause the intended fix to fail on modern
systems using InterTouch.
> - struct synaptics_device_info *info)
> + struct synaptics_device_info *info)
> {
> - int i;
> + int i;
[Severity: Low]
This isn't a bug, but it looks like spaces were used for indentation here and
in the modified function arguments instead of tabs. The Linux kernel coding
style mandates the use of 8-character tabs for indentation.
> +
> + /* Daffodil DC253D has a physical right button */
> + if (dmi_match(DMI_SYS_VENDOR, "Daffodil Computers Ltd.") &&
> + dmi_match(DMI_PRODUCT_NAME, "DC253D")) {
[Severity: Medium]
Will this exact string matching fail if the DMI strings contain trailing
whitespace or padding?
The dmi_match() function uses exact string matching. DMI strings from OEM BIOS
vendors frequently include trailing whitespace or padding, which would cause
this match to fail and the quirk to be silently ignored.
The standard and more robust approach used throughout the kernel and within
synaptics.c is to use dmi_check_system() with a struct dmi_system_id table.
This utilizes DMI_MATCH() to perform substring matching, safely ignoring any
trailing spaces.
> + info->ext_cap_0c &= ~BIT(20); /* Clear CLICKPAD bit */
> + psmouse_info(psmouse, "Force right button on Daffodil DC253D\n");
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/19e5047ead1.ce62bdb39017.6022546617731203250@zohomail.in?part=1
^ permalink raw reply
* [PATCH] Input: synaptics - add quirk for Daffodil DC253D touchpad
From: Ashwin Gundarapu @ 2026-05-22 15:22 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, dmitrytorokhov, ubuntu-kernel, hasan6l
From ee16efae3c4371d37467c49c0cae6af808e1b625 Mon Sep 17 00:00:00 2001
From: Ashwin Gundarapu <linuxuser509@zohomail.in>
Date: Fri, 22 May 2026 20:48:25 +0530
Subject: [PATCH] Input: synaptics - add quirk for Daffodil DC253D touchpad
On the Daffodil DC253D laptop, the touchpad is incorrectly detected as
a clickpad, causing the right button to be missing. Add a DMI quirk
to clear the CLICKPAD bit for this device.
Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2153983
Signed-off-by: Ashwin Gundarapu <linuxuser509@zohomail.in>
---
drivers/input/mouse/synaptics.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 26071128f43a..4ca85fb4ab70 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -534,9 +534,16 @@ static int synaptics_invert_y(int y)
* Apply quirk(s) if the hardware matches
*/
static void synaptics_apply_quirks(struct psmouse *psmouse,
- struct synaptics_device_info *info)
+ struct synaptics_device_info *info)
{
- int i;
+ int i;
+
+ /* Daffodil DC253D has a physical right button */
+ if (dmi_match(DMI_SYS_VENDOR, "Daffodil Computers Ltd.") &&
+ dmi_match(DMI_PRODUCT_NAME, "DC253D")) {
+ info->ext_cap_0c &= ~BIT(20); /* Clear CLICKPAD bit */
+ psmouse_info(psmouse, "Force right button on Daffodil DC253D\n");
+ }
for (i = 0; min_max_pnpid_table[i].pnp_ids; i++) {
if (!psmouse_matches_pnp_id(psmouse,
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 00/11] HID: storing pointers in 'hid_device_id::driver_data'
From: Pawel Zalewski @ 2026-05-22 14:18 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Ping Cheng, Jason Gerecke, linux-kernel, linux-input,
Christian A. Ehrhardt, Christian A. Ehrhardt
In-Reply-To: <ag8bARsGNVqA8xbQ@beelink>
> I would agree with sashiko that the series introduces an anti-pattern by
> allowing .driver_data to be an arbitrary pointer. The reason is that we
> can dynamically set driver_data through the kernel command line, and
> when it's used as a pointer, bad things will happen.
I have not considered the command line override ! This indeed would not
be a good case for having a pointer at all in there - in the current
form of the codebase.
The series does not introduce this anti-pattern - this is already the
current status quo in the codebase and the problem predates the patch
series itself. The series makes the problem more visible and validates
the existing status quo - agreed on this.
> I would think we should fix the 2 offenders to enforce not using a
> direct pointer but an offset in a table of pointers.
>
> How does that sound for you?
That could work, probably via a named enum (so that the idx is bounded
and validated to avoid out of bounds memory access) with its items mapped
into a table of const pointers. That way the command line override still
works as before and my goals set out in the series are met as well.
Perhaps an alternative solution would be to sanitize the user-input instead in
the 'hid-core::new_id_store' function, such that only 'driver_data' values
that match an existing entry in the driver's 'id_table' are accepted, this
is how it is done currently in the PCI subsystem in 'pci-driver::new_id_store',
so it would be consistent as well.
The bonus here is that all other drivers in the HID subsystem benefit from
this aproach as opposed to just the two current offenders, as providing
'driver_data' from the command line is now mandatory and must match the
existing table entry - which is what we want ?
So something among the lines of:
@hid-core::new_id_store
```
struct hid_driver *hdrv = to_hid_driver(drv);
const struct hid_device_id *ids = hdrv->id_table;
(...)
/* Only accept driver_data values that match an existing id_table
entry */
if (ids) {
ret = -EINVAL;
while (ids->vendor || ids->product) {
if (driver_data == ids->driver_data) {
ret = 0;
break;
}
ids++;
}
if (ret) /* No match */
return ret;
}
```
Would that approach work for you ? Do help in what is the correct
termination for the while loop - for this demo I went with 'ids->vendor'
and 'ids->product' fields as common-sense would indicate that quirks need
to be device-specific by definition but that was without any actual research.
The downside is that 'new_id' drivers that actually use a pointer for the
'driver_data' will be rejected - which would be actually covered by the
solution that you have proposed in the first place. To think of it the
input sanitizer could work additionaly in tandem with the enum-based
solution if this fact is an issue, so it might not be an alternative
actually but a complement for it.
In the meantime I think that patches 1-8 can be reviewed regardless
without any changes (as they predate patch 9 that adds the union and the
commit messages does not mention anything about it).
Best regards,
Pawel
^ permalink raw reply
* Re: [PATCH 02/10] [v3] input: gpio-keys: make legacy gpiolib optional
From: Matti Vaittinen @ 2026-05-22 12:45 UTC (permalink / raw)
To: Arnd Bergmann, Arnd Bergmann, open list:GPIO SUBSYSTEM
Cc: linux-kernel, Christian Lamparter, Johannes Berg, Aaro Koskinen,
Andreas Kemnade, Kevin Hilman, Roger Quadros, Tony Lindgren,
Thomas Bogendoerfer, John Paul Adrian Glaubitz, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Linus Walleij, Bartosz Golaszewski, Dmitry Torokhov, Lee Jones,
Pavel Machek, Florian Fainelli, Jonas Gorski, Andrew Lunn,
Vladimir Oltean, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, linux-wireless, Linux-OMAP, linux-arm-kernel,
linux-mips, linux-sh, linux-input, linux-leds, Netdev
In-Reply-To: <681f328c-bc8a-4410-b876-f8c5f116325d@app.fastmail.com>
On 22/05/2026 11:28, Arnd Bergmann wrote:
> On Fri, May 22, 2026, at 06:55, Matti Vaittinen wrote:
>> On 20/05/2026 21:38, Arnd Bergmann wrote:
>>> From: Arnd Bergmann <arnd@arndb.de>
>>>
>>> The two Rohm PMIC drivers use a gpio-keys device without an actual GPIO,
>>> passing an IRQ number instead. In order to keep this working both with
>>> and with CONFIG_GPIOLIB_LEGACY, change the gpio-keys driver to ignore
>>> the gpio number if an IRQ is passed.
>>>
>>
>> I am (still) all fine with this, even though I like Dmitry's set. I
>> suppose you already have a plan for merging this, but I still have to
>> ask - why the MFD changes aren't in own patch? I feel it would have
>> simplified merging, backporting, reviewing and reverting if needed.
>
> Splitting it out would break bisection: The gpio-leds change without
> the mfd change causes a build failure when assigning the .gpio
> field, and the reverse causes a runtime failure when .gpio=0 is
> a valid line.
Ah, yes. Thanks for explanation. (I feel a bit [a lot] stupid, as I
somehow missed the problem of .gpio = 0 being valid. That even though I
had it explicitly set to -1 myself :rolleyes: ).
No further questions ;)
Yours,
-- Matti
--
---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* [syzbot] Monthly input report (May 2026)
From: syzbot @ 2026-05-22 12:32 UTC (permalink / raw)
To: linux-input, linux-kernel, syzkaller-bugs
Hello input maintainers/developers,
This is a 31-day syzbot report for the input subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/input
During the period, 2 new issues were detected and 0 were fixed.
In total, 21 issues are still open and 63 have already been fixed.
There are also 2 low-priority issues.
Some of the still happening issues:
Ref Crashes Repro Title
<1> 184 Yes WARNING in cm109_input_open/usb_submit_urb (3)
https://syzkaller.appspot.com/bug?extid=ac0f9c4cc1e034160492
<2> 99 No KASAN: slab-use-after-free Read in report_descriptor_read
https://syzkaller.appspot.com/bug?extid=bc537ca7a0efe33988eb
<3> 22 Yes INFO: task hung in __input_unregister_device (5)
https://syzkaller.appspot.com/bug?extid=78e2288f58b881ed3c45
<4> 18 Yes INFO: task hung in console_callback (6)
https://syzkaller.appspot.com/bug?extid=6027421afa74a2ba440d
<5> 1983 No possible deadlock in evdev_pass_values (2)
https://syzkaller.appspot.com/bug?extid=13d3cb2a3dc61e6092f5
<6> 621 Yes KASAN: slab-out-of-bounds Read in mcp2221_raw_event (2)
https://syzkaller.appspot.com/bug?extid=1018672fe70298606e5f
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders
To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem
You may send multiple commands in a single email message.
^ permalink raw reply
* [PATCH 2/5] HID: multitouch: Fix stale MT slots when contact count drops to zero
From: Dave Carey @ 2026-05-22 11:15 UTC (permalink / raw)
To: linux-input; +Cc: jikos, bentiss, Dave Carey
In-Reply-To: <20260515175253.873796-1-carvsdriver@gmail.com>
The INGENIC 17EF:6161 touchscreen (Lenovo Yoga Book 9 14IAH10) reports
HID_DG_CONTACTCOUNT=0 in the frame immediately following the last finger
lift rather than omitting the frame entirely. In mt_touch_report() the
existing code only updates num_expected when contact_count is non-zero,
so a zero contact count on the first packet of a new frame leaves
num_expected at its previous value (e.g. 2 for a two-finger gesture).
The sync check "num_received >= num_expected" then evaluates "0 >= 2"
and never fires, preventing INPUT_MT_DROP_UNUSED from releasing the
stale slots. Those slots remain active in the kernel MT layer until the
next touch, at which point they are released in a batch alongside the
new contact — causing the userspace event consumer to miss the intervening
finger-up sequence and corrupt its gesture session state.
Fix by resetting num_expected to 0 when contact_count is zero and
num_received is still 0 (i.e., this is the first and only packet of the
frame, not a continuation packet in a multi-packet sequence). With
num_expected=0 the sync check "0 >= 0" fires immediately, calling
input_mt_sync_frame() which drops the stale slots via INPUT_MT_DROP_UNUSED.
The num_received==0 guard is critical: continuation packets in a
multi-packet frame arrive after at least one contact has already been
processed (num_received>0), so they are correctly excluded from this
path and the existing multi-packet logic is unaffected.
Signed-off-by: Dave Carey <carvsdriver@gmail.com>
Tested-by: Dave Carey <carvsdriver@gmail.com>
---
v2:
- Restructured contact_count block per Benjamin Tissoires' review:
replace three-branch if/else-if/else-if with a cleaner two-branch
form, dropping the outer if (contact_count >= 0) wrapper
- Add prev_scantime != scantime guard to the zero-contact sentinel case
drivers/hid/hid-multitouch.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index ec04dbafb..f1a2b3c4d5 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1321,21 +1321,18 @@ static void mt_touch_report(struct hid_device *hid,
* Includes multi-packet support where subsequent
* packets are sent with zero contactcount.
*/
- if (contact_count >= 0) {
+ if (contact_count > 0)
+ app->num_expected = contact_count;
+ else if (app->num_received == 0 && app->prev_scantime != scantime) {
/*
+ * New multi-report frame:
+ *
* For Win8 PTPs the first packet (td->num_received == 0) may
* have a contactcount of 0 if there only is a button event.
- * We double check that this is not a continuation packet
- * of a possible multi-packet frame be checking that the
- * timestamp has changed.
+ *
+ * Some other devices use a sentinel frame with 0 to release all contacts
*/
- if ((app->quirks & MT_QUIRK_WIN8_PTP_BUTTONS) &&
- app->num_received == 0 &&
- app->prev_scantime != scantime)
- app->num_expected = contact_count;
- /* A non 0 contact count always indicates a first packet */
- else if (contact_count)
- app->num_expected = contact_count;
+ app->num_expected = 0;
}
app->prev_scantime = scantime;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] Input: ims-pcu - fix usb_free_coherent() size in ims_pcu_buffers_alloc()
From: sashiko-bot @ 2026-05-22 10:55 UTC (permalink / raw)
To: Thomas Fourier; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260522085412.45430-2-fourier.thomas@gmail.com>
Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider:
New issues:
- [Low] Commit message inaccuracy regarding the fixed function.
Pre-existing issues:
- [High] Failure to release explicitly claimed secondary data interface on disconnect leaves the interface permanently claimed.
- [High] Memory leak of firmware object in `ims_pcu_process_async_firmware()`.
- [High] Missing interface verification in `ims_pcu_disconnect()` leads to Use-After-Free and Double-Free vulnerabilities.
- [High] Race condition in sysfs `reset_device` store callback due to missing mutex.
- [High] Type confusion vulnerability in CDC union descriptor parsing.
- [Medium] Missing endpoint type validation for the control interface.
- [Medium] Logic error in `ims_pcu_reset_packet()` breaks the parser state machine.
- [Medium] Out-of-bounds read and information leak due to missing response length checks.
- [Medium] Violation of DMA mapping rules by performing DMA on an embedded structure.
- [Low] Out-of-bounds read in `ims_pcu_irq()` debug logging.
--
commit b02b6acbca140f2c98e04a2bf7bf5d8b5b24ae41
Author: Thomas Fourier <fourier.thomas@gmail.com>
Input: ims-pcu - fix usb_free_coherent() size in ims_pcu_buffers_alloc()
This commit changes the size passed to usb_free_coherent() in
ims_pcu_buffers_free() from max_out_size to max_in_size, properly matching
the input buffer's allocation size.
> Input: ims-pcu - fix usb_free_coherent() size in ims_pcu_buffers_alloc()
>
> The input buffer size is pcu->max_in_size, but pcu->max_out_size is
> passed to usb_free_coherent().
>
> Change size to match the allocation size.
[Severity: Low]
Is the function name in the subject line accurate? This modifies
ims_pcu_buffers_free(), but the subject mentions ims_pcu_buffers_alloc().
The error handling path in ims_pcu_buffers_alloc() appears to already
correctly use pcu->max_in_size.
>
> Fixes: 628329d52474 ("Input: add IMS Passenger Control Unit driver")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
> diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
> index 4c022a36dbe84..7a1cb9333f53c 100644
> --- a/drivers/input/misc/ims-pcu.c
> +++ b/drivers/input/misc/ims-pcu.c
> @@ -1624,7 +1624,7 @@ static void ims_pcu_buffers_free(struct ims_pcu *pcu)
> usb_kill_urb(pcu->urb_in);
> usb_free_urb(pcu->urb_in);
>
> - usb_free_coherent(pcu->udev, pcu->max_out_size,
> + usb_free_coherent(pcu->udev, pcu->max_in_size,
> pcu->urb_in_buf, pcu->read_dma);
>
> kfree(pcu->urb_out_buf);
[Severity: High]
This is a pre-existing issue, but does ims_pcu_disconnect() leak the claimed
secondary data interface?
drivers/input/misc/ims-pcu.c:ims_pcu_disconnect() {
[ ... ]
if (alt->desc.bInterfaceClass != USB_CLASS_COMM)
return;
ims_pcu_stop_io(pcu);
[ ... ]
ims_pcu_buffers_free(pcu);
kfree(pcu);
}
During ims_pcu_probe(), the driver claims the secondary data interface using
usb_driver_claim_interface(), but ims_pcu_disconnect() never calls
usb_driver_release_interface() for pcu->data_intf when the primary control
interface is unbound. Does this leave the data interface permanently claimed,
causing subsequent binds to fail with -EBUSY?
[Severity: High]
This is a pre-existing issue, but can this logic in ims_pcu_disconnect() lead
to a use-after-free or double-free?
drivers/input/misc/ims-pcu.c:ims_pcu_disconnect() {
[ ... ]
/*
* See if we are dealing with control or data interface. The cleanup
* happens when we unbind primary (control) interface.
*/
if (alt->desc.bInterfaceClass != USB_CLASS_COMM)
return;
[ ... ]
kfree(pcu);
}
If a malicious USB device creates a secondary data interface with the
USB_CLASS_COMM class, and the user unbinds this malicious data interface via
sysfs, does ims_pcu_disconnect() bypass the return statement and prematurely
free the pcu structure while the primary interface is still active?
Should it verify if (intf != pcu->ctrl_intf) instead?
[Severity: High]
This is a pre-existing issue, but does this code cause a type confusion
regression during probe?
In ims_pcu_parse_cdc_data(), the driver looks up the control interface using
union_desc->bMasterInterface0 from the untrusted CDC union descriptor, and
assigns it to pcu->ctrl_intf without validating that pcu->ctrl_intf matches
the interface currently being probed.
drivers/input/misc/ims-pcu.c:ims_pcu_probe() {
[ ... ]
error = ims_pcu_parse_cdc_data(intf, pcu);
if (error)
goto err_free_mem;
[ ... ]
usb_set_intfdata(pcu->ctrl_intf, pcu);
[ ... ]
If a malicious device points bMasterInterface0 to an interface owned by a
completely different driver, does usb_set_intfdata() overwrite the other
driver's private data pointer, potentially leading to memory corruption?
[Severity: High]
This is a pre-existing issue, but does this error path leak the firmware
object?
If ihex_validate_fw(fw) returns an error in ims_pcu_process_async_firmware(),
the function jumps to the out label, which bypasses the release_firmware(fw)
call. Since fw is a valid pointer when ihex_validate_fw() is called, does this
directly leak the firmware object memory?
[Severity: High]
This is a pre-existing issue, but is there a race condition in the sysfs
reset_device store callback?
The ims_pcu_reset_device() sysfs callback directly calls
ims_pcu_execute_command() without acquiring pcu->cmd_mutex. This function
manipulates shared variables like pcu->expected_response, pcu->cmd_done, and
pcu->cmd_buf. If triggered concurrently with other commands (such as LED
brightness updates which properly lock the mutex), does this cause data races
and corruption of the shared command buffer?
[Severity: Medium]
This is a pre-existing issue, but does the driver validate the endpoint type
for the control interface?
In ims_pcu_parse_cdc_data(), the driver extracts the first endpoint:
pcu->ep_ctrl = &alt->endpoint[0].desc
Does it fail to verify that it is an IN interrupt endpoint? If a malicious
device provides a different endpoint type (e.g., bulk OUT), which is then
passed to usb_fill_int_urb() and usb_submit_urb(), does this trigger a
BOGUS urb xfer kernel warning?
[Severity: Medium]
This is a pre-existing issue, but is there a logic error in
ims_pcu_reset_packet() that breaks the parser state machine?
ims_pcu_reset_packet() is intended to reset the protocol state machine to wait
for the next packet, but it sets pcu->have_stx = true.
Does this cause the state machine to bypass the wait for the next STX byte
after the first packet is processed, and immediately accept garbage bytes? When
the actual STX byte later arrives, does the driver spam the kernel log with
Unexpected STX at byte X, discarding old data warnings?
[Severity: Medium]
This is a pre-existing issue, but does this code lead to out-of-bounds reads
and information leaks?
Functions like ims_pcu_report_events() and ims_pcu_get_info() process data
directly from the response buffers (e.g., pcu->read_buf[3] or pcu->cmd_buf)
without checking pcu->read_pos or pcu->cmd_buf_len to ensure the device
actually sent enough data.
If a device sends a shortened response, does the driver parse stale,
uninitialized data remaining in the buffer from previous commands?
[Severity: Medium]
This is a pre-existing issue, but does this code violate DMA mapping rules?
In ims_pcu_line_setup(), pcu->cmd_buf is passed to usb_control_msg() for a DMA
transfer:
drivers/input/misc/ims-pcu.c:ims_pcu_line_setup() {
[ ... ]
struct usb_cdc_line_coding *line = (void *)pcu->cmd_buf;
[ ... ]
error = usb_control_msg(pcu->udev, usb_sndctrlpipe(pcu->udev, 0),
USB_CDC_REQ_SET_LINE_CODING,
USB_TYPE_CLASS | USB_RECIP_INTERFACE,
0, interface->desc.bInterfaceNumber,
line, sizeof(struct usb_cdc_line_coding),
5000);
[ ... ]
Since pcu->cmd_buf is an array embedded directly inside the struct ims_pcu
allocation, and embedded fields are not guaranteed to be strictly cacheline
aligned, can cache maintenance operations on this buffer corrupt adjacent
fields in the struct ims_pcu on architectures without hardware cache coherency?
[Severity: Low]
This is a pre-existing issue, but is there an out-of-bounds read in the
ims_pcu_irq() debug logging?
In ims_pcu_irq(), the debug logging unconditionally prints data from
pcu->urb_in_buf up to urb->actual_length bytes. If the interrupt fired for
pcu->urb_ctrl, the actual transferred data resides in pcu->urb_ctrl_buf.
Does this result in printing unrelated stale data, and if urb->actual_length
from the control URB exceeds pcu->max_in_size, does it read out of bounds up
to the format limit?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522085412.45430-2-fourier.thomas@gmail.com?part=1
^ permalink raw reply
* [PATCH] Input: ims-pcu - fix usb_free_coherent() size in ims_pcu_buffers_alloc()
From: Thomas Fourier @ 2026-05-22 8:54 UTC (permalink / raw)
Cc: Thomas Fourier, stable, Dmitry Torokhov, Dan Carpenter,
Seungjin Bae, Sanghoon Choi, Kees Cook, linux-input, linux-kernel
The input buffer size is pcu->max_in_size, but pcu->max_out_size is
passed to usb_free_coherent().
Change size to match the allocation size.
Fixes: 628329d52474 ("Input: add IMS Passenger Control Unit driver")
Cc: <stable@vger.kernel.org>
Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
---
drivers/input/misc/ims-pcu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
index 4c022a36dbe8..7a1cb9333f53 100644
--- a/drivers/input/misc/ims-pcu.c
+++ b/drivers/input/misc/ims-pcu.c
@@ -1624,7 +1624,7 @@ static void ims_pcu_buffers_free(struct ims_pcu *pcu)
usb_kill_urb(pcu->urb_in);
usb_free_urb(pcu->urb_in);
- usb_free_coherent(pcu->udev, pcu->max_out_size,
+ usb_free_coherent(pcu->udev, pcu->max_in_size,
pcu->urb_in_buf, pcu->read_dma);
kfree(pcu->urb_out_buf);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] HID: core: Fix bsize printk specifier in hid_report_raw_event()
From: Benjamin Tissoires @ 2026-05-22 8:46 UTC (permalink / raw)
To: yaolu; +Cc: jikos, linux-input, linux-kernel, k2ci
In-Reply-To: <20260522072718.186707-1-yaolu@kylinos.cn>
On May 22 2026, yaolu@kylinos.cn wrote:
> From: Lu Yao <yaolu@kylinos.cn>
>
> Fix the following warning during build time:
>
> drivers/hid/hid-core.c:2053:43: error: format ‘%ld’ expects argument of type ‘long int’, but argument 5 has type ‘size_t’ {aka ‘unsigned int’} [-Werror=format=]
> drivers/hid/hid-core.c:2075:43: error: format ‘%ld’ expects argument of type ‘long int’, but argument 5 has type ‘size_t’ {aka ‘unsigned int’} [-Werror=format=]
>
> Fixes: 2c85c61d1332 ("HID: pass the buffer size to hid_report_raw_event")
> Reported-by: k2ci <kernel-bot@kylinos.cn>
> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
In addition to shashiko comments about %zu, it's already in linus' tree:
4d3a2a466b8d ("HID: core: Fix size_t specifier in hid_report_raw_event()")
Cheers,
Benjamin
> ---
> drivers/hid/hid-core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index b3596851c719..c21c58d7bf02 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2050,7 +2050,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
> return 0;
>
> if (unlikely(bsize < csize)) {
> - hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
> + hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %u)\n",
> report->id, csize, bsize);
> return -EINVAL;
> }
> @@ -2072,7 +2072,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
> rsize = max_buffer_size;
>
> if (bsize < rsize) {
> - hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
> + hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %u)\n",
> report->id, rsize, bsize);
> return -EINVAL;
> }
> --
> 2.25.1
>
>
^ permalink raw reply
* Re: [PATCH 02/10] [v3] input: gpio-keys: make legacy gpiolib optional
From: Arnd Bergmann @ 2026-05-22 8:28 UTC (permalink / raw)
To: Matti Vaittinen, Arnd Bergmann, open list:GPIO SUBSYSTEM
Cc: linux-kernel, Christian Lamparter, Johannes Berg, Aaro Koskinen,
Andreas Kemnade, Kevin Hilman, Roger Quadros, Tony Lindgren,
Thomas Bogendoerfer, John Paul Adrian Glaubitz, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Linus Walleij, Bartosz Golaszewski, Dmitry Torokhov, Lee Jones,
Pavel Machek, Florian Fainelli, Jonas Gorski, Andrew Lunn,
Vladimir Oltean, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, linux-wireless, Linux-OMAP, linux-arm-kernel,
linux-mips, linux-sh, linux-input, linux-leds, Netdev
In-Reply-To: <b79d8ad9-f36e-4769-9dc2-58baefe23000@gmail.com>
On Fri, May 22, 2026, at 06:55, Matti Vaittinen wrote:
> On 20/05/2026 21:38, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> The two Rohm PMIC drivers use a gpio-keys device without an actual GPIO,
>> passing an IRQ number instead. In order to keep this working both with
>> and with CONFIG_GPIOLIB_LEGACY, change the gpio-keys driver to ignore
>> the gpio number if an IRQ is passed.
>>
>
> I am (still) all fine with this, even though I like Dmitry's set. I
> suppose you already have a plan for merging this, but I still have to
> ask - why the MFD changes aren't in own patch? I feel it would have
> simplified merging, backporting, reviewing and reverting if needed.
Splitting it out would break bisection: The gpio-leds change without
the mfd change causes a build failure when assigning the .gpio
field, and the reverse causes a runtime failure when .gpio=0 is
a valid line.
It would be possible to only do the gpio-led driver patch without
the header file change first. This would let us apply the last
patch in the series without regression, but it would risk having
not catching other drivers that incorrectly set the .gpio field
while CONFIG_GPIOLIB_LEGACY is disabled.
Arnd
^ permalink raw reply
* Re: [syzbot] [input?] possible deadlock in tasklet_action_common (2)
From: Sebastian Andrzej Siewior @ 2026-05-22 7:35 UTC (permalink / raw)
To: Hillf Danton
Cc: syzbot, dmitry.torokhov, linux-input, Tetsuo Handa, linux-kernel,
syzkaller-bugs
In-Reply-To: <20260522072144.934-1-hdanton@sina.com>
On 2026-05-22 15:21:43 [+0800], Hillf Danton wrote:
> > > On RT spinlock is replaced with mutex, and softirq can be raised in the
> > > irq that could come any moment after spin_lock_irqsave().
> >
> > That is true on the other hand. That means having raised another tasklet
> > could lead to the backtrace. But it would have been two different locks,
> > not blocking on each other.
> >
> The last question, by two different locks, do you mean that the
> tasklet_sync_callback.cb_lock is per cpu?
Yes, it is but this does not matter. kbd_led_trigger_activate() does
tasklet_disable(&keyboard_tasklet) so you can't have kbd_bh() running
and led_set_brightness() which would kick the softirq again. Not from
kbd_led_trigger_activate().
Even if another component would raise a softirq in that window, you
could run tasklets, yes, but never kbd_bh(). Not in this window.
So the kbd_led_trigger_activate() -> input_inject_event() needs to be a
different event device than kbd_propagate_led_state() -> event. I don't
see how you could unfold this.
I *think* lockdep observed all this possible interactions and sketched
this possibility but it does not know about tasklet_disable().
I do have an idea how to avoid alloc_skb() invoking softirqs if someone
else raised them while alloc_skb() was preempted. This could avoid this
kind of splat.
Sebastian
^ permalink raw reply
* Re: [PATCH] HID: core: Fix bsize printk specifier in hid_report_raw_event()
From: sashiko-bot @ 2026-05-22 7:31 UTC (permalink / raw)
To: yaolu; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260522072718.186707-1-yaolu@kylinos.cn>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Incorrect format specifier for the `size_t` variable `bsize`. The patch replaces `%ld` with `%u` to fix a warning on 32-bit architectures (where `size_t` is `unsigned int`), but this introduces a new warning on 64-bit architectures (where `size_t` is `unsigned long`). The correct format specifier for `size_t` is `%zu`.
--
commit d6a5fb9c76c9718771a50e1f79f29b3df496f869
Author: Lu Yao <yaolu@kylinos.cn>
HID: core: Fix bsize printk specifier in hid_report_raw_event()
This commit attempts to fix a build warning on 32-bit architectures by
changing the format specifier for a size_t variable from %ld to %u.
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index b3596851c7191..c21c58d7bf02b 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2050,7 +2050,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
> return 0;
>
> if (unlikely(bsize < csize)) {
> - hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
> + hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %u)\n",
[Severity: Low]
Will this cause a new format string warning on 64-bit architectures? bsize is
of type size_t, which is an unsigned long on 64-bit systems. Should this use
the %zu format specifier instead to be portable across both 32-bit and 64-bit
architectures?
> report->id, csize, bsize);
> return -EINVAL;
> }
[ ... ]
> @@ -2072,7 +2072,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
> rsize = max_buffer_size;
>
> if (bsize < rsize) {
> - hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
> + hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %u)\n",
[Severity: Low]
Similarly here, should this also use %zu for the size_t variable bsize?
> report->id, rsize, bsize);
> return -EINVAL;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522072718.186707-1-yaolu@kylinos.cn?part=1
^ permalink raw reply
* [PATCH] HID: core: Fix bsize printk specifier in hid_report_raw_event()
From: yaolu @ 2026-05-22 7:27 UTC (permalink / raw)
To: jikos, bentiss; +Cc: linux-input, linux-kernel, Lu Yao, k2ci
From: Lu Yao <yaolu@kylinos.cn>
Fix the following warning during build time:
drivers/hid/hid-core.c:2053:43: error: format ‘%ld’ expects argument of type ‘long int’, but argument 5 has type ‘size_t’ {aka ‘unsigned int’} [-Werror=format=]
drivers/hid/hid-core.c:2075:43: error: format ‘%ld’ expects argument of type ‘long int’, but argument 5 has type ‘size_t’ {aka ‘unsigned int’} [-Werror=format=]
Fixes: 2c85c61d1332 ("HID: pass the buffer size to hid_report_raw_event")
Reported-by: k2ci <kernel-bot@kylinos.cn>
Signed-off-by: Lu Yao <yaolu@kylinos.cn>
---
drivers/hid/hid-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index b3596851c719..c21c58d7bf02 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2050,7 +2050,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
return 0;
if (unlikely(bsize < csize)) {
- hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",
+ hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %u)\n",
report->id, csize, bsize);
return -EINVAL;
}
@@ -2072,7 +2072,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
rsize = max_buffer_size;
if (bsize < rsize) {
- hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",
+ hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %u)\n",
report->id, rsize, bsize);
return -EINVAL;
}
--
2.25.1
^ permalink raw reply related
* Re: [syzbot] [input?] possible deadlock in tasklet_action_common (2)
From: Hillf Danton @ 2026-05-22 7:21 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: syzbot, dmitry.torokhov, linux-input, Tetsuo Handa, linux-kernel,
syzkaller-bugs
In-Reply-To: <20260522063938.ewKum8vW@linutronix.de>
On Fri, 22 May 2026 08:39:38 +0200 Sebastian Andrzej Siewior wrote:
> On 2026-05-22 06:35:14 [+0800], Hillf Danton wrote:
> > input_inject_event() is invoked in the brightness_set callback [1] for
> > example.
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/input/input-leds.c#n142
> >
> > > Now, input_inject_event() does spin_lock_irqsave() and alloc_skb() does
> > > local_bh_disable()/ local_bh_enable(). On !RT this is not legal. On RT
> >
> > Can you please specify why that is illegal on !RT?
>
> So if you do
> spin_lock_irq();
> local_bh_disable();
>
> then the
> local_bh_enable();
>
> has lockdep_assert_irqs_enabled() which will yell. Then there is also
Ah got it, thanks.
> this local_irq_enable() which will enable interrupts before the unlock
> which bad, again. Also, should softirqs been raised within this section,
> the this enable will invoke the required callback which again, enable
> interrupts.
>
> > > it is okay but then local_bh_enable() here should not invoke any
> > > softirqs because none were raised within the section (alloc_skb()).
> >
> > On RT spinlock is replaced with mutex, and softirq can be raised in the
> > irq that could come any moment after spin_lock_irqsave().
>
> That is true on the other hand. That means having raised another tasklet
> could lead to the backtrace. But it would have been two different locks,
> not blocking on each other.
>
The last question, by two different locks, do you mean that the
tasklet_sync_callback.cb_lock is per cpu?
^ permalink raw reply
* Re: [syzbot] [input?] possible deadlock in tasklet_action_common (2)
From: Sebastian Andrzej Siewior @ 2026-05-22 6:39 UTC (permalink / raw)
To: Hillf Danton
Cc: syzbot, dmitry.torokhov, linux-input, Tetsuo Handa, linux-kernel,
syzkaller-bugs
In-Reply-To: <20260521223516.914-1-hdanton@sina.com>
On 2026-05-22 06:35:14 [+0800], Hillf Danton wrote:
> input_inject_event() is invoked in the brightness_set callback [1] for
> example.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/input/input-leds.c#n142
>
> > Now, input_inject_event() does spin_lock_irqsave() and alloc_skb() does
> > local_bh_disable()/ local_bh_enable(). On !RT this is not legal. On RT
>
> Can you please specify why that is illegal on !RT?
So if you do
spin_lock_irq();
local_bh_disable();
then the
local_bh_enable();
has lockdep_assert_irqs_enabled() which will yell. Then there is also
this local_irq_enable() which will enable interrupts before the unlock
which bad, again. Also, should softirqs been raised within this section,
the this enable will invoke the required callback which again, enable
interrupts.
> > it is okay but then local_bh_enable() here should not invoke any
> > softirqs because none were raised within the section (alloc_skb()).
>
> On RT spinlock is replaced with mutex, and softirq can be raised in the
> irq that could come any moment after spin_lock_irqsave().
That is true on the other hand. That means having raised another tasklet
could lead to the backtrace. But it would have been two different locks,
not blocking on each other.
Sebastian
^ permalink raw reply
* Recall: [PATCH] HID: remove duplicate hid_warn_ratelimited definition
From: Liu Kai @ 2026-05-22 6:11 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Liu Kai would like to recall the message, "[PATCH] HID: remove duplicate hid_warn_ratelimited definition".
^ permalink raw reply
* Re: [PATCH 2/2] HID: lenovo: Use KEY_PERFORMANCE capability for ThinkPad X12 Tab Gen 2
From: sashiko-bot @ 2026-05-22 6:05 UTC (permalink / raw)
Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260522050632.8079-2-vishnuocv@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Low] The patch unconditionally adds the KEY_PERFORMANCE capability to all input nodes exposed by the device, causing capability pollution on unrelated interfaces.
Pre-existing issues:
- [High] The raw event handler emits keys like KEY_RFKILL, KEY_BOOKMARKS, and KEY_KBDILLUMTOGGLE, but their capabilities are never declared, causing the input subsystem to silently drop these events.
- [High] The raw event handler contains a loop that unconditionally fires key events on the first input node it encounters, ignoring capability routing and causing events to be dropped.
--
commit 1b40b44e417a26a21650fec78e6a2ea55f535004
Author: Vishnu Sankar <vishnuocv@gmail.com>
HID: lenovo: Use KEY_PERFORMANCE capability for ThinkPad X12 Tab Gen 2
This commit adds the KEY_PERFORMANCE capability for the ThinkPad X12 Tab Gen 2
and X13 Tab devices during input configuration. This allows userspace tools to
discover the key emitted via the raw event handler.
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index 35b00619f0456..3492ba8d0fb02 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -1525,6 +1525,7 @@ static int lenovo_input_configured(struct hid_device *hdev,
> hi->input->propbit);
> }
> break;
> + case USB_DEVICE_ID_LENOVO_X12_TAB2:
> case USB_DEVICE_ID_LENOVO_X13_TAB:
> input_set_capability(hi->input, EV_KEY, KEY_PERFORMANCE);
[Severity: Low]
Does this unconditionally add the KEY_PERFORMANCE capability to all input
nodes exposed by the device?
Since lenovo_input_configured() is invoked for every hid_input interface,
this regression might advertise the capability on unrelated interfaces.
Could we filter this by checking the application usage or verifying it is
the primary keyboard node?
[Severity: High]
This isn't a bug introduced by this patch, but I noticed the raw event
handler also emits keys like KEY_RFKILL, KEY_PICKUP_PHONE, KEY_BOOKMARKS,
and KEY_KBDILLUMTOGGLE.
Should we also declare the capabilities for these keys here?
Since they are generated from raw reports rather than standard HID
descriptors, they appear to be missing from the capability bits, which would
cause the input subsystem to silently drop these events.
[Severity: High]
This is a pre-existing issue, but looking at how the raw events are handled
in lenovo_raw_event_TP_X12_tab(), is it possible the events are firing on
the wrong input node?
The handler loops over hdev->inputs but returns immediately after reporting
the key event on the first node it encounters. If the first device lacks the
specific key capabilities, the input core might discard the event.
Should the loop instead locate the specific input node that possesses the
corresponding key capability before dispatching?
> break;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522050632.8079-1-vishnuocv@gmail.com?part=2
^ 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