* [PATCH v5 0/6] gpiolib: fence off legacy interfaces
From: Arnd Bergmann @ 2026-06-29 13:03 UTC (permalink / raw)
To: linux-gpio
Cc: Arnd Bergmann, 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, linux-sh, linux-kernel, linux-input, linux-leds
From: Arnd Bergmann <arnd@arndb.de>
This is the remainder of the series previously posted as v2
in [1]. I've changed the version to v5 for all patches to
not confuse b4 too much, but the patches are mostly unchanged.
The patch "Input: soc_button_array - select CONFIG_GPIOLIB_LEGACY"
was not part of the series last time, but the build bots reported
this as a regression since I had dropped that since v1.
I hope that all that remains now can just get merged through the
gpio tree. The gpio-keys patch needs a bit coordination with
another patch addressing the same issue that is already in
flight, so I expect that I'll rebase my series once more when
that is in a stable branch, but the state I have here should
just work as-is on top of v7.2-rc1.
Arnd
[1] https://lore.kernel.org/all/20260520183815.2510387-1-arnd@kernel.org/
Arnd Bergmann (6):
[v5] sh: select legacy gpiolib interface
[v5] x86/olpc: select GPIOLIB_LEGACY
[v5] Input: soc_button_array - select CONFIG_GPIOLIB_LEGACY
[v5] Input: gpio-keys: make legacy gpiolib optional
[v5] leds: gpio: make legacy gpiolib interface optional
[v5] gpiolib: turn off legacy interface by default
arch/sh/Kconfig | 1 +
arch/sh/boards/Kconfig | 8 ++++
arch/sh/boards/mach-highlander/Kconfig | 1 +
arch/sh/boards/mach-rsk/Kconfig | 3 ++
arch/x86/Kconfig | 1 +
arch/x86/platform/olpc/olpc-xo1-sci.c | 2 +-
drivers/gpio/Kconfig | 9 +++-
drivers/input/keyboard/gpio_keys.c | 9 ++--
drivers/input/keyboard/gpio_keys_polled.c | 4 +-
drivers/input/misc/Kconfig | 3 ++
drivers/input/misc/soc_button_array.c | 2 +-
drivers/leds/leds-gpio.c | 53 +++++++++++++++--------
drivers/mfd/rohm-bd71828.c | 1 -
drivers/mfd/rohm-bd718x7.c | 1 -
include/linux/gpio_keys.h | 2 +
include/linux/leds.h | 2 +
sound/pci/Kconfig | 1 +
sound/pci/cs5535audio/cs5535audio_olpc.c | 2 +-
18 files changed, 76 insertions(+), 29 deletions(-)
--
2.39.5
Cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Linus Walleij <linusw@kernel.org>
Cc: Bartosz Golaszewski <brgl@kernel.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Lee Jones <lee@kernel.org>
Cc: Pavel Machek <pavel@kernel.org>
Cc: linux-sh@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-gpio@vger.kernel.org
Cc: linux-input@vger.kernel.org
Cc: linux-leds@vger.kernel.org
^ permalink raw reply
* Re: [PATCH v4 00/10] HID: steelseries: Refactor Arctis driver and add Arctis Nova 7 Gen2 support
From: Jiri Kosina @ 2026-06-29 9:47 UTC (permalink / raw)
To: Sriman Achanta
Cc: Benjamin Tissoires, linux-input, linux-kernel, Simon Wood,
Christian Mayer, Bastien Nocera
In-Reply-To: <20260623172310.272708-1-srimanachanta@gmail.com>
Sriman,
thanks a lot for the effort. The patchset looks good to me, but Sashiko
found a couple issues.
I specifically believe the ones for 4/10 are valid. Have you looked into
those?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: hid-sensor-custom: Fix sysfs group leak on failure
From: Jiri Kosina @ 2026-06-29 9:28 UTC (permalink / raw)
To: Haoxiang Li
Cc: jic23, srinivas.pandruvada, bentiss, linux-input, linux-iio,
linux-kernel, stable
In-Reply-To: <20260623021950.1736413-1-haoxiang_li2024@163.com>
On Tue, 23 Jun 2026, Haoxiang Li wrote:
> hid_sensor_custom_add_attributes() creates one sysfs group for each
> custom sensor field. If sysfs_create_group() fails after some groups
> have already been created, the function currently breaks out of the
> loop and returns the error directly.
>
> Fix this by adding a local unwind path when sysfs_create_group() fails.
> The unwind path removes all sysfs groups that were successfully created
> before the failure and frees sensor_inst->fields.
>
> Fixes: 4a7de0519df5 ("HID: sensor: Custom and Generic sensor support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
> ---
> drivers/hid/hid-sensor-custom.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
> index afffea894021..cd676516e6b0 100644
> --- a/drivers/hid/hid-sensor-custom.c
> +++ b/drivers/hid/hid-sensor-custom.c
> @@ -609,7 +609,7 @@ static int hid_sensor_custom_add_attributes(struct hid_sensor_custom
> &sensor_inst->fields[i].
> hid_custom_attribute_group);
> if (ret)
> - break;
> + goto err_remove_groups;
>
> /* For power or report field store indexes */
> if (sensor_inst->fields[i].attribute.attrib_id ==
> @@ -621,6 +621,13 @@ static int hid_sensor_custom_add_attributes(struct hid_sensor_custom
> }
>
> return ret;
> +
> +err_remove_groups:
> + while (--i >= 0)
> + sysfs_remove_group(&sensor_inst->pdev->dev.kobj,
> + &sensor_inst->fields[i].hid_custom_attribute_group);
> + kfree(sensor_inst->fields);
I believe Sashiko is right here abou the UAF. Could you please fix that
and resubmit?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v5 0/5] HID: asus: security fixes and more hardware support
From: Jiri Kosina @ 2026-06-29 9:21 UTC (permalink / raw)
To: Denis Benato
Cc: linux-kernel, linux-input, Benjamin Tissoires, Luke D . Jones,
Mateusz Schyboll, Denis Benato, Antheas Kapenekakis, Connor Belli
In-Reply-To: <20260619001103.1189200-1-denis.benato@linux.dev>
On Fri, 19 Jun 2026, Denis Benato wrote:
> Hi all,
>
> I have added support for controlling the (way too bright) XG mobile
> LEDs in hid-asus and added the i2c version of already supported
> hardware that was probed only when it's a USB: these are two separate
> features changes and are the only two that are not fixes for
> pre-existing issue (see below).
>
> Auto-review bot has spotted a bunch of pre-existing problems alongside
> problems in my own code therefore at this point I am going to fix the
> more problems I can and including those fixes and improvements in this
> patchset.
>
> For v4 I decided to follow Antheas' suggestion of reusing the existing
> workqueue and by making it more generic I solved a good bunch of issues.
>
> The v5 iteration is simply me fixing a bunch of bugs in new code spotted
> by the bot. Thanks for providing such a useful tool!
>
> On a side node this patchset has a few more warnings: specifically
> "WARNING: Prefer kzalloc_obj over kzalloc with sizeof" but it's a false
> positive as that would introduce sleeping calls in atomic contexts.
Denis,
thanks. Could you please flag which patches you'd prefer to go in still
for 7.1 and which ones are not critical and could wait for 7.2? The whole
lot is quite big.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: lg-g15: cancel pending work on remove to fix a use-after-free
From: Jiri Kosina @ 2026-06-29 9:18 UTC (permalink / raw)
To: Maoyi Xie; +Cc: Hans de Goede, Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <178176639579.3377656.12792307991044339915@maoyixie.com>
On Thu, 18 Jun 2026, Maoyi Xie wrote:
> lg_g15_data is allocated with devm and holds a work item. The report
> handlers schedule that work straight from device input.
> lg_g15_event() and lg_g15_v2_event() do it on the backlight cycle key,
> and lg_g510_leds_event() does it too. The worker dereferences the
> lg_g15_data back through container_of.
>
> The driver had no remove callback and never cancelled the work. So if a
> report scheduled the work and the keyboard was then unplugged, devres
> freed lg_g15_data while the work was still pending or running, and the
> worker touched freed memory. This is a use-after-free. It is reachable
> as a race on device unplug.
>
> Add a remove callback that cancels the work before devres frees the
> state. g15->work is only initialized for the models that schedule it
> (G15, G15 v2, G510). The G13 and Z-10 leave it zeroed, so guard the
> cancel on g15->work.func to avoid cancelling a work that was never set
> up. The g15 NULL test mirrors the one already in lg_g15_raw_event().
>
> Fixes: 97b741aba918 ("HID: lg-g15: Add keyboard and LCD backlight control")
> Cc: stable@vger.kernel.org
> Suggested-by: Hans de Goede <hansg@kernel.org>
> Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
Thanks, applied.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: logitech-dj: Fix maxfield check in DJ short report validation
From: Jiri Kosina @ 2026-06-29 9:18 UTC (permalink / raw)
To: HyeongJun An
Cc: Benjamin Tissoires, Filipe Laíns, Lee Jones, linux-input,
linux-kernel, stable
In-Reply-To: <20260618063737.211468-1-sammiee5311@gmail.com>
On Thu, 18 Jun 2026, HyeongJun An wrote:
> Commit b6a57912854e ("HID: logitech-dj: Prevent REPORT_ID_DJ_SHORT
> related user initiated OOB write") added validation for the DJ short
> output report, but the error path dereferences rep->field[0] even when
> rep->maxfield is zero.
>
> Commit 8b9a097eb2fc ("HID: logitech-dj: fix wrong detection of bad
> DJ_SHORT output report") made the check conditional on rep being present,
> but a crafted descriptor can still create report ID 0x20 with only padding
> output items. hid-core registers the report, ignores the padding field,
> and leaves rep->maxfield as zero.
>
> In that case the validation enters the rep->maxfield < 1 branch and then
> dereferences rep->field[0]->report_count while printing the error message,
> causing a NULL pointer dereference during probe. This is reproducible with
> uhid by emulating a Logitech receiver with a padding-only DJ short output
> report:
>
> BUG: KASAN: null-ptr-deref in logi_dj_probe+0xb1/0x754 [hid_logitech_dj]
> Read of size 4 at addr 0000000000000028 by task kworker/4:1/129
> ...
> Call Trace:
> logi_dj_probe+0xb1/0x754 [hid_logitech_dj]
> hid_device_probe+0x329/0x3f0 [hid]
> really_probe+0x162/0x570
> __device_attach+0x137/0x2c0
> bus_probe_device+0x38/0xc0
> device_add+0xa56/0xce0
> hid_add_device+0x19c/0x280 [hid]
> uhid_device_add_worker+0x2c/0xb0 [uhid]
>
> Reject the zero-field report before printing the field report_count.
>
> Fixes: b6a57912854e ("HID: logitech-dj: Prevent REPORT_ID_DJ_SHORT related user initiated OOB write")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 0/2] HID: roccat: bound device-supplied profile index
From: Jiri Kosina @ 2026-06-29 9:16 UTC (permalink / raw)
To: Michael Bommarito
Cc: Stefan Achatz, Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20260618030036.1880139-1-michael.bommarito@gmail.com>
On Wed, 17 Jun 2026, Michael Bommarito wrote:
> The Roccat Kone driver uses an 8-bit value taken straight from a USB HID
> interrupt report as an index into a fixed 5-element profiles[] array,
> without any range check. A malicious or counterfeit device that claims
> the Roccat Kone VID/PID can send a "switch profile" report with an
> out-of-range value and make the driver read out of bounds; the same
> unbounded index is also reachable at probe time from a device-supplied
> startup_profile field. The read result is stored in actual_dpi and
> exposed to user space through the actual_dpi sysfs attribute.
>
> Michael Bommarito (2):
> HID: roccat: bound device-supplied profile index
> HID: roccat: add KUnit test for kone profile-index bounds
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 0/4] HID: sony: more cleanup
From: Jiri Kosina @ 2026-06-29 9:11 UTC (permalink / raw)
To: Rosalie Wanders; +Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <5o965110-3959-0061-1s41-q7224452q746@xreary.bet>
On Tue, 16 Jun 2026, Jiri Kosina wrote:
> now that this is sent as a series, I can easily apply it on top
> altogether when the merge window is over.
Applied to hid.git#for-7.2/sony, thanks!
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 1/1] HID: core: Fix OOB read in hid_get_report for numbered reports
From: Jiri Kosina @ 2026-06-29 9:10 UTC (permalink / raw)
To: Lee Jones
Cc: Benjamin Tissoires, Johan Hovold, Greg Kroah-Hartman, linux-input,
linux-kernel
In-Reply-To: <20260616112700.1990813-1-lee@kernel.org>
On Tue, 16 Jun 2026, 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>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v4 1/3] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: Jiri Kosina @ 2026-06-29 9:05 UTC (permalink / raw)
To: Lee Jones
Cc: Ping Cheng, Jason Gerecke, Benjamin Tissoires, Peter Hutterer,
Dmitry Torokhov, linux-input, linux-kernel
In-Reply-To: <20260616092658.1714548-1-lee@kernel.org>
Lee,
thanks, it looks good to me now.
I'd still like to get an Ack from either Jason or Ping on this before
applying, especially due to nature of patch #3.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v6] HID: i2c-hid: Refactor _DSM helper and add i2c-hid-acpi-prp0001 driver
From: Jiri Kosina @ 2026-06-29 9:04 UTC (permalink / raw)
To: 谢致邦 (XIE Zhibang)
Cc: linux-input, hansg, dmitry.torokhov, bentiss, dianders,
linux-kernel, sashiko-bot, sashiko-reviews, superm1
In-Reply-To: <tencent_FD772005ED5B8FB435F3DFDF73437BC58F06@qq.com>
On Wed, 24 Jun 2026, 谢致邦 (XIE Zhibang) wrote:
> Move the _DSM call that gets the HID descriptor address from
> i2c-hid-acpi.c into i2c-hid-acpi.h as a static inline so both the ACPI
> and the new PRP0001 driver can use it. While refactoring, move the
> blacklist check and the _DSM call to the top of probe() to avoid a
> pointless alloc when the device is blacklisted or does not implement the
> _DSM.
>
> Some devices, for example the Lenovo KaiTian N60d and Inspur CP300L3,
> are declared with _HID "PRP0001" and _DSD compatible "hid-over-i2c" but
> lack "hid-descr-addr" from the _DSD and provide the HID descriptor
> address only through an ACPI _DSM. The OF driver fails to probe them
> because it requires hid-descr-addr. Add a new driver that handles these
> devices by calling the shared _DSM helper.
>
> Fixes: b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are separate modules")
> Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
> ---
> v2: Name the unused parameter and document why
> acpi_device_fix_up_power() is skipped.
> v3: Add a dev_warn() asking users to contact vendors for firmware
> updates, and use existing locals in devm_kzalloc() and
> acpi_device_fix_up_power().
> v4: Double the power-up delay from 250ms to 500ms.
> v5: Document why of_match_ptr() on the of_match_table is safe when
> CONFIG_OF=n.
> v6: Increase power-up delay from 500ms to 750ms. During cold boot on low
> battery, 500ms causes non-fatal I2C transfer errors (-ENXIO). 750ms
> fixes them.
Thanks for all the effort you've put into this.
[ ... snip ... ]
> --- a/drivers/hid/i2c-hid/Kconfig
> +++ b/drivers/hid/i2c-hid/Kconfig
> @@ -22,6 +22,24 @@ config I2C_HID_ACPI
> will be called i2c-hid-acpi. It will also build/depend on the
> module i2c-hid.
>
> +config I2C_HID_ACPI_PRP0001
> + tristate "Driver for PRP0001 devices missing hid-descr-addr"
> + depends on ACPI
> + depends on DRM || !DRM
> + select I2C_HID_CORE
> + help
> + Say Y here if you want support for I2C HID touchpads that
> + are declared with _HID "PRP0001" and _DSD compatible
> + "hid-over-i2c" but lack the "hid-descr-addr" property from
> + the _DSD. The HID descriptor address is instead provided
> + through an ACPI _DSM. Known affected devices include the
> + Lenovo KaiTian N60d and Inspur CP300L3.
> +
> + If unsure, say N.
> +
> + This support is also available as a module. If so, the
> + module will be called i2c-hid-acpi-prp0001.
The only question I have -- do we really want to have this build-time
selectable by the endusers? It's unlikely that the description will make
sense to most of the people, being very low-level technical, and they'd be
unsure what to chose.
Is there a reason not to include it alwyas under the I2C_HID_CORE
umbrella?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: nintendo: Fix imu_timestamp_us double increment per report
From: Jiri Kosina @ 2026-06-29 8:57 UTC (permalink / raw)
To: Christos Maragkos; +Cc: linux-input, djogorchock, bentiss
In-Reply-To: <20260603152134.94439-1-whitetowersoftware@gmail.com>
On Wed, 3 Jun 2026, Christos Maragkos wrote:
> Previously, the imu_timestamp_us variable was incremented twice per
> report, causing it to advance by two times the desired amount.
>
> This resulted in incorrect jumps in IMU timestamps reported using
> MSC_TIMESTAMP, so userspace applications saw corrupted timing on
> functions such as gyroscope-based aim and motion controls.
>
> This is fixed by removing the redundant increment at the start of the
> report handling so the remaining can account for the full report
> interval.
>
> Fixes: 4ff5b10840a88 ("HID: nintendo: add IMU support")
> Signed-off-by: Christos Maragkos <whitetowersoftware@gmail.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] HID: wacom: avoid copying Bluetooth input reports
From: Jiri Kosina @ 2026-06-29 8:54 UTC (permalink / raw)
To: Ruoyu Wang
Cc: Ping Cheng, Jason Gerecke, Benjamin Tissoires, linux-input,
linux-kernel
In-Reply-To: <20260617072035.3373487-1-ruoyuw560@gmail.com>
On Wed, 17 Jun 2026, Ruoyu Wang wrote:
> wacom_intuos_bt_irq() duplicates the received Bluetooth report with
> kmemdup() so that it can pass 10-byte input report payloads to the
> common Intuos parser. The helper then copies each payload back into
> wacom->data before calling wacom_intuos_irq().
>
> Avoid the allocation and copy by temporarily pointing wacom->data at the
> current 10-byte payload while the common parser runs, then restoring the
> original report pointer. The Bluetooth report parser keeps using the
> original report buffer for dispatch and battery parsing, while the common
> parser sees the same payload bytes as before.
>
> This also removes the unchecked kmemdup() result from the Bluetooth IRQ
> path.
>
> Suggested-by: Jason Gerecke <jason.gerecke@wacom.com>
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v10] HID: steelseries: Add MSI Raider A18 HX A9WJG RGB support
From: Jiri Kosina @ 2026-06-29 8:52 UTC (permalink / raw)
To: David Glushkov
Cc: Benjamin Tissoires, linux-input, linux-kernel, kernel test robot
In-Reply-To: <20260605175035.12257-1-david.glushkov@sntiq.com>
On Fri, 5 Jun 2026, David Glushkov wrote:
> The MSI Raider A18 HX A9WJG exposes two internal SteelSeries USB HID
> devices for RGB lighting: KLC (1038:1122) for the keyboard and ALC
> (1038:1161) for the lightbar/logo zones.
>
> Add DMI-gated support for these devices and expose them as multicolor
> LED class devices. The driver sends the same HID class SET_REPORT
> control transfer as the tested userspace implementation for this
> machine and writes a uniform RGB value to all known keyboard keys or
> ALC zones.
>
> The ALC payload uses sparse LED IDs on this chassis: 0x00, 0x01,
> 0x02 and 0x03 are physical zones, while 0x04 and 0x05 do not appear
> to map to physical LEDs. Unused payload LED ID slots are initialized
> to 0xff so they are ignored by the controller instead of defaulting
> to LED ID 0x00.
>
> Limit RGB support to USB interface 0 and the tested DMI system because
> the KLC product ID is shared across MSI laptop designs and the key
> layout mapping is model-specific. If the DMI or interface check does
> not match, keep the device bound as a regular HID device instead of
> failing probe.
>
> Also make the existing Arctis 9 vendor usage-page check defensive by
> returning false for report descriptors shorter than three bytes before
> inspecting hdev->rdesc[0..2].
>
> Tested on MSI Raider A18 HX A9WJG. Both internal SteelSeries ALC
> (1038:1161) and KLC (1038:1122) HID devices bind on interface 0 and
> create steelseries::lightbar and steelseries::kbd_backlight. Setting
> multi_intensity and brightness changes the keyboard and lightbar
> colors.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202606010709.X0QYNjFZ-lkp@intel.com/
> Signed-off-by: David Glushkov <david.glushkov@sntiq.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v11 0/4] Add MSI Claw HID Configuration Driver
From: Jiri Kosina @ 2026-06-29 8:51 UTC (permalink / raw)
To: Derek J. Clark
Cc: Benjamin Tissoires, Pierre-Loup A . Griffais, Denis Benato,
Zhouwang Huang, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260529072111.7565-1-derekjohn.clark@gmail.com>
On Fri, 29 May 2026, Derek J. Clark wrote:
> This series adds an HID Configuration driver for the MSI Claw line of
> Handheld Gaming PC's. The MSI Claw HID interface provides multiple
> features, such as the ability to switch between xinput, dinput, and a
> desktop mode, RGB control, rumble intensity, and mapping of the rear "M"
> keys. There are additional gamepad modes that are not included in this
> driver as they appear to be used in assembly line testing or are
> incomplete in the firmware. During my testing I found them to be unstable.
>
> The initial version of this driver was written by Denis Benato, which
> contained the initial reverse-engineering and implementation for the
> gamepad mode switching. This work was later expanded by Zhouwang Huang
> to include more gamepad modes and additional features. Finally, I
> refactored the entire driver, fixed multiple bugs, and refined the overall
> format to conform to kernel driver best practices and style guide.
>
> Claude was used initially by Zhouwang Huang to quickly parse HID captures
> during the reverse-engineering of some of the features. Since Claude had
> already been used, as a test of its capabilities I had it implement the
> rumble intensity attribute after I had already rewritten most of the
> driver, which I then manually edited to fix some mistakes. I also used
> Claude to review the driver and these patches for any mistakes and bugs.
>
> Assisted-by: Claude:claude-sonnet-4-6
> Co-developed-by: Denis Benato <denis.benato@linux.dev>
> Signed-off-by: Denis Benato <denis.benato@linux.dev>
> Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
> Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
> ---
> v11:
> - Restore dropped changes from v10.
Thanks Derek.
Sashiko had quite a few insightful comments, I find especially the one
with IRQ-vs-kernel context deadlock on drvdata->profile_lock valid.
Could you please go through those, and address it?
Thanks again for all the effort,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [RFC PATCH] HID: core: quiesce input in hid_hw_stop() to prevent use-after-free
From: Jiri Kosina @ 2026-06-29 8:48 UTC (permalink / raw)
To: Philipp Weber
Cc: bentiss, eadavis, linux-input, linux-kernel, syzkaller-bugs,
syzbot+9eebf5f6544c5e873858
In-Reply-To: <20260519130014.34521-1-kernel@phwe.de>
On Tue, 19 May 2026, Philipp Weber wrote:
> A driver's probe calls hid_device_io_start() to enable input delivery,
> then fails at a later initialization step and unwinds via hid_hw_stop().
> The unwind frees struct hidraw via hidraw_disconnect() while in-flight
> HID reports may still be running on another CPU, dereferencing the
> freed object through hidraw_report_event(). syzbot reports the
> resulting use-after-free for the corsair-psu HID driver.
>
> Edward Adam Davis posted a per-driver fix for corsair-psu that adds
> an explicit hid_device_io_stop() before hid_hw_stop() in the probe
> error path ("hwmon: prevent packets from going to driver for probe",
> 2026-04-28). Auditing the tree shows 15 drivers call
> hid_device_io_start(); 7 also call hid_device_io_stop() and 8 do not:
>
> drivers calling hid_device_io_start() without a matching
> hid_device_io_stop() before hid_hw_stop():
> drivers/hwmon/corsair-psu.c (fix posted by Edward)
> drivers/hwmon/corsair-cpro.c
> drivers/hwmon/nzxt-kraken3.c
> drivers/hwmon/nzxt-smart2.c
> drivers/hwmon/gigabyte_waterforce.c
> drivers/hid/hid-logitech-dj.c
> drivers/hid/hid-nintendo.c
> drivers/hid/hid-mcp2221.c
>
> Roughly half of all callers of the API are exposed. Centralize the
> quiesce in hid_hw_stop() so callers do not have to remember the
> matching stop: if a driver has left hdev->io_started true on entry,
> call hid_device_io_stop() before hid_disconnect().
>
> For the 7 drivers that already call hid_device_io_stop() correctly,
> hdev->io_started is false on entry, the guard short-circuits, and
> behavior is unchanged.
>
> No Fixes: tag because the affected drivers gained their
> hid_device_io_start() calls independently over years; the bug is a
> class-wide API misuse rather than a regression from one commit.
>
> Reported-by: syzbot+9eebf5f6544c5e873858@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=9eebf5f6544c5e873858
> Signed-off-by: Philipp Weber <kernel@phwe.de>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: picolcd: prevent NULL pointer dereference in picolcd_send_and_wait()
From: Jiri Kosina @ 2026-06-29 8:46 UTC (permalink / raw)
To: Georgiy Osokin
Cc: bonbons, bentiss, linux-input, linux-kernel, lvc-project,
s.shtylyov
In-Reply-To: <20260517120639.38003-1-g.osokin@auroraos.dev>
On Sun, 17 May 2026, Georgiy Osokin wrote:
> In picolcd_send_and_wait(), an integer overflow of the signed loop counter
> 'k' can theoretically lead to a NULL pointer dereference of 'raw_data'.
> If the loop executes more than INT_MAX times, 'k' becomes negative,
> making the condition 'k < size' true even when 'size' is 0.
>
> Change the type of 'k' to 'unsigned int' to prevent the overflow and
> eliminate the out-of-bounds access.
>
> Found by Linux Verification Center (linuxtesting.org) with the Svace static
> analysis tool.
>
> Fixes: fabdbf2 ("HID: picoLCD: split driver code")
Next time, please make the shas of commits a little bit longer to avoid
uncertainity.
> Signed-off-by: Georgiy Osokin <g.osokin@auroraos.dev>
Applied, thanks!
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] HID: appleir: fix UAF on pending key_up_timer in remove()
From: Jiri Kosina @ 2026-06-29 8:44 UTC (permalink / raw)
To: Manish Khadka; +Cc: linux-input, Benjamin Tissoires, linux-kernel
In-Reply-To: <20260515173252.77757-1-maskmemanish@gmail.com>
On Fri, 15 May 2026, Manish Khadka wrote:
> appleir_remove() runs hid_hw_stop() before timer_delete_sync().
> hid_hw_stop() synchronously unregisters the HID input device via
> hid_disconnect() -> hidinput_disconnect() -> input_unregister_device(),
> which drops the last reference and frees the underlying input_dev when
> no userspace handle holds it open.
>
> key_up_tick() reads appleir->input_dev and calls input_report_key() /
> input_sync() on it. The timer is armed from appleir_raw_event() with
> a HZ/8 (~125 ms) timeout on every keydown and key-repeat report. If a
> key was pressed shortly before the device is disconnected, the timer
> can fire after hid_hw_stop() has freed input_dev but before the
> teardown drains it.
>
> A simple reorder is not sufficient. Putting the timer drain first
> still leaves a window where a USB URB completion (raw_event) running
> during hid_hw_stop() can call mod_timer() and re-arm the timer, which
> then fires after hidinput_disconnect() has freed input_dev. The same
> URB-completion window also lets raw_event() reach key_up(), key_down()
> and battery_flat() directly, all of which dereference
> appleir->input_dev.
>
> Introduce a 'removing' flag on struct appleir, gated by the existing
> spinlock. appleir_remove() sets the flag under the lock and then
> shuts down the timer with timer_shutdown_sync(), which both drains any
> in-flight callback and permanently disables further mod_timer() calls.
> appleir_raw_event() and key_up_tick() bail out early if the flag is
> set, so no path can arm or run the timer, or dereference
> appleir->input_dev, after remove() has started tearing down.
>
> The keyrepeat and flatbattery branches of appleir_raw_event()
> previously called into the input layer without holding the spinlock;
> take it now so the flag check is well-defined. This incidentally
> closes a pre-existing read-side race on appleir->current_key in the
> keyrepeat branch.
>
> This bug is structurally a sibling of commit 4db2af929279 ("HID:
> appletb-kbd: fix UAF in inactivity-timer cleanup path") and has been
> present since the driver was introduced.
>
> Fixes: 9a4a5574ce42 ("HID: appleir: add support for Apple ir devices")
> Cc: stable@vger.kernel.org
> Signed-off-by: Manish Khadka <maskmemanish@gmail.com>
Applied, thank you.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] HID: letsketch: fix UAF on inrange_timer at driver unbind
From: Jiri Kosina @ 2026-06-29 8:43 UTC (permalink / raw)
To: Manish Khadka
Cc: linux-input, Hans de Goede, Benjamin Tissoires, linux-kernel
In-Reply-To: <20260515164200.77039-1-maskmemanish@gmail.com>
On Fri, 15 May 2026, Manish Khadka wrote:
> letsketch_driver does not provide a .remove callback, but
> letsketch_probe() arms a per-device timer:
>
> timer_setup(&data->inrange_timer, letsketch_inrange_timeout, 0);
>
> The timer is re-armed from letsketch_raw_event() with a 100 ms
> timeout on every pen-in-range report, and its callback dereferences
> data->input_tablet to deliver a synthetic BTN_TOOL_PEN release.
>
> letsketch_data is allocated with devm_kzalloc(), and its input_dev
> fields are devm-allocated via letsketch_setup_input_tablet(). On
> device unbind (USB unplug or rmmod), the HID core runs its default
> teardown and devm cleanup frees both letsketch_data and the input
> devices. Because no .remove callback exists, nothing drains the
> timer first: if raw_event armed it within ~100 ms of the unbind,
> the pending timer fires on freed memory. This is a UAF read of
> data and of data->input_tablet, followed by input_report_key() /
> input_sync() into the freed input_dev.
>
> The same problem can occur on the probe error path: if
> hid_hw_start() enabled I/O on an always-poll-quirk device and then
> failed, raw_event may have armed the timer before devm releases
> data.
>
> Fix by adding a .remove callback that calls hid_hw_stop() first.
> hid_hw_stop() synchronously kills the URBs that deliver raw_event(),
> so once it returns no path can re-arm the timer. timer_shutdown_sync()
> then drains any in-flight callback and permanently disables further
> mod_timer() calls. Apply the same timer_shutdown_sync() in the probe
> error path so the timer is guaranteed not to outlive data.
>
> Fixes: 33a5c2793451 ("HID: Add new Letsketch tablet driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Manish Khadka <maskmemanish@gmail.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] Input: maplemouse - fix NULL pointer dereference in open()
From: sashiko-bot @ 2026-06-28 23:27 UTC (permalink / raw)
To: Florian Fuchs; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260628230715.2982552-1-fuchsfl@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Initialization of maple bus driver data occurs after exposing the input device to userspace, leading to a potential NULL pointer dereference.
- [High] Teardown asymmetry causes a race condition where a background callback is inadvertently re-armed during device removal, leading to UAF or NULL pointer dereference.
--
commit 5babe5754528798c9e3476330c1a7834fe83388a
Author: Florian Fuchs <fuchsfl@gmail.com>
Input: maplemouse - fix NULL pointer dereference in open()
Commit 555c765b0cc2 dropped the input_set_drvdata() call in probe, which
caused a NULL pointer dereference in open() because maple_get_drvdata()
was still being used. This patch restores input_set_drvdata() and
converts open() and close() to use input_get_drvdata().
> diff --git a/drivers/input/mouse/maplemouse.c b/drivers/input/mouse/maplemouse.c
> index c99f7e2342191..c41182766538a 100644
> --- a/drivers/input/mouse/maplemouse.c
> +++ b/drivers/input/mouse/maplemouse.c
[ ... ]
> @@ -58,7 +58,7 @@ static int dc_mouse_open(struct input_dev *dev)
>
> static void dc_mouse_close(struct input_dev *dev)
> {
> - struct dc_mouse *mse = maple_get_drvdata(to_maple_dev(&dev->dev));
> + struct dc_mouse *mse = input_get_drvdata(dev);
>
> maple_getcond_callback(mse->mdev, dc_mouse_callback, 0,
> MAPLE_FUNC_MOUSE);
> }
[Severity: High]
This is a pre-existing issue, but does calling maple_getcond_callback() here
create a race condition during device removal?
During removal, remove_maple_mouse() clears the callback before unregistering
the device:
drivers/input/mouse/maplemouse.c:remove_maple_mouse() {
...
mdev->callback = NULL;
input_unregister_device(mse->dev);
...
}
If the device is open, input_unregister_device() synchronously calls
dc_mouse_close(). This unconditionally calls maple_getcond_callback(), which
re-arms mdev->callback back to dc_mouse_callback.
After input_unregister_device() returns, remove_maple_mouse() clears the
driver data and frees mse. If a delayed maple bus packet arrives after this,
dc_mouse_callback() will execute and attempt to use a freed mse pointer or a
NULL driver data pointer, causing a kernel panic.
Would it be safer to avoid re-arming the callback if the device is being
removed?
[ ... ]
> @@ -88,6 +88,7 @@ static int probe_maple_mouse(struct device *dev)
> mse->dev = input_dev;
> mse->mdev = mdev;
>
> + input_set_drvdata(input_dev, mse);
> input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
> input_dev->keybit[BIT_WORD(BTN_MOUSE)] = BIT_MASK(BTN_LEFT) |
> BIT_MASK(BTN_RIGHT) | BIT_MASK(BTN_MIDDLE);
[Severity: High]
This is a pre-existing issue, but is it possible for a callback to fire
before driver data is fully initialized?
Later in probe_maple_mouse(), the device is registered:
drivers/input/mouse/maplemouse.c:probe_maple_mouse() {
...
error = input_register_device(input_dev);
if (error)
goto fail_register;
mdev->driver = mdrv;
maple_set_drvdata(mdev, mse);
...
}
If userspace immediately opens the device upon registration, dc_mouse_open()
will schedule dc_mouse_callback(). If the callback executes before
maple_set_drvdata() completes, maple_get_drvdata(mapledev) inside the
callback will return NULL, leading to a NULL pointer dereference when
accessing mse->dev.
Could the maple_set_drvdata() call be moved before input_register_device()
to prevent this?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628230715.2982552-1-fuchsfl@gmail.com?part=1
^ permalink raw reply
* [PATCH] Input: maplemouse - fix NULL pointer dereference in open()
From: Florian Fuchs @ 2026-06-28 23:07 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: linux-sh, Guenter Roeck, linux-kernel, Florian Fuchs
Commit 555c765b0cc2 ("Input: mouse - drop unnecessary calls to
input_set_drvdata") dropped the input_set_drvdata() call in probe
because the data appeared to be unused. However, dc_mouse_open() and
dc_mouse_close() were using maple_get_drvdata(to_maple_dev(&dev->dev)).
This actually retrieves driver data from the input device's embedded
struct device. After input_set_drvdata() was removed, that lookup started
returning NULL and opening the input device dereferences mse->mdev.
Restore input_set_drvdata() and convert open() and close() to use
input_get_drvdata() so the dependency is no longer hidden.
Fixes: 555c765b0cc2 ("Input: mouse - drop unnecessary calls to input_set_drvdata")
Signed-off-by: Florian Fuchs <fuchsfl@gmail.com>
---
This fix was tested on the target platform. The following is the error I
get, when using the unpatched kernel:
BUG: unable to handle kernel NULL pointer dereference at 00000004
PC: [<8c26eec4>] dc_mouse_open+0xc/0x28
pgd = f700ee57
[00000004] *pgd=00000000
Oops: 0000 [#1]
CPU: 0 UID: 0 PID: 45 Comm: Xfbdev Not tainted 7.1.1 #84 PREEMPT
PC is at dc_mouse_open+0xc/0x28
PR is at input_open_device+0x7c/0xe0
PC : 8c26eec4 SP : 8c7bbd9c SR : 40008100 TEA : 00000004
R0 : 8c26eeb8 R1 : 00000000 R2 : 00000001 R3 : 00000000
R4 : 8c6b0dc0 R5 : 8c26efa8 R6 : 8c7b64c0 R7 : 00000200
R8 : 00000000 R9 : 8c6b0d70 R10 : 8c6b0c00 R11 : 8c6ce604
R12 : 8c390a64 R13 : 8c6b0d3c R14 : 8c0e9ba0
MACH: 00000006 MACL: 8686868d GBR : 29609ff4 PR : 8c265fc8
Call trace:
[<8c265fc8>] input_open_device+0x7c/0xe0
[<8c26b2d0>] mousedev_open_device+0x38/0x68
[<8c26b77c>] mousedev_open+0xa4/0x110
[<8c0e9cc6>] chrdev_open+0x112/0x15c
[<8c0e2e42>] do_dentry_open+0x27e/0x2fc
[<8c0e9bb4>] chrdev_open+0x0/0x15c
[<8c0f32d2>] path_openat+0x1d2/0x7cc
[<8c0f3956>] do_file_open+0x8a/0xf0
[<8c0f3100>] path_openat+0x0/0x7cc
[<8c1efeac>] strncpy_from_user+0x64/0xe4
[<8c0ffc7e>] alloc_fd+0x106/0x124
[<8c0e41ed>] sys_openat2+0xb9/0xbc
[<8c0e3fc6>] do_sys_openat2+0x76/0xd4
[<8c0e40ee>] do_sys_open+0x2a/0x54
[<8c00e25a>] syscall_call+0x18/0x1e
[<8c0e4118>] sys_open+0x0/0x10
drivers/input/mouse/maplemouse.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/input/mouse/maplemouse.c b/drivers/input/mouse/maplemouse.c
index c99f7e234219..c41182766538 100644
--- a/drivers/input/mouse/maplemouse.c
+++ b/drivers/input/mouse/maplemouse.c
@@ -48,7 +48,7 @@ static void dc_mouse_callback(struct mapleq *mq)
static int dc_mouse_open(struct input_dev *dev)
{
- struct dc_mouse *mse = maple_get_drvdata(to_maple_dev(&dev->dev));
+ struct dc_mouse *mse = input_get_drvdata(dev);
maple_getcond_callback(mse->mdev, dc_mouse_callback, HZ/50,
MAPLE_FUNC_MOUSE);
@@ -58,7 +58,7 @@ static int dc_mouse_open(struct input_dev *dev)
static void dc_mouse_close(struct input_dev *dev)
{
- struct dc_mouse *mse = maple_get_drvdata(to_maple_dev(&dev->dev));
+ struct dc_mouse *mse = input_get_drvdata(dev);
maple_getcond_callback(mse->mdev, dc_mouse_callback, 0,
MAPLE_FUNC_MOUSE);
@@ -88,6 +88,7 @@ static int probe_maple_mouse(struct device *dev)
mse->dev = input_dev;
mse->mdev = mdev;
+ input_set_drvdata(input_dev, mse);
input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
input_dev->keybit[BIT_WORD(BTN_MOUSE)] = BIT_MASK(BTN_LEFT) |
BIT_MASK(BTN_RIGHT) | BIT_MASK(BTN_MIDDLE);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] HID: hiddev: keep state alive through disconnect unlock
From: Hillf Danton @ 2026-06-28 22:26 UTC (permalink / raw)
To: Yousef Alhouseen
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-usb,
linux-kernel, syzbot+563191a4939ddbfe73d4
In-Reply-To: <20260628093245.42065-1-alhouseenyousef@gmail.com>
On Sun, 28 Jun 2026 11:32:45 +0200 Yousef Alhouseen wrote:
> hiddev_disconnect() drops existancelock before freeing the hiddev state,
> but a waiting final file release can run as soon as the mutex becomes
> available. On PREEMPT_RT, that waiter may free hiddev while the disconnect
> thread is still executing the mutex slow-unlock path, causing a
> use-after-free in the mutex implementation.
>
The root cause is not specified as clearly as expected.
hiddev_disconnect() hiddev_release()
--- ---
mutex_lock(&hiddev->existancelock);
hiddev->exist = 0;
if (hiddev->open) {
hid_hw_close(hiddev->hid);
wake_up_interruptible(&hiddev->wait);
mutex_unlock(&hiddev->existancelock);
__mutex_unlock_slowpath()
raw_spin_lock_irqsave() //UAF
mutex_lock(&list->hiddev->existancelock);
if (!--list->hiddev->open) {
if (list->hiddev->exist) {
hid_hw_close(list->hiddev->hid);
hid_hw_power(list->hiddev->hid, PM_HINT_NORMAL);
} else {
mutex_unlock(&list->hiddev->existancelock);
kfree(list->hiddev); // free mem
vfree(list);
return 0;
}
}
mutex_unlock(&list->hiddev->existancelock);
Given the syzbot report, uaf occured upon acquiring the raw spinlock in
__mutex_unlock_slowpath(), but no mutex waiter could be waken up without
the raw spinlock held, thus the report sounds false positive.
> Give the connection and each open file an explicit reference. Drop each
> reference only after its existancelock critical section has completed, so
> the state cannot be freed from the other unlock path.
>
> Fixes: 079034073faf ("HID: hiddev cleanup -- handle all error conditions properly")
> Reported-by: syzbot+563191a4939ddbfe73d4@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=563191a4939ddbfe73d4
> Cc: stable@vger.kernel.org
> Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
> ---
> drivers/hid/usbhid/hiddev.c | 37 +++++++++++++++++++------------------
> include/linux/hiddev.h | 2 ++
> 2 files changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> index 6378801b22c6..21396481995b 100644
> --- a/drivers/hid/usbhid/hiddev.c
> +++ b/drivers/hid/usbhid/hiddev.c
> @@ -46,6 +46,12 @@ struct hiddev_list {
> struct mutex thread_lock;
> };
>
> +static void hiddev_put(struct hiddev *hiddev)
> +{
> + if (refcount_dec_and_test(&hiddev->refcount))
> + kfree(hiddev);
> +}
> +
> /*
> * Find a report, given the report's type and ID. The ID can be specified
> * indirectly by REPORT_ID_FIRST (which returns the first report of the given
> @@ -216,26 +222,21 @@ static int hiddev_fasync(int fd, struct file *file, int on)
> static int hiddev_release(struct inode * inode, struct file * file)
> {
> struct hiddev_list *list = file->private_data;
> + struct hiddev *hiddev = list->hiddev;
> unsigned long flags;
>
> - spin_lock_irqsave(&list->hiddev->list_lock, flags);
> + spin_lock_irqsave(&hiddev->list_lock, flags);
> list_del(&list->node);
> - spin_unlock_irqrestore(&list->hiddev->list_lock, flags);
> + spin_unlock_irqrestore(&hiddev->list_lock, flags);
>
> - mutex_lock(&list->hiddev->existancelock);
> - if (!--list->hiddev->open) {
> - if (list->hiddev->exist) {
> - hid_hw_close(list->hiddev->hid);
> - hid_hw_power(list->hiddev->hid, PM_HINT_NORMAL);
> - } else {
> - mutex_unlock(&list->hiddev->existancelock);
> - kfree(list->hiddev);
> - vfree(list);
> - return 0;
> - }
> + mutex_lock(&hiddev->existancelock);
> + if (!--hiddev->open && hiddev->exist) {
> + hid_hw_close(hiddev->hid);
> + hid_hw_power(hiddev->hid, PM_HINT_NORMAL);
> }
>
> - mutex_unlock(&list->hiddev->existancelock);
> + mutex_unlock(&hiddev->existancelock);
> + hiddev_put(hiddev);
> vfree(list);
>
> return 0;
> @@ -270,6 +271,7 @@ static int __hiddev_open(struct hiddev *hiddev, struct file *file)
> spin_unlock_irq(&hiddev->list_lock);
>
> file->private_data = list;
> + refcount_inc(&hiddev->refcount);
>
> return 0;
>
> @@ -897,6 +899,7 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
> INIT_LIST_HEAD(&hiddev->list);
> spin_lock_init(&hiddev->list_lock);
> mutex_init(&hiddev->existancelock);
> + refcount_set(&hiddev->refcount, 1);
> hid->hiddev = hiddev;
> hiddev->hid = hid;
> hiddev->exist = 1;
> @@ -937,9 +940,7 @@ void hiddev_disconnect(struct hid_device *hid)
> if (hiddev->open) {
> hid_hw_close(hiddev->hid);
> wake_up_interruptible(&hiddev->wait);
> - mutex_unlock(&hiddev->existancelock);
> - } else {
> - mutex_unlock(&hiddev->existancelock);
> - kfree(hiddev);
> }
> + mutex_unlock(&hiddev->existancelock);
> + hiddev_put(hiddev);
> }
> diff --git a/include/linux/hiddev.h b/include/linux/hiddev.h
> index 2164c03d2c72..8e9f8a33e359 100644
> --- a/include/linux/hiddev.h
> +++ b/include/linux/hiddev.h
> @@ -13,6 +13,7 @@
> #ifndef _HIDDEV_H
> #define _HIDDEV_H
>
> +#include <linux/refcount.h>
> #include <uapi/linux/hiddev.h>
>
>
> @@ -24,6 +25,7 @@ struct hiddev {
> int minor;
> int exist;
> int open;
> + refcount_t refcount;
> struct mutex existancelock;
> wait_queue_head_t wait;
> struct hid_device *hid;
> --
> 2.54.0
^ permalink raw reply
* Re: [PATCH 6/6] HID: roccat-ryos: reject short special reports
From: sashiko-bot @ 2026-06-28 17:01 UTC (permalink / raw)
To: Yousef Alhouseen; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260628164611.17467-6-alhouseenyousef@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The `roccat_report_event()` function acquires sleepable mutexes while running in an atomic context (USB URB completion handler), which leads to a "scheduling while atomic" BUG.
--
commit de88f5cf3ef84f642bf5e1815c5ed0d7ee05a061
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>
HID: roccat-ryos: reject short special reports
This commit fixes an out-of-bounds read in ryos_raw_event() by ensuring
that only complete special reports are forwarded to the character-device
layer. It drops reports that are shorter than the expected report size.
> diff --git a/drivers/hid/hid-roccat-ryos.c b/drivers/hid/hid-roccat-ryos.c
> index db83f42457da5..5087defc7c373 100644
> --- a/drivers/hid/hid-roccat-ryos.c
> +++ b/drivers/hid/hid-roccat-ryos.c
> @@ -189,7 +189,8 @@ static int ryos_raw_event(struct hid_device *hdev,
> != RYOS_USB_INTERFACE_PROTOCOL)
> return 0;
>
> - if (data[0] != RYOS_REPORT_NUMBER_SPECIAL)
> + if (data[0] != RYOS_REPORT_NUMBER_SPECIAL ||
> + size < sizeof(struct ryos_report_special))
> return 0;
>
> if (ryos != NULL && ryos->roccat_claimed)
> roccat_report_event(ryos->chrdev_minor, data);
[Severity: High]
This is a pre-existing issue, but is it safe to call roccat_report_event()
from this context?
The ryos_raw_event() callback executes in an atomic context (softirq)
during the USB URB completion handler:
hid_irq_in()
hid_safe_input_report()
__hid_input_report()
hdrv->raw_event() -> ryos_raw_event()
However, looking at drivers/hid/hid-roccat.c, roccat_report_event() acquires
sleepable mutexes:
roccat_report_event()
{
...
mutex_lock(&device->readers_lock);
mutex_lock(&device->cbuf_lock);
...
}
Will this sequence trigger a scheduling while atomic bug when a valid
special report is received and forwarded?
>
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628164611.17467-1-alhouseenyousef@gmail.com?part=6
^ permalink raw reply
* Re: [PATCH 1/6] HID: roccat-koneplus: reject short button reports
From: sashiko-bot @ 2026-06-28 17:01 UTC (permalink / raw)
To: Yousef Alhouseen; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260628164611.17467-1-alhouseenyousef@gmail.com>
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Use-After-Free during device teardown due to incorrect removal ordering.
- [High] Incomplete Fix: Out-of-bounds read via short USB reports in the savu driver.
- [High] Memory leak of `struct roccat_reader` and `struct roccat_device` on hot-unplug.
- [High] Sleeping in atomic context due to mutex_lock in the raw_event path.
- [High] Lockless Access and Data Race on the `devices` array causing NULL pointer dereference or UAF.
- [High] Kernel information leak via sysfs attributes due to uninitialized memory copying and ignored errors.
- [Low] Unsynchronized access to `actual_profile` between process and interrupt context.
--
commit d5a3862120a746721ced4fff1b161538d9d1d57c
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>
HID: roccat-koneplus: reject short button reports
The Koneplus raw-event path casts button reports to an eight-byte
structure before updating profile state and forwarding an event. A
malformed USB device can identify a shorter report as a button report
and trigger out-of-bounds reads.
Require the complete button report before either consumer sees it.
Fixes: 47dbdbffe15b ("HID: roccat: Add support for Roccat Kone[+] v2")
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
> diff --git a/drivers/hid/hid-roccat-koneplus.c b/drivers/hid/hid-roccat-koneplus.c
> index f80a60539a967..e0f35251e81a9 100644
> --- a/drivers/hid/hid-roccat-koneplus.c
> +++ b/drivers/hid/hid-roccat-koneplus.c
> @@ -523,6 +523,10 @@ static int koneplus_raw_event(struct hid_device *hdev,
> if (koneplus == NULL)
> return 0;
>
> + if (data[0] == KONEPLUS_MOUSE_REPORT_NUMBER_BUTTON &&
> + size < sizeof(struct koneplus_mouse_report_button))
> + return 0;
> +
[Severity: High]
This is a pre-existing issue, but does the savu driver also need a similar
check?
It looks like the savu driver lacks length validation. savu_raw_event()
forwards reports to savu_report_to_chrdev(), which casts the buffer:
drivers/hid/hid-roccat-savu.c:savu_report_to_chrdev() {
...
roccat_report.type = special_report->type;
roccat_report.data[0] = special_report->data[0];
roccat_report.data[1] = special_report->data[1];
...
}
If a malicious USB device sends a 1-byte report to the savu driver, could
these accesses read out-of-bounds of the heap buffer and leak information
to userspace via the character device?
> koneplus_keep_values_up_to_date(koneplus, data);
[Severity: Low]
This isn't a bug introduced by this patch, but is it safe to update the
profile state here without synchronization?
koneplus->actual_profile is updated from sysfs in process context under
koneplus_lock, but it is also updated here in interrupt context via
koneplus_keep_values_up_to_date() without acquiring any locks:
drivers/hid/hid-roccat-koneplus.c:koneplus_keep_values_up_to_date() {
...
koneplus_profile_activated(koneplus, button_report->data1 - 1);
...
}
Could this data race cause stale profile values to be read if sysfs
attributes are accessed concurrently with profile-changing USB events?
>
> if (koneplus->roccat_claimed)
[Severity: High]
This is a pre-existing issue, but can this path sleep in atomic context?
koneplus_raw_event() runs in atomic interrupt context from the USB URB
completion handler. If roccat_claimed is true, it calls
koneplus_report_to_chrdev(), which eventually calls roccat_report_event():
drivers/hid/hid-roccat.c:roccat_report_event() {
...
mutex_lock(&device->readers_lock);
mutex_lock(&device->cbuf_lock);
...
}
Would acquiring these mutexes inside the raw_event path trigger a
scheduling-while-atomic bug upon receiving valid input events?
[Severity: High]
This is another pre-existing issue, but does roccat_report_event() have a
data race with device removal?
drivers/hid/hid-roccat.c:roccat_report_event() {
...
device = devices[minor];
new_value = kmemdup(data, device->report_size, GFP_ATOMIC);
...
}
If roccat_disconnect() runs concurrently, takes devices_lock, and sets
devices[minor] = NULL, could roccat_report_event() fetch a NULL or freed
device pointer locklessly, leading to a crash or use-after-free when
accessing device->report_size?
I also noticed a few other pre-existing issues in the ROCCAT subsystem
that might be worth addressing:
[Severity: Critical]
This is a pre-existing issue, but could there be a use-after-free during
device teardown?
In koneplus_remove(), the driver state is freed before the hardware is
stopped:
drivers/hid/hid-roccat-koneplus.c:koneplus_remove() {
...
koneplus_remove_specials(hdev);
hid_hw_stop(hdev);
...
}
If a USB event arrives exactly during this window before hid_hw_stop()
halts the in-flight USB URBs, koneplus_raw_event() will fetch the
freed driver data:
drivers/hid/hid-roccat-koneplus.c:koneplus_raw_event() {
struct koneplus_device *koneplus = hid_get_drvdata(hdev);
...
koneplus_keep_values_up_to_date(koneplus, data);
...
}
Would this dereference the freed koneplus pointer?
[Severity: High]
This is a pre-existing issue, but does hot-unplugging the device while it
is held open by userspace cause a memory leak?
When a ROCCAT device is unplugged, roccat_disconnect() clears the array slot:
drivers/hid/hid-roccat.c:roccat_disconnect() {
...
devices[minor] = NULL;
...
}
If a user-space process holds the char device open, the memory isn't freed
immediately. When the process later closes the file descriptor,
roccat_release() checks the array:
drivers/hid/hid-roccat.c:roccat_release() {
...
device = devices[minor];
if (!device) {
mutex_unlock(&devices_lock);
pr_emerg("roccat device with minor %d doesn't exist\n", minor);
return -ENODEV;
}
...
}
Since the device was set to NULL during disconnect, it returns early and
bypasses freeing the reader and device structures. Are these structures
leaked permanently?
[Severity: High]
This is a pre-existing issue, but can sysfs attributes leak uninitialized
heap memory to userspace?
In roccat_common2_receive(), a buffer is allocated via kmalloc() without
GFP_ZERO. If usb_control_msg() fails, it still executes memcpy():
drivers/hid/hid-roccat-common.c:roccat_common2_receive() {
...
len = usb_control_msg(...);
memcpy(data, buf, size);
...
}
Sysfs show handlers ignore the return value and emit the potentially
uninitialized data:
drivers/hid/hid-roccat-koneplus.c:koneplus_sysfs_show_firmware_version() {
...
roccat_common2_receive(usb_dev, KONEPLUS_COMMAND_INFO,
&info, KONEPLUS_SIZE_INFO);
...
return sysfs_emit(buf, "%d\n", info.firmware_version);
}
If the USB transfer fails, does this expose uninitialized memory to any
local user able to read the sysfs files?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628164611.17467-1-alhouseenyousef@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH] HID: cp2112: validate input response lengths
From: sashiko-bot @ 2026-06-28 17:00 UTC (permalink / raw)
To: Yousef Alhouseen; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260628164731.17614-1-alhouseenyousef@gmail.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] The irqchip callback `irq_shutdown` causes a system panic by sleeping in atomic context.
- [High] Concurrent masking and unmasking corrupts `dev->irq_mask`.
- [High] Stale state reads occur due to missing memory ordering between data updates and the availability flag.
- [High] Late USB responses from aborted I2C transfers wrongly satisfy new transfer wait conditions.
--
commit 4d019b9aa500a3c35cd141dda658a1f88ad1c46a
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>
HID: cp2112: validate input response lengths
cp2112_raw_event() parses transfer-status responses as a fixed seven-byte
structure without checking the report size. It also trusts the length in
data-read responses and copies that many bytes even when the received
report is shorter. A malformed USB device can use either path to trigger
out-of-bounds reads from the HID input buffer.
Reject short status responses with -EMSGSIZE. Treat truncated data
responses as zero-length reads so the waiting transfer fails instead of
timing out or copying beyond the report.
Fixes: e932d8178667 ("HID: add hid-cp2112 driver")
Cc: stable@vger.kernel.org
> diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> index 04379db93571b..f2988659a5cbb 100644
> --- a/drivers/hid/hid-cp2112.c
> +++ b/drivers/hid/hid-cp2112.c
> @@ -1430,6 +1430,12 @@ static int cp2112_raw_event(struct hid_device *hdev, struct hid_report *report,
>
> switch (data[0]) {
> case CP2112_TRANSFER_STATUS_RESPONSE:
> + if (size < sizeof(*xfer)) {
> + dev->xfer_status = -EMSGSIZE;
> + atomic_set(&dev->xfer_avail, 1);
[Severity: High]
This isn't a bug introduced by this patch, but is it safe to use atomic_set()
here without a write memory barrier?
If a thread waiting on another CPU reads the flag locklessly via
wait_event_interruptible_timeout() in cp2112_wait(), could the CPU reorder
the data stores and loads? For example:
cp2112_wait()
ret = wait_event_interruptible_timeout(dev->wait,
atomic_read(avail), msecs_to_jiffies(RESPONSE_TIMEOUT));
Could this cause the reader to see the flag as set but read stale data?
[Severity: High]
This is also a pre-existing issue, but does setting the availability flag
unconditionally here leave us vulnerable to late USB responses?
If an I2C transfer times out and a new transfer is initiated, could a delayed
response from the aborted transfer arrive here and incorrectly wake up the new
transfer since there's no sequence number checking?
> + break;
> + }
> +
[ ... ]
> case CP2112_DATA_READ_RESPONSE:
> + if (size < 3 || data[2] > size - 3) {
> + dev->read_length = 0;
> + atomic_set(&dev->read_avail, 1);
> + break;
> + }
> +
While reviewing this file, I noticed a couple of other pre-existing issues:
[Severity: High]
This is a pre-existing issue, but does disabling a GPIO IRQ cause a system
panic by sleeping in atomic context?
The irqchip callback irq_shutdown is called by the IRQ core with the
per-IRQ desc->lock raw spinlock held. In cp2112_gpio_irq_shutdown():
cp2112_gpio_irq_shutdown()
cancel_delayed_work_sync(&dev->gpio_poll_worker);
Could calling this sleeping function inside a raw spinlock trigger a BUG
scheduling while atomic?
[Severity: High]
This is another pre-existing issue, but can concurrent masking and unmasking
corrupt dev->irq_mask?
The irq_mask and irq_unmask callbacks are called with the per-pin desc->lock
held, so they can execute concurrently on different CPUs for different pins.
In cp2112_gpio_irq_mask():
__clear_bit(hwirq, &dev->irq_mask);
And in cp2112_gpio_irq_unmask():
__set_bit(hwirq, &dev->irq_mask);
Do these non-atomic bitwise updates create a read-modify-write data race on
the shared dev->irq_mask, leading to lost or stuck interrupts?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628164731.17614-1-alhouseenyousef@gmail.com?part=1
^ 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