* Re: [PATCH 02/10] [v3] input: gpio-keys: make legacy gpiolib optional
From: Arnd Bergmann @ 2026-06-01 18:32 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-input, Dmitry Torokhov, Hans de Goede, Jin Yao
In-Reply-To: <20260530004519.1C8E81F00893@smtp.kernel.org>
On Sat, May 30, 2026, at 02:45, sashiko-bot@kernel.org wrote:
> [Severity: High]
> Does this change break legacy devices that legitimately provide both a valid
> GPIO and a valid IRQ?
>
> If a legacy platform provides both a valid GPIO and an IRQ, this condition
> now evaluates to false.
>
> By skipping the GPIO request entirely, the driver falls back to treating the
> device as a pure IRQ button. This forces the driver to lose hardware
> debouncing and rely on a simulated release timer instead of checking the
> physical GPIO line.
There are only three drivers that provide the ->irq field. Two are the
ones that I'm changing in this patch, the soc_button_array driver
is the third and none of them set ->irq and ->gpio at the same time.
>> */
>> struct gpio_keys_button {
>> unsigned int code;
>> +#ifdef CONFIG_GPIOLIB_LEGACY
>> int gpio;
>> +#endif
>> int active_low;
>
> [Severity: Medium]
> Will this cause build failures for drivers that assign to this field when
> CONFIG_GPIOLIB_LEGACY is disabled?
Yes, that is the intention of the patch: nothing should set the
->gpio field unless GPIOLIB_LEGACY is set.
> Other modern drivers dynamically create the platform device and pass legacy
> GPIO numbers by directly assigning to this field:
>
> drivers/input/misc/soc_button_array.c:soc_button_device_create() {
> ...
> gpio_keys[n_buttons].gpio = gpio;
> ...
> }
>
> When CONFIG_GPIOLIB_LEGACY is disabled, this results in a compilation failure
> since struct gpio_keys_button no longer has the gpio member.
I previously included a patch force-enable GPIOLIB_LEGACY in this
patch series, see
https://lore.kernel.org/all/npijagtgyad33xxlq46b7kwzydhcgt5tkgd5ihsjl6t4czbqyf@umovipwh73i2/
I made a mistake during a rebase, so my older patch was still
included in my randconfig testing but not in the series I
sent. All the other changes in it are now redundant, bit
the soc_button_array portion indeed still remains.
Hans, Dmitry, do you already have plans to deal with the
soc_button_array driver to move it away from legacy GPIOs?
So far I can see four possible ways we can deal with it,
but none that I actually like:
a) delay the problem, apply my original oneline change
to 'select GPIOLIB_LEGACY' and fix it later, so we
can make GPIOLIB_LEGACY default-disabled in 7.3.
b) add a gpiod member to struct gpio_keys_button and skip
the intermediate gpio number here. Clean it up later.
c) always pass the gpio as an interrupt, as the driver
already does for some machines
d) add dynamic device properties that duplicate the
information from ACPI/DMI, so the driver can
stop using platform data
e) disconnect gpio_keys_button from gpio-keys.c and
register the buttons to the input subsystem
directly from soc_button_device_create().
Any suggestions?
Arnd
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: remove excess kernel-doc member in hidpp_scroll_counter
From: Benjamin Tissoires @ 2026-06-01 18:20 UTC (permalink / raw)
To: linux-input, Rosen Penev
Cc: Filipe Laíns, Bastien Nocera, Jiri Kosina, Hans de Goede,
linux-kernel
In-Reply-To: <20260531000150.378695-1-rosenp@gmail.com>
On Sat, 30 May 2026 17:01:50 -0700, Rosen Penev wrote:
> The @dev member described in the kernel-doc does not exist in the
> struct. Remove the stale entry.
>
> Fixes: 0610430e3dea ("HID: logitech-hidpp: add input_device ptr to struct
> hidpp_device")
> Assisted-by: opencode:big-pickle
>
> [...]
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-7.2/logitech), thanks!
[1/1] HID: logitech-hidpp: remove excess kernel-doc member in hidpp_scroll_counter
https://git.kernel.org/hid/hid/c/f22a5db8a7d3
Cheers,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 0/2] HID: wacom: fix sleeping in atomic context in wacom_wac_queue_flush()
From: Benjamin Tissoires @ 2026-06-01 18:19 UTC (permalink / raw)
To: linux-input, dmitry.torokhov, Jinmo Yang
Cc: jikos, stable, Benjamin Tissoires
In-Reply-To: <20260601134124.1560886-1-jinmo44.yang@gmail.com>
On Mon, 01 Jun 2026 22:41:22 +0900, Jinmo Yang wrote:
> wacom_wac_queue_flush() uses GFP_KERNEL for kzalloc, but it can be
> called from atomic context via the .raw_event callback path. Patch 1
> fixes this by switching to GFP_ATOMIC, and patch 2 converts the buffer
> management to use __free(kfree) cleanup as suggested by Dmitry.
>
> Changes since v1:
> - Replaced Suggested-by with Reported-by for Sashiko-bot
> - Added patch 2 to use __free(kfree) cleanup facility (Dmitry)
>
> [...]
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-7.2/wacom), thanks!
[1/2] HID: wacom: use GFP_ATOMIC in wacom_wac_queue_flush()
https://git.kernel.org/hid/hid/c/55f1ad573e34
[2/2] HID: wacom: use cleanup.h for wacom_wac_queue_flush() buffer management
https://git.kernel.org/hid/hid/c/cb605d48dac9
Cheers,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply
* Re: [PATCH 0/1] HID: wacom: fix slab-out-of-bounds write in kfifo_copy_in
From: Benjamin Tissoires @ 2026-06-01 18:19 UTC (permalink / raw)
To: Jason Gerecke, Ping Cheng, Jinmo Yang
Cc: Jiri Kosina, linux-input, linux-kernel, stable
In-Reply-To: <20260524135203.1996265-1-jinmo44.yang@gmail.com>
On Sun, 24 May 2026 22:52:02 +0900, Jinmo Yang wrote:
> I found the following slab-out-of-bounds write in the wacom HID driver
> while fuzzing with syzkaller on v7.1.0-rc4-next-20260522:
>
> BUG: KASAN: slab-out-of-bounds in kfifo_copy_in+0xf3/0x130 lib/kfifo.c:106
> Write of size 3842 at addr ffff888009179000 by task syz.3.9362/61135
>
> CPU: 1 UID: 0 PID: 61135 Comm: syz.3.9362 Not tainted 7.1.0-rc4-next-20260522-dirty #3 PREEMPT(lazy)
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:94 [inline]
> dump_stack_lvl+0x97/0xe0 lib/dump_stack.c:120
> print_address_description mm/kasan/report.c:378 [inline]
> print_report+0x157/0x4c9 mm/kasan/report.c:482
> kasan_report+0xce/0x100 mm/kasan/report.c:595
> check_region_inline mm/kasan/generic.c:186 [inline]
> kasan_check_range+0x10f/0x1e0 mm/kasan/generic.c:200
> __asan_memcpy+0x3c/0x60 mm/kasan/shadow.c:106
> kfifo_copy_in+0xf3/0x130 lib/kfifo.c:106
> __kfifo_in_r lib/kfifo.c:442 [inline]
> __kfifo_in_r+0x1b2/0x230 lib/kfifo.c:434
> wacom_wac_queue_insert drivers/hid/wacom_sys.c:65 [inline]
> wacom_wac_pen_serial_enforce drivers/hid/wacom_sys.c:165 [inline]
> wacom_raw_event+0x900/0xa90 drivers/hid/wacom_sys.c:179
> __hid_input_report.constprop.0+0x39a/0x4d0 drivers/hid/hid-core.c:2161
> uhid_dev_input2 drivers/hid/uhid.c:618 [inline]
> uhid_char_write+0xa8a/0xfa0 drivers/hid/uhid.c:776
> vfs_write+0x2c0/0xe40 fs/read_write.c:686
> ksys_write+0x1f8/0x250 fs/read_write.c:740
> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> do_syscall_64+0xee/0x590 arch/x86/entry/syscall_64.c:94
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> [...]
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-7.2/wacom), thanks!
[1/1] HID: wacom: fix slab-out-of-bounds write in wacom_wac_queue_insert
https://git.kernel.org/hid/hid/c/6b3014ec0e9a
Cheers,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply
* Re: (subset) [PATCH v3 0/2] Add support for Microsoft Surface Pro 12in
From: Benjamin Tissoires @ 2026-06-01 18:18 UTC (permalink / raw)
To: linux-kernel, linux-input, linux-arm-msm, devicetree,
Harrison Vanderbyl
Cc: jikos, andersson, konradybcio, robh, krzk+dt, conor+dt,
dmitry.baryshkov
In-Reply-To: <20260529011619.9586-1-harrison.vanderbyl@gmail.com>
On Fri, 29 May 2026 11:16:14 +1000, Harrison Vanderbyl wrote:
> Changes in v3:
>
> Rebase:
> - Rebased on next-20260528
> - Removed ice device tree changes
>
> Device tree:
> - Fixed C++ style comment in &i2c9 to use /* */ style
> - Flattened mdss_dp3 port into &mdss_dp3_out directly
> - Whitespace and formatting nits
>
> [...]
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-7.2/core), thanks!
[1/2] hid: Pen battery quirk for Surface Pro 12in
https://git.kernel.org/hid/hid/c/3a1e4e77e3ee
Cheers,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply
* Re: (subset) [PATCH 00/11] HID: storing pointers in 'hid_device_id::driver_data'
From: Benjamin Tissoires @ 2026-06-01 18:18 UTC (permalink / raw)
To: Jiri Kosina, Ping Cheng, Jason Gerecke,
Pawel Zalewski (The Capable Hub)
Cc: linux-kernel, linux-input, Christian A. Ehrhardt,
Christian A. Ehrhardt
In-Reply-To: <20260518-mod-devicetable-hid_device_id-v1-0-a08e3989c283@thegoodpenguin.co.uk>
On Mon, 18 May 2026 17:06:19 +0100, Pawel Zalewski (The Capable Hub) wrote:
> The <linux/mod_devicetable.h> has multiple structs that follow
> the pattern of having either 'driver_data' or 'driver_info'
> fields which are of the 'kernel_ulong_t' type. Then how to
> interpret that field is user defined, some users will treat
> the value as an actual integer, others as a valid pointer to
> dereference.
>
> [...]
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-7.2/cleanup_driver_data), thanks!
[05/11] HID: hid-belkin: clean up usage of 'driver_data'
https://git.kernel.org/hid/hid/c/4de4b8a5ddc2
[06/11] HID: hid-cypress: clean up usage of 'driver_data'
https://git.kernel.org/hid/hid/c/73e784ddf895
[07/11] HID: hid-gfrm: clean up usage of 'driver_data'
https://git.kernel.org/hid/hid/c/0b8bb8c3c913
[08/11] HID: hid-ite: clean up usage of 'driver_data'
https://git.kernel.org/hid/hid/c/b11dfa6cc3c8
Cheers,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply
* [PATCH v2 3/3] HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing
From: 谢致邦 (XIE Zhibang) @ 2026-06-01 18:15 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov
Cc: Yeking, bentiss, dianders, jikos, linux-kernel, superm1,
谢致邦 (XIE Zhibang), Pin-yen Lin, Xu Rao,
Kwok Kin Ming, Dan Carpenter
In-Reply-To: <20260601181510.38705-1-Yeking@Red54.com>
Before commit b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are
separate modules"), the unified i2c-hid driver handled both PNP0C50 ACPI
devices and hid-over-i2c OF devices. After the split, devices with _HID
"PRP0001" and _DSD compatible "hid-over-i2c" are only probed by
i2c_hid_of, which requires "hid-descr-addr" in the _DSD. Some devices,
for example the Lenovo KaiTian N60d and Inspur CP300L3, provide the HID
descriptor address only through the _DSM method. Call the common
i2c_hid_core_acpi_get_descriptor() helper as a fallback, and set safe
post-power-on and post-reset-deassert delays.
Fixes: b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are separate modules")
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
drivers/hid/i2c-hid/i2c-hid-of.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c
index 57379b77e977..e925e2d2cfe0 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of.c
@@ -92,6 +92,36 @@ static int i2c_hid_of_probe(struct i2c_client *client)
ihid_of->ops.power_down = i2c_hid_of_power_down;
ret = device_property_read_u32(dev, "hid-descr-addr", &val);
+ if (ret) {
+ /*
+ * Some devices, for example the Lenovo KaiTian N60d and Inspur
+ * CP300L3, declare their I2C HID touchpad with _HID "PRP0001"
+ * and _DSD compatible "hid-over-i2c" but lack the
+ * "hid-descr-addr" property. Fall back to _DSM to obtain the
+ * HID descriptor address.
+ */
+ int dsm_ret = i2c_hid_core_acpi_get_descriptor(dev);
+
+ if (dsm_ret >= 0) {
+ dev_warn(dev,
+ "hid-descr-addr NOT found, using _DSM fallback. Contact vendor for firmware update!\n");
+ val = dsm_ret;
+
+ /*
+ * Firmware providing the descriptor address only
+ * through _DSM may also lack "post-power-on-delay-ms"
+ * or "post-reset-deassert-delay-ms", leaving the
+ * driver without enough delay before the first HID
+ * descriptor read. Set safe defaults to avoid reading
+ * the descriptor before the device has finished its
+ * internal power-on reset.
+ */
+ ihid_of->post_power_delay_ms = 250;
+ ihid_of->post_reset_delay_ms = 250;
+
+ ret = 0;
+ }
+ }
if (ret) {
dev_err(dev, "HID register address not provided\n");
return -ENODEV;
--
2.54.0
^ permalink raw reply related
* [PATCH v2 2/3] HID: i2c-hid: Move common ACPI _DSM helper into core
From: 谢致邦 (XIE Zhibang) @ 2026-06-01 18:15 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov
Cc: Yeking, bentiss, dianders, jikos, linux-kernel, superm1,
谢致邦 (XIE Zhibang), Pin-yen Lin, Xu Rao,
Kwok Kin Ming, Dan Carpenter
In-Reply-To: <20260601181510.38705-1-Yeking@Red54.com>
Move the _DSM call that gets the HID descriptor address from
i2c-hid-acpi.c to a shared helper in i2c-hid-core.c so both
i2c-hid-acpi.c and i2c-hid-of.c can use it.
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
drivers/hid/i2c-hid/i2c-hid-acpi.c | 32 ++++-----------------------
drivers/hid/i2c-hid/i2c-hid-core.c | 35 ++++++++++++++++++++++++++++++
drivers/hid/i2c-hid/i2c-hid.h | 11 ++++++++++
3 files changed, 50 insertions(+), 28 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
index f65fb6396b69..234789a07047 100644
--- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
@@ -25,12 +25,12 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pm.h>
-#include <linux/uuid.h>
#include "i2c-hid.h"
struct i2c_hid_acpi {
struct i2chid_ops ops;
+ struct i2c_client *client;
struct acpi_device *adev;
};
@@ -48,36 +48,11 @@ static const struct acpi_device_id i2c_hid_acpi_blacklist[] = {
{ }
};
-/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
-static guid_t i2c_hid_guid =
- GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
- 0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
-
-static int i2c_hid_acpi_get_descriptor(struct i2c_hid_acpi *ihid_acpi)
-{
- struct acpi_device *adev = ihid_acpi->adev;
- acpi_handle handle = acpi_device_handle(adev);
- union acpi_object *obj;
- u16 hid_descriptor_address;
-
- obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
- ACPI_TYPE_INTEGER);
- if (!obj) {
- acpi_handle_err(handle, "Error _DSM call to get HID descriptor address failed\n");
- return -ENODEV;
- }
-
- hid_descriptor_address = obj->integer.value;
- ACPI_FREE(obj);
-
- return hid_descriptor_address;
-}
-
static void i2c_hid_acpi_restore_sequence(struct i2chid_ops *ops)
{
struct i2c_hid_acpi *ihid_acpi = container_of(ops, struct i2c_hid_acpi, ops);
- i2c_hid_acpi_get_descriptor(ihid_acpi);
+ i2c_hid_core_acpi_get_descriptor(&ihid_acpi->client->dev);
}
static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
@@ -102,11 +77,12 @@ static int i2c_hid_acpi_probe(struct i2c_client *client)
if (!ihid_acpi)
return -ENOMEM;
+ ihid_acpi->client = client;
ihid_acpi->adev = adev;
ihid_acpi->ops.shutdown_tail = i2c_hid_acpi_shutdown_tail;
ihid_acpi->ops.restore_sequence = i2c_hid_acpi_restore_sequence;
- ret = i2c_hid_acpi_get_descriptor(ihid_acpi);
+ ret = i2c_hid_core_acpi_get_descriptor(dev);
if (ret < 0)
return ret;
hid_descriptor_address = ret;
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 3adb16366e93..1e1a8df5686d 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -1405,6 +1405,41 @@ const struct dev_pm_ops i2c_hid_core_pm = {
};
EXPORT_SYMBOL_GPL(i2c_hid_core_pm);
+#ifdef CONFIG_ACPI
+#include <linux/acpi.h>
+
+/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
+static guid_t i2c_hid_guid =
+ GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
+ 0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
+
+int i2c_hid_core_acpi_get_descriptor(struct device *dev)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+ acpi_handle handle;
+ union acpi_object *obj;
+ u16 hid_descriptor_address;
+
+ if (!adev)
+ return -ENODEV;
+
+ handle = acpi_device_handle(adev);
+ obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
+ ACPI_TYPE_INTEGER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID descriptor address failed\n");
+ return -ENODEV;
+ }
+
+ hid_descriptor_address = obj->integer.value;
+ ACPI_FREE(obj);
+
+ return hid_descriptor_address;
+}
+EXPORT_SYMBOL_GPL(i2c_hid_core_acpi_get_descriptor);
+#endif
+
MODULE_DESCRIPTION("HID over I2C core driver");
MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
MODULE_LICENSE("GPL");
diff --git a/drivers/hid/i2c-hid/i2c-hid.h b/drivers/hid/i2c-hid/i2c-hid.h
index 1724a435c783..bc8661c65b1a 100644
--- a/drivers/hid/i2c-hid/i2c-hid.h
+++ b/drivers/hid/i2c-hid/i2c-hid.h
@@ -44,4 +44,15 @@ void i2c_hid_core_shutdown(struct i2c_client *client);
extern const struct dev_pm_ops i2c_hid_core_pm;
+#ifdef CONFIG_ACPI
+struct device;
+int i2c_hid_core_acpi_get_descriptor(struct device *dev);
+#else
+struct device;
+static inline int i2c_hid_core_acpi_get_descriptor(struct device *dev)
+{
+ return -ENODEV;
+}
+#endif
+
#endif
--
2.54.0
^ permalink raw reply related
* [PATCH v2 1/3] HID: i2c-hid-acpi: Move blacklist check to probe() before devm_kzalloc()
From: 谢致邦 (XIE Zhibang) @ 2026-06-01 18:15 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov
Cc: Yeking, bentiss, dianders, jikos, linux-kernel, superm1,
谢致邦 (XIE Zhibang), Pin-yen Lin, Xu Rao,
Kwok Kin Ming, Dan Carpenter
In-Reply-To: <20260601181510.38705-1-Yeking@Red54.com>
Move the check so the function can return early without wasting an
allocation. This is a pure refactoring, no functional change.
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
v2: Merge declaration into assignment.
drivers/hid/i2c-hid/i2c-hid-acpi.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
index abd700a101f4..f65fb6396b69 100644
--- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
@@ -60,9 +60,6 @@ static int i2c_hid_acpi_get_descriptor(struct i2c_hid_acpi *ihid_acpi)
union acpi_object *obj;
u16 hid_descriptor_address;
- if (acpi_match_device_ids(adev, i2c_hid_acpi_blacklist) == 0)
- return -ENODEV;
-
obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
ACPI_TYPE_INTEGER);
if (!obj) {
@@ -93,15 +90,19 @@ static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
static int i2c_hid_acpi_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
+ struct acpi_device *adev = ACPI_COMPANION(dev);
struct i2c_hid_acpi *ihid_acpi;
u16 hid_descriptor_address;
int ret;
+ if (acpi_match_device_ids(adev, i2c_hid_acpi_blacklist) == 0)
+ return -ENODEV;
+
ihid_acpi = devm_kzalloc(&client->dev, sizeof(*ihid_acpi), GFP_KERNEL);
if (!ihid_acpi)
return -ENOMEM;
- ihid_acpi->adev = ACPI_COMPANION(dev);
+ ihid_acpi->adev = adev;
ihid_acpi->ops.shutdown_tail = i2c_hid_acpi_shutdown_tail;
ihid_acpi->ops.restore_sequence = i2c_hid_acpi_restore_sequence;
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v1] HID: i2c-hid-of: Use named initializers for struct i2c_device_id
From: Benjamin Tissoires @ 2026-06-01 18:15 UTC (permalink / raw)
To: Jiri Kosina, Uwe Kleine-König (The Capable Hub)
Cc: linux-input, linux-kernel
In-Reply-To: <20260519160420.1597193-2-u.kleine-koenig@baylibre.com>
On Tue, 19 May 2026 18:04:20 +0200, Uwe Kleine-König (The Capable Hub) wrote:
> While being less compact, using named initializers allows to more easily
> see which members of the structs are assigned which value without having
> to lookup the declaration of the struct. And it's also more robust
> against changes to the struct definition.
>
> This patch doesn't modify the compiled array, only its representation in
> source form benefits. The former was confirmed with x86 and arm64
> builds.
>
> [...]
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-7.2/i2c-hid), thanks!
[1/1] HID: i2c-hid-of: Use named initializers for struct i2c_device_id
https://git.kernel.org/hid/hid/c/eda0f9e57087
Cheers,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply
* [PATCH v2 0/3] HID: i2c-hid: Fix some PRP0001 touchpads probe after OF/ACPI split
From: 谢致邦 (XIE Zhibang) @ 2026-06-01 18:15 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov
Cc: Yeking, bentiss, dianders, jikos, linux-kernel, superm1,
谢致邦 (XIE Zhibang), Pin-yen Lin, Xu Rao,
Kwok Kin Ming, Dan Carpenter
In-Reply-To: <ahnqMMhD8jn51ch7@google.com>
Before commit b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are
separate modules"), the unified i2c-hid driver handled both PNP0C50 ACPI
devices and hid-over-i2c OF devices. After the split, devices with _HID
"PRP0001" and _DSD compatible "hid-over-i2c" are only probed by
i2c_hid_of, which requires "hid-descr-addr" in the _DSD. Some devices,
for example the Lenovo KaiTian N60d and Inspur CP300L3, provide the HID
descriptor address only through the _DSM method and thus fail to probe.
Patch 1 moves the blacklist check so the function can return early
without wasting an allocation.
Patch 2 moves the _DSM call that gets the HID descriptor address from
i2c-hid-acpi.c to a shared helper in i2c-hid-core.c so both
i2c-hid-acpi.c and i2c-hid-of.c can use it.
Patch 3 calls the common helper as a fallback when "hid-descr-addr" is
missing, and sets safe post-power-on and post-reset-deassert delays.
谢致邦 (XIE Zhibang) (3):
HID: i2c-hid-acpi: Move blacklist check to probe() before
devm_kzalloc()
HID: i2c-hid: Move common ACPI _DSM helper into core
HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing
drivers/hid/i2c-hid/i2c-hid-acpi.c | 41 +++++++-----------------------
drivers/hid/i2c-hid/i2c-hid-core.c | 35 +++++++++++++++++++++++++
drivers/hid/i2c-hid/i2c-hid-of.c | 30 ++++++++++++++++++++++
drivers/hid/i2c-hid/i2c-hid.h | 11 ++++++++
4 files changed, 85 insertions(+), 32 deletions(-)
--
2.54.0
^ permalink raw reply
* [PATCH 3/3] HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing
From: 谢致邦 (XIE Zhibang) @ 2026-06-01 17:37 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov
Cc: Yeking, bentiss, dianders, jikos, linux-kernel, superm1,
谢致邦 (XIE Zhibang), Pin-yen Lin, Xu Rao,
Kwok Kin Ming, Dan Carpenter
In-Reply-To: <20260601173722.38151-1-Yeking@Red54.com>
Before commit b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are
separate modules"), the unified i2c-hid driver handled both PNP0C50 ACPI
devices and hid-over-i2c OF devices. After the split, devices with _HID
"PRP0001" and _DSD compatible "hid-over-i2c" are only probed by
i2c_hid_of, which requires "hid-descr-addr" in the _DSD. Some devices,
for example the Lenovo KaiTian N60d and Inspur CP300L3, provide the HID
descriptor address only through the _DSM method. Call the common
i2c_hid_core_acpi_get_descriptor() helper as a fallback, and set safe
post-power-on and post-reset-deassert delays.
Fixes: b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are separate modules")
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
drivers/hid/i2c-hid/i2c-hid-of.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c
index 57379b77e977..e925e2d2cfe0 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of.c
@@ -92,6 +92,36 @@ static int i2c_hid_of_probe(struct i2c_client *client)
ihid_of->ops.power_down = i2c_hid_of_power_down;
ret = device_property_read_u32(dev, "hid-descr-addr", &val);
+ if (ret) {
+ /*
+ * Some devices, for example the Lenovo KaiTian N60d and Inspur
+ * CP300L3, declare their I2C HID touchpad with _HID "PRP0001"
+ * and _DSD compatible "hid-over-i2c" but lack the
+ * "hid-descr-addr" property. Fall back to _DSM to obtain the
+ * HID descriptor address.
+ */
+ int dsm_ret = i2c_hid_core_acpi_get_descriptor(dev);
+
+ if (dsm_ret >= 0) {
+ dev_warn(dev,
+ "hid-descr-addr NOT found, using _DSM fallback. Contact vendor for firmware update!\n");
+ val = dsm_ret;
+
+ /*
+ * Firmware providing the descriptor address only
+ * through _DSM may also lack "post-power-on-delay-ms"
+ * or "post-reset-deassert-delay-ms", leaving the
+ * driver without enough delay before the first HID
+ * descriptor read. Set safe defaults to avoid reading
+ * the descriptor before the device has finished its
+ * internal power-on reset.
+ */
+ ihid_of->post_power_delay_ms = 250;
+ ihid_of->post_reset_delay_ms = 250;
+
+ ret = 0;
+ }
+ }
if (ret) {
dev_err(dev, "HID register address not provided\n");
return -ENODEV;
--
2.54.0
^ permalink raw reply related
* [PATCH 2/3] HID: i2c-hid: Move common ACPI _DSM helper into core
From: 谢致邦 (XIE Zhibang) @ 2026-06-01 17:37 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov
Cc: Yeking, bentiss, dianders, jikos, linux-kernel, superm1,
谢致邦 (XIE Zhibang), Pin-yen Lin, Xu Rao,
Kwok Kin Ming, Dan Carpenter
In-Reply-To: <20260601173722.38151-1-Yeking@Red54.com>
Move the _DSM call that gets the HID descriptor address from
i2c-hid-acpi.c to a shared helper in i2c-hid-core.c so both
i2c-hid-acpi.c and i2c-hid-of.c can use it.
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
drivers/hid/i2c-hid/i2c-hid-acpi.c | 32 ++++-----------------------
drivers/hid/i2c-hid/i2c-hid-core.c | 35 ++++++++++++++++++++++++++++++
drivers/hid/i2c-hid/i2c-hid.h | 11 ++++++++++
3 files changed, 50 insertions(+), 28 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
index acfe0fb9e99a..bd888134f4cd 100644
--- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
@@ -25,12 +25,12 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pm.h>
-#include <linux/uuid.h>
#include "i2c-hid.h"
struct i2c_hid_acpi {
struct i2chid_ops ops;
+ struct i2c_client *client;
struct acpi_device *adev;
};
@@ -48,36 +48,11 @@ static const struct acpi_device_id i2c_hid_acpi_blacklist[] = {
{ }
};
-/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
-static guid_t i2c_hid_guid =
- GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
- 0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
-
-static int i2c_hid_acpi_get_descriptor(struct i2c_hid_acpi *ihid_acpi)
-{
- struct acpi_device *adev = ihid_acpi->adev;
- acpi_handle handle = acpi_device_handle(adev);
- union acpi_object *obj;
- u16 hid_descriptor_address;
-
- obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
- ACPI_TYPE_INTEGER);
- if (!obj) {
- acpi_handle_err(handle, "Error _DSM call to get HID descriptor address failed\n");
- return -ENODEV;
- }
-
- hid_descriptor_address = obj->integer.value;
- ACPI_FREE(obj);
-
- return hid_descriptor_address;
-}
-
static void i2c_hid_acpi_restore_sequence(struct i2chid_ops *ops)
{
struct i2c_hid_acpi *ihid_acpi = container_of(ops, struct i2c_hid_acpi, ops);
- i2c_hid_acpi_get_descriptor(ihid_acpi);
+ i2c_hid_core_acpi_get_descriptor(&ihid_acpi->client->dev);
}
static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
@@ -103,11 +78,12 @@ static int i2c_hid_acpi_probe(struct i2c_client *client)
if (!ihid_acpi)
return -ENOMEM;
+ ihid_acpi->client = client;
ihid_acpi->adev = adev;
ihid_acpi->ops.shutdown_tail = i2c_hid_acpi_shutdown_tail;
ihid_acpi->ops.restore_sequence = i2c_hid_acpi_restore_sequence;
- ret = i2c_hid_acpi_get_descriptor(ihid_acpi);
+ ret = i2c_hid_core_acpi_get_descriptor(dev);
if (ret < 0)
return ret;
hid_descriptor_address = ret;
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 3adb16366e93..1e1a8df5686d 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -1405,6 +1405,41 @@ const struct dev_pm_ops i2c_hid_core_pm = {
};
EXPORT_SYMBOL_GPL(i2c_hid_core_pm);
+#ifdef CONFIG_ACPI
+#include <linux/acpi.h>
+
+/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
+static guid_t i2c_hid_guid =
+ GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
+ 0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
+
+int i2c_hid_core_acpi_get_descriptor(struct device *dev)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+ acpi_handle handle;
+ union acpi_object *obj;
+ u16 hid_descriptor_address;
+
+ if (!adev)
+ return -ENODEV;
+
+ handle = acpi_device_handle(adev);
+ obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
+ ACPI_TYPE_INTEGER);
+ if (!obj) {
+ acpi_handle_err(handle,
+ "Error _DSM call to get HID descriptor address failed\n");
+ return -ENODEV;
+ }
+
+ hid_descriptor_address = obj->integer.value;
+ ACPI_FREE(obj);
+
+ return hid_descriptor_address;
+}
+EXPORT_SYMBOL_GPL(i2c_hid_core_acpi_get_descriptor);
+#endif
+
MODULE_DESCRIPTION("HID over I2C core driver");
MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
MODULE_LICENSE("GPL");
diff --git a/drivers/hid/i2c-hid/i2c-hid.h b/drivers/hid/i2c-hid/i2c-hid.h
index 1724a435c783..bc8661c65b1a 100644
--- a/drivers/hid/i2c-hid/i2c-hid.h
+++ b/drivers/hid/i2c-hid/i2c-hid.h
@@ -44,4 +44,15 @@ void i2c_hid_core_shutdown(struct i2c_client *client);
extern const struct dev_pm_ops i2c_hid_core_pm;
+#ifdef CONFIG_ACPI
+struct device;
+int i2c_hid_core_acpi_get_descriptor(struct device *dev);
+#else
+struct device;
+static inline int i2c_hid_core_acpi_get_descriptor(struct device *dev)
+{
+ return -ENODEV;
+}
+#endif
+
#endif
--
2.54.0
^ permalink raw reply related
* [PATCH 1/3] HID: i2c-hid-acpi: Move blacklist check to probe() before devm_kzalloc()
From: 谢致邦 (XIE Zhibang) @ 2026-06-01 17:37 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov
Cc: Yeking, bentiss, dianders, jikos, linux-kernel, superm1,
谢致邦 (XIE Zhibang), Pin-yen Lin, Xu Rao,
Kwok Kin Ming, Dan Carpenter
In-Reply-To: <20260601173722.38151-1-Yeking@Red54.com>
Move the check so the function can return early without wasting an
allocation. This is a pure refactoring, no functional change.
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
drivers/hid/i2c-hid/i2c-hid-acpi.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
index abd700a101f4..acfe0fb9e99a 100644
--- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
@@ -60,9 +60,6 @@ static int i2c_hid_acpi_get_descriptor(struct i2c_hid_acpi *ihid_acpi)
union acpi_object *obj;
u16 hid_descriptor_address;
- if (acpi_match_device_ids(adev, i2c_hid_acpi_blacklist) == 0)
- return -ENODEV;
-
obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
ACPI_TYPE_INTEGER);
if (!obj) {
@@ -93,15 +90,20 @@ static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
static int i2c_hid_acpi_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
+ struct acpi_device *adev;
struct i2c_hid_acpi *ihid_acpi;
u16 hid_descriptor_address;
int ret;
+ adev = ACPI_COMPANION(dev);
+ if (acpi_match_device_ids(adev, i2c_hid_acpi_blacklist) == 0)
+ return -ENODEV;
+
ihid_acpi = devm_kzalloc(&client->dev, sizeof(*ihid_acpi), GFP_KERNEL);
if (!ihid_acpi)
return -ENOMEM;
- ihid_acpi->adev = ACPI_COMPANION(dev);
+ ihid_acpi->adev = adev;
ihid_acpi->ops.shutdown_tail = i2c_hid_acpi_shutdown_tail;
ihid_acpi->ops.restore_sequence = i2c_hid_acpi_restore_sequence;
--
2.54.0
^ permalink raw reply related
* [PATCH 0/3] HID: i2c-hid: Fix some PRP0001 touchpads probe after OF/ACPI split
From: 谢致邦 (XIE Zhibang) @ 2026-06-01 17:37 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov
Cc: Yeking, bentiss, dianders, jikos, linux-kernel, superm1,
谢致邦 (XIE Zhibang), Pin-yen Lin, Xu Rao,
Kwok Kin Ming, Dan Carpenter
In-Reply-To: <ahnqMMhD8jn51ch7@google.com>
Before commit b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are
separate modules"), the unified i2c-hid driver handled both PNP0C50 ACPI
devices and hid-over-i2c OF devices. After the split, devices with _HID
"PRP0001" and _DSD compatible "hid-over-i2c" are only probed by
i2c_hid_of, which requires "hid-descr-addr" in the _DSD. Some devices,
for example the Lenovo KaiTian N60d and Inspur CP300L3, provide the HID
descriptor address only through the _DSM method and thus fail to probe.
Patch 1 moves the blacklist check so the function can return early
without wasting an allocation.
Patch 2 moves the _DSM call that gets the HID descriptor address from
i2c-hid-acpi.c to a shared helper in i2c-hid-core.c so both
i2c-hid-acpi.c and i2c-hid-of.c can use it.
Patch 3 calls the common helper as a fallback when "hid-descr-addr" is
missing, and sets safe post-power-on and post-reset-deassert delays.
谢致邦 (XIE Zhibang) (3):
HID: i2c-hid-acpi: Move blacklist check to probe() before
devm_kzalloc()
HID: i2c-hid: Move common ACPI _DSM helper into core
HID: i2c-hid-of: Fall back to ACPI _DSM when hid-descr-addr is missing
drivers/hid/i2c-hid/i2c-hid-acpi.c | 42 +++++++-----------------------
drivers/hid/i2c-hid/i2c-hid-core.c | 35 +++++++++++++++++++++++++
drivers/hid/i2c-hid/i2c-hid-of.c | 30 +++++++++++++++++++++
drivers/hid/i2c-hid/i2c-hid.h | 11 ++++++++
4 files changed, 86 insertions(+), 32 deletions(-)
--
2.54.0
^ permalink raw reply
* Re: [PATCH v2 1/4] HID: multitouch: set INPUT_PROP_PRESSUREPAD based on Digitizer/Button Type
From: Rong Zhang @ 2026-06-01 17:25 UTC (permalink / raw)
To: devnull+peter.hutterer.who-t.net
Cc: bentiss, dmitry.torokhov, jikos, linux-input, linux-kernel,
linux-kselftest, peter.hutterer, shuah, vadim
In-Reply-To: <20251222-wip-hid-pressurepad-v2-1-054ac9689bb7@who-t.net>
Hi all,
Hopefully I'm not too late to show up here.
> From: Peter Hutterer <peter.hutterer@who-t.net>
>
> A Digitizer/Button Type value of 1 indicates the device is a
> pressurepad, see
> https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-windows-precision-touchpad-collection#device-capabilities-feature-report
>
> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
> ---
> drivers/hid/hid-multitouch.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 179dc316b4b518d78bdc900d9fd15756c5eba83e..382e6f50c4f7e663af7d028abb8be7cb2e6e7b8e 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -81,6 +81,7 @@ MODULE_LICENSE("GPL");
> #define MT_INPUTMODE_TOUCHPAD 0x03
>
> #define MT_BUTTONTYPE_CLICKPAD 0
> +#define MT_BUTTONTYPE_PRESSUREPAD 1
>
> enum latency_mode {
> HID_LATENCY_NORMAL = 0,
> @@ -179,6 +180,7 @@ struct mt_device {
> __u8 inputmode_value; /* InputMode HID feature value */
> __u8 maxcontacts;
> bool is_buttonpad; /* is this device a button pad? */
> + bool is_pressurepad; /* is this device a pressurepad? */
> bool is_haptic_touchpad; /* is this device a haptic touchpad? */
> bool serial_maybe; /* need to check for serial protocol */
>
> @@ -530,8 +532,14 @@ static void mt_feature_mapping(struct hid_device *hdev,
> }
>
> mt_get_feature(hdev, field->report);
> - if (field->value[usage->usage_index] == MT_BUTTONTYPE_CLICKPAD)
> + switch (field->value[usage->usage_index]) {
> + case MT_BUTTONTYPE_CLICKPAD:
> td->is_buttonpad = true;
> + break;
> + case MT_BUTTONTYPE_PRESSUREPAD:
> + td->is_pressurepad = true;
> + break;
> + }
>
> break;
> case 0xff0000c5:
> @@ -1393,6 +1401,8 @@ static int mt_touch_input_configured(struct hid_device *hdev,
>
> if (td->is_buttonpad)
> __set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
> + if (td->is_pressurepad)
> + __set_bit(INPUT_PROP_PRESSUREPAD, input->propbit);
I noticed that this leads to dual reporting on my device.
Consider previous checks:
if (application == HID_DG_TOUCHPAD) {
mt_application->mt_flags |= INPUT_MT_POINTER;
td->inputmode_value = MT_INPUTMODE_TOUCHPAD;
}
...
/* check for clickpads */
if ((app->mt_flags & INPUT_MT_POINTER) &&
(app->buttons_count == 1))
td->is_buttonpad = true;
... where `td->is_buttonpad' is set to true when a pressure pad has only
one button, i.e., the "touchpad button integrated with digitizer" [1].
Most (if not all) pressure pads fall into this category. As a result,
the presence of INPUT_PROP_PRESSUREPAD is always accompanied by the
presence of INPUT_PROP_BUTTONPAD.
Since the corresponding testcase neither expects dual reporting nor
prohibits it, I am confused of the intended properties to expose. Is it
a mistake or an intended behavior? If it's the former, I am going to
submit a patch to fix it.
[1]: https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-buttons-report-level-usages
Thanks,
Rong
>
> app->pending_palm_slots = devm_kcalloc(&hi->input->dev,
> BITS_TO_LONGS(td->maxcontacts),
>
> --
> 2.51.1
>
^ permalink raw reply
* Re: [bug report] Potential atomicity bug in drivers/input/joydev.c, between joydev_0x_read() and joydev_ioctl_common()
From: Dmitry Torokhov @ 2026-06-01 17:22 UTC (permalink / raw)
To: Ginger; +Cc: linux-input
In-Reply-To: <CAGp+u1YARE-sqwuNtbpG7E4TO-Nt1KQEeWiHn6pjoLz5kmt8HQ@mail.gmail.com>
Hi Ginger,
On Mon, Jun 01, 2026 at 03:07:05PM +0800, Ginger wrote:
> Dear Linux kernel maintainers,
>
> My research-based static analyzer found a potential atomicity bug
> within the 'drivers/input' subsystem, more specifically, in
> 'drivers/input/joydev.c'.
>
> This potential issue is present as of git commit
> eb3f4b7426cfd2b79d65b7d37155480b32259a11 of the mainline kernel.
>
> Potential concurrent triggering executions:
> T0:
> joydev_0x_read
> --> spin_lock_irq(&input->event_lock);
> --> read from joydev->abs
> --> spin_unlock_irq(&input->event_lock);
>
> T1:
> joydev_ioctl_common
> --> case JSIOCSCORR:
> --> write to joydev->abs[i] (no unlocked)
>
> The above trace is meant to demonstrate an illustrative example of the issue:
> IMHO, in 'joydev_0x_read', the 'input->event_lock' is adopted to
> serialize the read
> accesses to joydev's fields like 'abs' and 'keypam' or input's fields
> like 'input->key.
> However, in either case, the write-side accesses to these fields are
> not similarly
> serialized.
Yes, there is lack of locking in joydev. Some of this might be OK (if
we prevent tearing on reads/writes) since the data may actually be
obsolete immediately after we read it, while in many places we
actually do need consistency, especially when we adjust key and axis
maps.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 01/10] [v2] [net-next] net: dsa: b53: hide legacy gpiolib usage on non-mips
From: Arnd Bergmann @ 2026-06-01 16:50 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Dmitry Torokhov, linux-input
In-Reply-To: <20260530004519.A9BAB1F00898@smtp.kernel.org>
On Sat, May 30, 2026, at 02:45, sashiko-bot@kernel.org wrote:
>
> [Severity: High]
> Does this silently ignore probe errors like -EPROBE_DEFER?
>
> Previously, if devm_gpio_request_one failed, the error was returned to the
> caller. Now, b53_switch_get_reset_gpio handles the request and returns an
> ERR_PTR on failure, but that error pointer is simply stored in dev->reset_gpio
> while b53_switch_init unconditionally returns 0.
>
> Later in the code, b53_switch_reset_gpio relies on IS_ERR(dev->reset_gpio)
> to gracefully return, which would silently skip the reset sequence entirely
> if the GPIO controller simply isn't ready yet (-EPROBE_DEFER) or if there
> was a memory allocation failure (-ENOMEM).
The driver already ignored errors previously on BCM47xx. and
I can see that -EPROBE_DEFER cannot happen because the gpio
driver is very simplistic and always built-in.
I've updated the patch to propagate -EPROBE_DEFER anway as that
is clearly the correct thing to do in case it ever gets used extended
to other platforms. All other errors continue to be ignored to
keep the current behavior of the driver.
Arnd
^ permalink raw reply
* Re: [PATCH v4 00/36] HID: iio: basic clean up and introduce devm_ API for HID sensors
From: srinivas pandruvada @ 2026-06-01 15:55 UTC (permalink / raw)
To: Jonathan Cameron, Sanjay Chitroda
Cc: jikos, dlechner, nuno.sa, andy, sakari.ailus, linux-input,
linux-iio, linux-kernel
In-Reply-To: <20260526165639.42d6e98e@jic23-huawei>
On Tue, 2026-05-26 at 16:56 +0100, Jonathan Cameron wrote:
> On Mon, 25 May 2026 00:50:23 +0530
> Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
>
> > From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> >
> > Key highlights:
> > - 0000-0024: General cleanup and kernel few coding style fixes
> > across HID IIO drivers
> > - 0025: Remove unused iio_dev argument from HID IIO sensor helper
> > - 0026: Introduce devm_hid_sensor_setup_trigger() device-managed
> > API
> > - 0027-0036: Convert HID IIO sensor drivers to use the new devm-
> > based trigger setup
> >
> > changes in v4:
> > - Extend the series to cover remaining HID IIO drivers with devm
> > API usage
> > - Reorder patches to place cleanup and warning fix at beginning
> > and,
> > devm-related changes toward the end based on feedback from David
> > - v3 series ->
> > https://lore.kernel.org/all/20260509101040.791404-1-sanjayembedded@gmail.com/
> > changes in v3:
> > - Added cleanup and prepratory changes before adding devm_ API
> > conversion based on self review: 0002, 0004, 0006, 0007 and 0008
> > - Address andy's review comment on commit message and coding style
> > - v2 series ->
> > https://lore.kernel.org/all/20260429175918.2541914-1-sanjayembedded@gmail.com/
> > changes in v2:
> > - Following input from Jonathan and Andy, squash initial patch v1
> > series in single change as individual change should not break
> > anything
> > - Add devm API support and two driver using the same
> > - v1 series ->
> > https://lore.kernel.org/all/20260428071613.1134053-1-sanjayembedded@gmail.com/
> >
> > Testing:
> > - Compiled with W=1 for each patch in series
> > - Build-tested on QEMU x86_64
> >
> > P.S.
> > - Sashiko reported an issue in a different driver and noted that it
> > is not
> > introduced by this series. I have taken this feedback into
> > account and
> > will address the actual issue in a separate series focus on that
> > driver.
> > - Once this series is merged into the IIO tree, a number of HID IIO
> > drivers will become available to fully converted to devm API
> > usage.
> > - The changes are organized across drivers to keep similar
> > modifications
> > grouped together for consistency, making the series easier to
> > review,
> > rather than grouping all changes per driver.
> >
> The series is mostly fine, though not sure what happened with how you
> sent
> it as I have it in seperate threads of 10 patches at a time. If you
> are having
> email troubles with big series, use b4 to send them instead
> (instructions on kernel.org)
>
> My main concern is this is a lot of churn on a critical driver. I'd
> like therefore
> to be able to see the full benefit of those devm patches rather than
> get us
> part way as this does.
>
I am not able to apply this patchset cleanly on 7.1-rc6. This series
does formatting changes, u32 changes and devm changes. Atleast three
different series will be better.
Thanks,
Srinivas
> Jonathan
>
> > Thanks,
> > Sanjay Chitroda
> >
> > Sanjay Chitroda (36):
> > iio: hid-sensors: add missing blank line after declarations
> > iio: gyro: hid-sensor-gyro-3d: use u32 instead of unsigned
> > iio: accel: hid-sensor-accel-3d: use u32 instead of unsigned
> > iio: light: hid-sensor-als: use u32 instead of unsigned
> > iio: light: hid-sensor-prox: use u32 instead of unsigned
> > iio: orientation: hid-sensor-incl-3d: use u32 instead of unsigned
> > iio: orientation: hid-sensor-rotation: use u32 instead of
> > unsigned
> > iio: pressure: hid-sensor-press: use u32 instead of unsigned
> > iio: humidity: hid-sensor-humidity: align parenthesis for
> > readability
> > iio: gyro: hid-sensor-gyro-3d: align parenthesis for readability
> > iio: magnetometer: hid-sensor-magn-3d: align parenthesis for
> > readability
> > iio: humidity: hid-sensor-humidity: use common device for devres
> > iio: position: hid-sensor-custom-intel-hinge: use common device
> > for
> > devres
> > iio: temperature: hid-sensor-temperature: use common device for
> > devres
> > iio: humidity: hid-sensor-humidity: use local struct device
> > iio: gyro: hid-sensor-gyro-3d: use local struct device
> > iio: accel: hid-sensor-accel-3d: use local struct device
> > iio: light: hid-sensor-als: use local struct device
> > iio: light: hid-sensor-prox: use local struct device
> > iio: magnetometer: hid-sensor-magn-3d: use local struct device
> > iio: orientation: hid-sensor-incl-3d: use local struct device
> > iio: orientation: hid-sensor-rotation: use local struct device
> > iio: position: hid-sensor-custom-intel-hinge: use local struct
> > device
> > iio: pressure: hid-sensor-press: use local struct device
> > iio: hid-sensors: remove unused iio_dev argument
> > iio: hid-sensors: introduce device managed API
> > iio: gyro: hid-sensor-gyro-3d: drop hid_sensor_remove_trigger()
> > using
> > devm API
> > iio: humidity: hid-sensor-humidity: drop
> > hid_sensor_remove_trigger()
> > using devm API
> > iio: light: hid-sensor-prox: drop hid_sensor_remove_trigger()
> > using
> > devm API
> > iio: light: hid-sensor-als: drop hid_sensor_remove_trigger()
> > using
> > devm API
> > iio: magnetometer: hid-sensor-magn-3d: drop
> > hid_sensor_remove_trigger() using devm API
> > iio: orientation: hid-sensor-incl-3d: drop
> > hid_sensor_remove_trigger()
> > using devm API
> > iio: orientation: hid-sensor-rotation: drop
> > hid_sensor_remove_trigger() using devm API
> > iio: position: hid-sensor-custom-intel-hinge: drop
> > hid_sensor_remove_trigger() using devm API
> > iio: pressure: hid-sensor-press: drop hid_sensor_remove_trigger()
> > using devm API
> > iio: temperature: hid-sensor-temperature: drop
> > hid_sensor_remove_trigger() using devm API
> >
> > drivers/iio/accel/hid-sensor-accel-3d.c | 30 ++---
> > .../common/hid-sensors/hid-sensor-trigger.c | 24 +++-
> > .../common/hid-sensors/hid-sensor-trigger.h | 5 +-
> > drivers/iio/gyro/hid-sensor-gyro-3d.c | 96 ++++++++------
> > -
> > drivers/iio/humidity/hid-sensor-humidity.c | 61 +++++-----
> > drivers/iio/light/hid-sensor-als.c | 31 +++--
> > drivers/iio/light/hid-sensor-prox.c | 30 ++---
> > drivers/iio/magnetometer/hid-sensor-magn-3d.c | 112 +++++++++-----
> > ----
> > drivers/iio/orientation/hid-sensor-incl-3d.c | 36 +++---
> > drivers/iio/orientation/hid-sensor-rotation.c | 38 +++---
> > .../position/hid-sensor-custom-intel-hinge.c | 27 ++---
> > drivers/iio/pressure/hid-sensor-press.c | 36 +++---
> > .../iio/temperature/hid-sensor-temperature.c | 15 +--
> > 13 files changed, 264 insertions(+), 277 deletions(-)
> >
> >
> > base-commit: 08297ca8422541dde6c8b7e6b1d68bd4aa4568ef
^ permalink raw reply
* Re: [PATCH v3] HID: Expose LattePanda IOTA UPS as a power_supply device
From: Benjamin Tissoires @ 2026-06-01 15:24 UTC (permalink / raw)
To: Andrew Maney; +Cc: jikos, linux-kernel, linux-input
In-Reply-To: <CAO1jzHj6AAB_hkitjsGJDXeN5iiSWcrEdCMeZeuruXmhfwZzYQ@mail.gmail.com>
On May 21 2026, Andrew Maney wrote:
> On Thu, May 21, 2026 at 2:54 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
> >
> > On May 20 2026, Andrew Maney wrote:
> > > This driver exposes the DFRobot LattePanda IOTA UPS board as a standard
> > > power_supply device, allowing desktop environments and power management
> > > tools such as UPower and systemd-logind to display battery status,
> > > remaining capacity, and charging status without any special
> > > configuration. It also enables automatic suspend or shutdown on low
> > > battery and power profile configuration via any tool that supports the
> > > standard power_supply interface.
> > >
> > > The UPS presents itself as an Arduino Leonardo HID device running custom
> > > firmware (VID 0x2341, PID 0x8036). It reports status and capacity via
> > > HID reports 0x07 and 0x0C respectively.
> >
> > As sashiko detected, you are using a standard Arduino Leonardo and not
> > making any specific detections. So I was thinking that maybe we should
> > implement that feature as a HID-BPF program on top of the generic HID
> > handling.
> >
> > However, the handling of battery supplies in the HID generic core is not
> > entirely filling all of the requirements here, but that's something I
> > wanted to do for a couple of month but I have been swamped with other
> > projects.
> >
> > Anyway, I wanted to understand why this product was using a generic
> > Leonardo PID, and: https://wiki.dfrobot.com/dfr1247#tech_specs
> >
> > "it leverages the standard HID-UPS protocol to be natively recognized by
> > Windows as a battery-powered device"
> >
> > So. Instead of working on a custom driver, why not simply implement (or
> > finish the generic implementation) of HID-UPS in hid-input.c?
> >
> > Cheers,
> > Benjamin
> >
>
> Hi Benjamin,
>
> Thank you for the feedback! I'll look into the HID-UPS implementation or a
> HID-BPF program.
>
> I should mention that the kernel does already detect the IOTA UPS via the
> generic HID-UPS support, but the updates that it receives are so slow that
> it becomes practically unusable for a device with smaller capacity batteries
> like this. Changes can take several minutes or more to be detected. I
> originally went with a driver that reads the HID reports directly because it
> was providing updates far faster.
>
> I implemented a check in driver v4 that verifies that the device's HID reports
> are what's expected before binding to it. In theory, that should prevent the
> driver from binding to other Arduino Leonardo devices, but I agree that fixing
> the generic HID-UPS implementation would be a better long-term solution.
>
> Which approach do you think would be more suitable for this, completing the
> HID-UPS implementation or a HID-BPF program? I'm happy to work on either,
> though as a first-time kernel contributor I'd appreciate any pointers
> on where to
> start.
Honestly, if the device is compliant with HID-UPS, you should work in
the kernel implementation. A HID-BPF would be used to fix device issues,
but if the kernel is not correct enough you can't do much in just a
HID-BPF.
If I'm not wrong, the HID-UPS implementation we have currently is the
hid_battery_* functions. And that implementation assumes a HID connected
device is a separate device, not a power supply of the main machine. So
you'll have to deal with a few new HID fields.
Also, I've started the cleanup work on the hid_battery code a couple of
months ago and never got the chance to send it. Maybe I should send it
so you can have a better environment with testing.
Cheers,
Benjamin
>
> Thanks,
> Andrew
>
> > >
> > > The charge limit (80% or 100%) is configured via a physical DIP switch
> > > on the UPS board and cannot be detected automatically. Userspace can
> > > inform the driver of the configured limit via
> > > charge_control_end_threshold.
> > >
> > > ---
> > >
> > > Changes in v3:
> > > - Deferred power_supply registration to workqueue to avoid blocking probe
> > > - Fixed kernel panic when instantiated via uhid by checking hid_is_usb()
> > > before dereferencing USB-specific structures
> > >
> > > - Fixed ERR_PTR dereference in raw_event by only assigning ups->psu on
> > > successful registration
> > >
> > > - Fixed data race on ups->charge_limit using spin_lock_irqsave()
> > > - Removed TIME_TO_EMPTY_NOW and TIME_TO_FULL_NOW properties to avoid
> > > spurious shutdowns
> > >
> > > - Changed plugged-in but not charging state from FULL to NOT_CHARGING
> > > - Used devm_kasprintf() for a unique sysfs name in order to support
> > > multiple devices
> > >
> > > - Added POWER_SUPPLY and HIDRAW dependencies to Kconfig
> > > - Used %pe for more human-readable error messages
> > >
> > > Changes in v2:
> > > - Rebased on top of the current tree
> > > - Moved vendor and device IDs to drivers/hid/hid-ids.h
> > > - Added Kconfig entry under HID bus support -> Special HID drivers
> > > - Added build rule to drivers/hid/Makefile
> > >
> > > Signed-off-by: Andrew Maney <andrewmaney05@gmail.com>
> > > ---
> > > MAINTAINERS | 6 +
> > > drivers/hid/Kconfig | 10 +
> > > drivers/hid/Makefile | 1 +
> > > drivers/hid/hid-ids.h | 3 +
> > > drivers/hid/hid-lattepanda-iota-ups.c | 409 ++++++++++++++++++++++++++
> > > 5 files changed, 429 insertions(+)
> > > create mode 100644 drivers/hid/hid-lattepanda-iota-ups.c
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 10e825318..d80721c2c 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -11416,6 +11416,12 @@ F: include/uapi/linux/hid*
> > > F: samples/hid/
> > > F: tools/testing/selftests/hid/
> > >
> > > +HID LATTEPANDA IOTA UPS DRIVER
> > > +M: Andrew Maney <andrewmaney05@gmail.com>
> > > +L: linux-input@vger.kernel.org
> > > +S: Maintained
> > > +F: drivers/hid/hid-lattepanda-iota-ups.c
> > > +
> > > HID LOGITECH DRIVERS
> > > R: Filipe Laíns <lains@riseup.net>
> > > L: linux-input@vger.kernel.org
> > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > > index ff2f580b6..21ffc2fd0 100644
> > > --- a/drivers/hid/Kconfig
> > > +++ b/drivers/hid/Kconfig
> > > @@ -510,6 +510,16 @@ config HID_KYSONA
> > > Say Y here if you have a Kysona M600 mouse
> > > and want to be able to read its battery capacity.
> > >
> > > +config HID_LATTEPANDA_IOTA_UPS
> > > + tristate "LattePanda IOTA UPS"
> > > + depends on USB_HID && USB_HIDDEV && X86 && POWER_SUPPLY
> > > + help
> > > + Support for the LattePanda IOTA UPS (DFRobot, VID 0x2341 PID 0x8036).
> > > + Exposes the battery status and capacity via the power_supply interface.
> > > +
> > > + To compile as a module, choose M here: the module will be
> > > + called hid-lattepanda-iota-ups.
> > > +
> > > config HID_UCLOGIC
> > > tristate "UC-Logic"
> > > depends on USB_HID
> > > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> > > index 0597fd6a4..d7ad3fc8f 100644
> > > --- a/drivers/hid/Makefile
> > > +++ b/drivers/hid/Makefile
> > > @@ -74,6 +74,7 @@ obj-$(CONFIG_HID_KENSINGTON) += hid-kensington.o
> > > obj-$(CONFIG_HID_KEYTOUCH) += hid-keytouch.o
> > > obj-$(CONFIG_HID_KYE) += hid-kye.o
> > > obj-$(CONFIG_HID_KYSONA) += hid-kysona.o
> > > +obj-$(CONFIG_HID_LATTEPANDA_IOTA_UPS) += hid-lattepanda-iota-ups.o
> > > obj-$(CONFIG_HID_LCPOWER) += hid-lcpower.o
> > > obj-$(CONFIG_HID_LENOVO) += hid-lenovo.o
> > > obj-$(CONFIG_HID_LENOVO_GO) += hid-lenovo-go.o
> > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > > index 4657d96fb..6ded2c943 100644
> > > --- a/drivers/hid/hid-ids.h
> > > +++ b/drivers/hid/hid-ids.h
> > > @@ -859,6 +859,9 @@
> > > #define USB_DEVICE_ID_LD_HYBRID 0x2090
> > > #define USB_DEVICE_ID_LD_HEATCONTROL 0x20A0
> > >
> > > +#define USB_VENDOR_ID_LATTEPANDA_IOTA 0x2341
> > > +#define USB_DEVICE_ID_LATTEPANDA_IOTA_UPS 0x8036
> > > +
> > > #define USB_VENDOR_ID_LENOVO 0x17ef
> > > #define USB_DEVICE_ID_LENOVO_TPKBD 0x6009
> > > #define USB_DEVICE_ID_LENOVO_CUSBKBD 0x6047
> > > diff --git a/drivers/hid/hid-lattepanda-iota-ups.c b/drivers/hid/hid-lattepanda-iota-ups.c
> > > new file mode 100644
> > > index 000000000..f5d522695
> > > --- /dev/null
> > > +++ b/drivers/hid/hid-lattepanda-iota-ups.c
> > > @@ -0,0 +1,409 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +#include <linux/power_supply.h>
> > > +#include <linux/completion.h>
> > > +#include <linux/workqueue.h>
> > > +#include <linux/module.h>
> > > +#include <linux/spinlock.h>
> > > +#include <linux/hid.h>
> > > +#include <linux/usb.h>
> > > +#include "hid-ids.h"
> > > +
> > > +#define REPORT_ID_CAPACITY 0x0C
> > > +#define REPORT_ID_STATUS 0x07
> > > +
> > > +#define STATUS_DISCHARGING BIT(1)
> > > +#define STATUS_PLUGGED_IN BIT(0)
> > > +#define STATUS_CHARGING BIT(2)
> > > +
> > > +MODULE_AUTHOR("Andrew Maney");
> > > +MODULE_DESCRIPTION("LattePanda IOTA UPS power supply driver");
> > > +MODULE_LICENSE("GPL");
> > > +
> > > +struct iota_ups {
> > > + struct power_supply_desc psu_desc;
> > > + struct power_supply *psu;
> > > + struct hid_device *hiddev;
> > > + spinlock_t lock; /* Protects cached HID report values */
> > > +
> > > + /* Cached values that are updated from HID reports */
> > > + bool plugged_in;
> > > + char serial[64];
> > > + int charge_limit;
> > > + int psu_status;
> > > + int capacity;
> > > +
> > > + /*
> > > + * Wait for both status and capacity reports before registering
> > > + * with the power_supply core, so initial values are correct and
> > > + * not erroneous.
> > > + */
> > > + struct completion got_initial_data;
> > > + struct work_struct register_work;
> > > + bool got_capacity;
> > > + bool data_ready;
> > > + bool got_status;
> > > +};
> > > +
> > > +static enum power_supply_property iota_ups_properties[] = {
> > > + POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD,
> > > + POWER_SUPPLY_PROP_SERIAL_NUMBER,
> > > + POWER_SUPPLY_PROP_MANUFACTURER,
> > > + POWER_SUPPLY_PROP_MODEL_NAME,
> > > + POWER_SUPPLY_PROP_TECHNOLOGY,
> > > + POWER_SUPPLY_PROP_CAPACITY,
> > > + POWER_SUPPLY_PROP_PRESENT,
> > > + POWER_SUPPLY_PROP_ONLINE,
> > > + POWER_SUPPLY_PROP_STATUS,
> > > + POWER_SUPPLY_PROP_SCOPE,
> > > +};
> > > +
> > > +static const struct hid_device_id iota_ups_devices[] = {
> > > + { HID_USB_DEVICE(USB_VENDOR_ID_LATTEPANDA_IOTA,
> > > + USB_DEVICE_ID_LATTEPANDA_IOTA_UPS) },
> > > + { }
> > > +};
> > > +MODULE_DEVICE_TABLE(hid, iota_ups_devices);
> > > +
> > > +static int iota_ups_get_property(struct power_supply *supply,
> > > + enum power_supply_property psp,
> > > + union power_supply_propval *val)
> > > +{
> > > + struct iota_ups *ups = power_supply_get_drvdata(supply);
> > > + unsigned long flags;
> > > +
> > > + spin_lock_irqsave(&ups->lock, flags);
> > > +
> > > + switch (psp) {
> > > + case POWER_SUPPLY_PROP_STATUS:
> > > + val->intval = ups->psu_status;
> > > + break;
> > > +
> > > + /* Remaining capacity as a percentage from 0 to 100 */
> > > + case POWER_SUPPLY_PROP_CAPACITY:
> > > + val->intval = ups->capacity;
> > > + break;
> > > +
> > > + /* The UPS is always present if the driver is loaded */
> > > + case POWER_SUPPLY_PROP_PRESENT:
> > > + val->intval = 1;
> > > + break;
> > > +
> > > + /* Whether mains power is connected */
> > > + case POWER_SUPPLY_PROP_ONLINE:
> > > + val->intval = ups->plugged_in ? 1 : 0;
> > > + break;
> > > +
> > > + /*
> > > + * The UPS board supplies power to the IOTA and any
> > > + * peripherals connected to it, therefore its scope
> > > + * is system-wide.
> > > + */
> > > + case POWER_SUPPLY_PROP_SCOPE:
> > > + val->intval = POWER_SUPPLY_SCOPE_SYSTEM;
> > > + break;
> > > +
> > > + /* V1.0 only accepts 18650 Li-ion cells */
> > > + case POWER_SUPPLY_PROP_TECHNOLOGY:
> > > + val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
> > > + break;
> > > +
> > > + /* 80% or 100%, configured via a DIP switch on the UPS board */
> > > + case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
> > > + val->intval = ups->charge_limit;
> > > + break;
> > > +
> > > + case POWER_SUPPLY_PROP_MANUFACTURER:
> > > + val->strval = "DFRobot";
> > > + break;
> > > +
> > > + case POWER_SUPPLY_PROP_MODEL_NAME:
> > > + val->strval = "LattePanda IOTA UPS";
> > > + break;
> > > +
> > > + /* Retrieved from the USB descriptor */
> > > + case POWER_SUPPLY_PROP_SERIAL_NUMBER:
> > > + val->strval = ups->serial;
> > > + break;
> > > +
> > > + default:
> > > + spin_unlock_irqrestore(&ups->lock, flags);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + spin_unlock_irqrestore(&ups->lock, flags);
> > > + return 0;
> > > +}
> > > +
> > > +static int iota_ups_set_property(struct power_supply *supply,
> > > + enum power_supply_property psp,
> > > + const union power_supply_propval *val)
> > > +{
> > > + struct iota_ups *ups = power_supply_get_drvdata(supply);
> > > +
> > > + if (psp == POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD) {
> > > + unsigned long flags;
> > > +
> > > + /*
> > > + * V1.0 supports 80% and 100% charge limits only, which is
> > > + * set via a DIP switch on the board. This property allows
> > > + * userspace to inform the driver which limit is configured.
> > > + */
> > > + if (val->intval != 80 && val->intval != 100)
> > > + return -EINVAL;
> > > +
> > > + spin_lock_irqsave(&ups->lock, flags);
> > > + ups->charge_limit = val->intval;
> > > + spin_unlock_irqrestore(&ups->lock, flags);
> > > + return 0;
> > > + }
> > > +
> > > + return -EINVAL;
> > > +}
> > > +
> > > +static int iota_ups_property_is_writable(struct power_supply *supply,
> > > + enum power_supply_property psp)
> > > +{
> > > + return psp == POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD;
> > > +}
> > > +
> > > +static int iota_ups_raw_event(struct hid_device *hdev,
> > > + struct hid_report *report,
> > > + u8 *data, int size)
> > > +{
> > > + struct iota_ups *ups = hid_get_drvdata(hdev);
> > > + unsigned long flags;
> > > + bool changed = false;
> > > +
> > > + /* All of the UPS's reports are at least 2 bytes */
> > > + if (size < 2)
> > > + return 0;
> > > +
> > > + spin_lock_irqsave(&ups->lock, flags);
> > > +
> > > + switch (data[0]) {
> > > + case REPORT_ID_STATUS: {
> > > + u8 status = data[1];
> > > + int new_status;
> > > + bool plugged_in = !!(status & STATUS_PLUGGED_IN);
> > > +
> > > + /*
> > > + * The UPS status is determined as follows:
> > > + * Battery full:
> > > + * UPS is plugged in
> > > + * Battery is at full capacity
> > > + *
> > > + * Battery charging:
> > > + * UPS is plugged in
> > > + * Battery is not at full capacity
> > > + *
> > > + * Battery discharging:
> > > + * UPS is not plugged in
> > > + *
> > > + * Battery not charging:
> > > + * UPS is plugged in
> > > + * UPS has halted charging for some reason
> > > + *
> > > + * Unknown:
> > > + * None of the above conditions are met
> > > + */
> > > + if (status & STATUS_CHARGING) {
> > > + if (ups->capacity >= ups->charge_limit)
> > > + new_status = POWER_SUPPLY_STATUS_FULL;
> > > + else
> > > + new_status = POWER_SUPPLY_STATUS_CHARGING;
> > > +
> > > + } else if (status & STATUS_DISCHARGING) {
> > > + new_status = POWER_SUPPLY_STATUS_DISCHARGING;
> > > +
> > > + } else if (plugged_in) {
> > > + new_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> > > +
> > > + } else {
> > > + new_status = POWER_SUPPLY_STATUS_UNKNOWN;
> > > + }
> > > +
> > > + if (new_status != ups->psu_status ||
> > > + plugged_in != ups->plugged_in) {
> > > + ups->plugged_in = plugged_in;
> > > + ups->psu_status = new_status;
> > > + changed = true;
> > > + }
> > > +
> > > + ups->got_status = true;
> > > + break;
> > > + }
> > > +
> > > + case REPORT_ID_CAPACITY: {
> > > + int new_cap = clamp((int)data[1], 0, 100);
> > > +
> > > + if (new_cap != ups->capacity) {
> > > + ups->capacity = new_cap;
> > > + changed = true;
> > > + }
> > > +
> > > + ups->got_capacity = true;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + /*
> > > + * Signal that the UPS is ready to be registered because we have
> > > + * received both capacity and status reports.
> > > + */
> > > + if (!ups->data_ready && ups->got_status && ups->got_capacity) {
> > > + ups->data_ready = true;
> > > + complete(&ups->got_initial_data);
> > > + }
> > > +
> > > + spin_unlock_irqrestore(&ups->lock, flags);
> > > +
> > > + /*
> > > + * Notify the power_supply core outside the spinlock to avoid
> > > + * a deadlock; power_supply_changed() may call back into
> > > + * get_property() which acquires the same lock.
> > > + */
> > > + if (changed && ups->psu)
> > > + power_supply_changed(ups->psu);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void iota_ups_register_work(struct work_struct *work)
> > > +{
> > > + struct iota_ups *ups = container_of(work, struct iota_ups, register_work);
> > > + struct power_supply_config psu_config = {};
> > > + struct power_supply *psu;
> > > +
> > > + /*
> > > + * Wait for both status and capacity reports before registering.
> > > + * The device sends reports every ~1 second, so 3 seconds is safe.
> > > + * We wait here in order to prevent registration in an unknown
> > > + * state, since this could cause emergency shutdowns or other
> > > + * undesired effects.
> > > + */
> > > + wait_for_completion_timeout(&ups->got_initial_data,
> > > + msecs_to_jiffies(3000));
> > > +
> > > + /* Configure the UPS's power supply properties */
> > > + ups->psu_desc.name = devm_kasprintf(&ups->hiddev->dev, GFP_KERNEL,
> > > + "lattepanda-iota-ups.%s",
> > > + dev_name(&ups->hiddev->dev));
> > > +
> > > + if (!ups->psu_desc.name) {
> > > + hid_err(ups->hiddev, "failed to allocate power supply name\n");
> > > + return;
> > > + }
> > > +
> > > + ups->psu_desc.property_is_writeable = iota_ups_property_is_writable;
> > > + ups->psu_desc.num_properties = ARRAY_SIZE(iota_ups_properties);
> > > + ups->psu_desc.get_property = iota_ups_get_property;
> > > + ups->psu_desc.set_property = iota_ups_set_property;
> > > + ups->psu_desc.properties = iota_ups_properties;
> > > + ups->psu_desc.type = POWER_SUPPLY_TYPE_BATTERY;
> > > + psu_config.drv_data = ups;
> > > +
> > > + /* Register the UPS as a power_supply device */
> > > + psu = devm_power_supply_register(&ups->hiddev->dev, &ups->psu_desc, &psu_config);
> > > + if (IS_ERR(psu)) {
> > > + hid_err(ups->hiddev, "power supply registration failed: %pe\n", psu);
> > > + return;
> > > + }
> > > +
> > > + /*
> > > + * Finally, notify the power_supply core so userspace reads the correct
> > > + * initial state immediately after registration.
> > > + */
> > > + ups->psu = psu;
> > > + power_supply_changed(ups->psu);
> > > + hid_info(ups->hiddev, "LattePanda IOTA UPS registered as a power_supply device\n");
> > > +}
> > > +
> > > +static int iota_ups_probe(struct hid_device *hdev,
> > > + const struct hid_device_id *id)
> > > +{
> > > + struct iota_ups *ups;
> > > + int ret;
> > > +
> > > + ups = devm_kzalloc(&hdev->dev, sizeof(*ups), GFP_KERNEL);
> > > + if (!ups)
> > > + return -ENOMEM;
> > > +
> > > + ups->hiddev = hdev;
> > > + ups->psu_status = POWER_SUPPLY_STATUS_UNKNOWN;
> > > +
> > > + /* 50% is a safe default if wait_for_completion_timeout() times out. */
> > > + ups->capacity = 50;
> > > +
> > > + /*
> > > + * Default to 100% to prevent unexpected shutdowns.
> > > + * Userspace can update this via charge_control_end_threshold.
> > > + */
> > > + ups->charge_limit = 100;
> > > +
> > > + init_completion(&ups->got_initial_data);
> > > + spin_lock_init(&ups->lock);
> > > + hid_set_drvdata(hdev, ups);
> > > +
> > > + /*
> > > + * Retrieve the UPS's serial number from the USB descriptor. If the device is not
> > > + * a USB device, we can use the unique device identifier as the serial number.
> > > + */
> > > + if (hid_is_usb(hdev)) {
> > > + struct usb_device *udev = to_usb_device(hdev->dev.parent->parent);
> > > +
> > > + if (udev->serial)
> > > + strscpy(ups->serial, udev->serial, sizeof(ups->serial));
> > > + else
> > > + strscpy(ups->serial, "Unknown", sizeof(ups->serial));
> > > + } else {
> > > + if (*hdev->uniq)
> > > + strscpy(ups->serial, hdev->uniq, sizeof(ups->serial));
> > > + else
> > > + strscpy(ups->serial, "Unknown", sizeof(ups->serial));
> > > + }
> > > +
> > > + ret = hid_parse(hdev);
> > > + if (ret) {
> > > + hid_err(hdev, "HID parse failed: %pe\n", ERR_PTR(ret));
> > > + return ret;
> > > + }
> > > +
> > > + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> > > + if (ret) {
> > > + hid_err(hdev, "HID hw start failed: %pe\n", ERR_PTR(ret));
> > > + return ret;
> > > + }
> > > +
> > > + ret = hid_hw_open(hdev);
> > > + if (ret) {
> > > + hid_err(hdev, "HID hw open failed: %pe\n", ERR_PTR(ret));
> > > + goto err_stop;
> > > + }
> > > +
> > > + /* Probe for the UPS in a worker queue so we don't halt the enumeration thread */
> > > + INIT_WORK(&ups->register_work, iota_ups_register_work);
> > > + schedule_work(&ups->register_work);
> > > + return 0;
> > > +
> > > +err_stop:
> > > + hid_hw_stop(hdev);
> > > + return ret;
> > > +}
> > > +
> > > +static void iota_ups_remove(struct hid_device *hdev)
> > > +{
> > > + struct iota_ups *ups = hid_get_drvdata(hdev);
> > > +
> > > + cancel_work_sync(&ups->register_work);
> > > + hid_hw_close(hdev);
> > > + hid_hw_stop(hdev);
> > > +}
> > > +
> > > +static struct hid_driver iota_ups_driver = {
> > > + .name = "lattepanda-iota-ups",
> > > + .id_table = iota_ups_devices,
> > > + .probe = iota_ups_probe,
> > > + .remove = iota_ups_remove,
> > > + .raw_event = iota_ups_raw_event,
> > > +};
> > > +module_hid_driver(iota_ups_driver);
> > > --
> > > 2.54.0
> > >
> > >
>
^ permalink raw reply
* Re: [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: Markus Elfring @ 2026-06-01 14:48 UTC (permalink / raw)
To: Lee Jones, linux-input, Benjamin Tissoires, Dmitry Torokhov,
Jason Gerecke, Jiri Kosina, Peter Hutterer, Ping Cheng
Cc: LKML
In-Reply-To: <20260601140845.1237227-1-lee@kernel.org>
…
> +++ b/drivers/hid/wacom_sys.c
> @@ -877,10 +877,16 @@ static void wacom_remove_shared_data(void *res)
> data = container_of(wacom_wac->shared, struct wacom_hdev_data,
> shared);
>
> - if (wacom_wac->shared->touch == wacom->hdev)
> + mutex_lock(&wacom_udev_list_lock);
> + if (wacom_wac->shared->touch == wacom->hdev) {
> wacom_wac->shared->touch = NULL;
…
> + }
> + mutex_unlock(&wacom_udev_list_lock);
> +
> + synchronize_rcu();
>
> kref_put(&data->kref, wacom_release_shared_data);
…
Under which circumstances would you dare to apply another lock guard?
https://elixir.bootlin.com/linux/v7.1-rc5/source/include/linux/mutex.h#L253
Regards,
Markus
^ permalink raw reply
* Re: [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: sashiko-bot @ 2026-06-01 14:26 UTC (permalink / raw)
To: Lee Jones; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260601140845.1237227-1-lee@kernel.org>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Critical] Moving wacom_add_shared_data() to the end of wacom_parse_and_register() causes an unconditional NULL pointer dereference during device probe.
- [High] Moving wacom_add_shared_data() to the end of the probe function creates a window where wacom_wac->shared is NULL while hardware interrupts are active, leading to an asynchronous NULL pointer dereference.
Pre-existing issues:
- [High] The patch incompletely fixes a Time-of-Check to Time-of-Use (TOCTOU) race condition. It successfully converts wacom_intuos_pad() to use RCU, but leaves identical lockless accesses unprotected in wacom_wac_pad_event(), wacom_wireless_irq(), and wacom_status_irq().
- [High] Modifying input capabilities on the sibling touch device after it has already been registered with the input core.
--
commit 5bec6e8c12bc84f46d120679cc4bef944e215dc2
Author: Lee Jones <lee@kernel.org>
HID: wacom: Fix Use-After-Free in wacom_intuos_pad
This commit resolves a Time-of-Check to Time-of-Use race condition by
introducing RCU protection for the touch_input pointer. It also delays the
call to wacom_add_shared_data() to ensure correct devres release order.
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
[ ... ]
> @@ -2362,11 +2374,14 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
> }
>
> if (wacom_wac->shared->has_mute_touch_switch &&
> - wacom_wac->shared->touch_input) {
> - set_bit(EV_SW, wacom_wac->shared->touch_input->evbit);
> - input_set_capability(wacom_wac->shared->touch_input, EV_SW,
> - SW_MUTE_DEVICE);
> + rcu_access_pointer(wacom_wac->shared->touch_input)) {
> + struct input_dev *touch_input = rcu_dereference_protected(wacom_wac->shared->touch_input,
> + lockdep_is_held(&wacom_udev_list_lock));
> + set_bit(EV_SW, touch_input->evbit);
> + input_set_capability(touch_input, EV_SW, SW_MUTE_DEVICE);
> }
> +
> + mutex_unlock(&wacom_udev_list_lock);
> }
[Severity: High]
This is a pre-existing issue, but does this capability modification happen
too late?
In wacom_set_shared_values(), input capabilities are modified on the sibling
touch device. Because the Pen interface probes after the Touch interface,
this occurs on a sibling's touch_input device that was already fully probed
and registered.
The input subsystem requires that all capabilities must be fully initialized
before calling input_register_device(). Modifying capabilities
post-registration means the ADD uevent has already been emitted, and
userspace components miss the capability changes and might fail to process
events like SW_MUTE_DEVICE.
[ ... ]
> @@ -2442,10 +2457,6 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> goto fail;
> }
>
> - error = wacom_add_shared_data(hdev);
> - if (error)
> - goto fail;
> -
> error = wacom_setup_inputs(wacom);
> if (error)
> goto fail;
[Severity: Critical]
Does this change introduce a regression during device probe?
By removing wacom_add_shared_data() from this earlier location,
wacom_wac->shared is left uninitialized (NULL). However, wacom_setup_inputs()
is called immediately after:
wacom_setup_inputs()
wacom_setup_touch_input_capabilities()
if (wacom_wac->shared->touch->product == 0x361) {
Because wacom_wac->shared is unconditionally dereferenced for some devices
like INTUOSP2_BT and WACOM_27QHDT inside wacom_setup_touch_input_capabilities(),
could this cause a NULL pointer dereference during initialization?
[ ... ]
> @@ -2496,6 +2507,10 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
> }
> }
>
> + error = wacom_add_shared_data(hdev);
> + if (error)
> + goto fail_quirks;
> +
> wacom_set_shared_values(wacom_wac);
> devres_close_group(&hdev->dev, wacom);
[Severity: High]
Is there a race condition here since wacom_add_shared_data() is now called
at the very end of the probe function?
Before reaching this point, wacom_parse_and_register() has already called
hid_hw_start() and wacom_register_inputs(). hid_hw_start() enables incoming
HID reports and wacom_register_inputs() exposes the device to userspace.
If a hardware interrupt fires right after hid_hw_start(), the interrupt
handlers can execute while wacom_wac->shared is still NULL:
wacom_bpt_pen()
wacom->shared->stylus_in_proximity = true;
Could this cause an asynchronous NULL pointer dereference if a report
arrives before the shared data is allocated?
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
[ ... ]
> @@ -650,11 +650,18 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
> input_report_key(input, KEY_CONTROLPANEL, menu);
> input_report_key(input, KEY_INFO, info);
>
> - if (wacom->shared && wacom->shared->touch_input) {
> - input_report_switch(wacom->shared->touch_input,
> - SW_MUTE_DEVICE,
> - !wacom->shared->is_touch_on);
> - input_sync(wacom->shared->touch_input);
> + if (wacom->shared) {
> + struct input_dev *touch_input;
> +
> + rcu_read_lock();
> + touch_input = rcu_dereference(wacom->shared->touch_input);
> + if (touch_input) {
> + input_report_switch(touch_input,
> + SW_MUTE_DEVICE,
> + !wacom->shared->is_touch_on);
> + input_sync(touch_input);
> + }
> + rcu_read_unlock();
> }
[Severity: High]
This is a pre-existing issue, but did this patch miss a few other places
where touch_input is accessed locklessly?
This update successfully converts wacom_intuos_pad() to use RCU, but there
are other HID event handlers in wacom_wac.c that still directly dereference
wacom_wac->shared->touch_input without holding rcu_read_lock().
For example, in wacom_wac_pad_event():
if (wacom_wac->shared->touch_input) {
...
input_report_switch(wacom_wac->shared->touch_input, ...);
Similar direct accesses happen in wacom_wireless_irq() and
wacom_status_irq(). Because these readers do not establish an RCU read-side
critical section, synchronize_rcu() in wacom_remove_shared_data() will not
wait for them.
If the sibling touch device is unbound, could the input_dev be freed while
these handlers are still running, resulting in the same use-after-free issue?
Direct access to an __rcu pointer may also trigger Sparse warnings.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260601140845.1237227-1-lee@kernel.org?part=1
^ permalink raw reply
* [PATCH v2 2/2] HID: wacom: Redesign shared sibling data lifecycle
From: Lee Jones @ 2026-06-01 14:08 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Dmitry Torokhov, Peter Hutterer, linux-input, linux-kernel
In-Reply-To: <20260601140845.1237227-1-lee@kernel.org>
The Wacom driver coordinates state between sibling interfaces of the same
physical device using a shared structure 'wacom_shared' inside
'wacom_hdev_data'. The driver kept a volatile representative pointer
'data->dev' pointing to a sibling 'hid_device' for physical path comparisons
during sibling matching.
This pointer management is fragile. When the representative device is
disconnected, wacom_remove_shared_data() failed to clear/update
'data->dev', leading to a Use-After-Free vulnerability when subsequent
sibling probes dereference the dangling 'data->dev' pointer.
Resolve this issue by redesigning the sibling data lifecycle:
- Eliminate the volatile 'data->dev' representative pointer completely
- Redesign 'wacom_hdev_data' to store stable static copies of the required
attributes upon first allocation: 'phys' path string, 'vendor', 'product
IDs and the sibling's 'device_type'
- Use these static attributes for stable sibling matching in
wacom_are_sibling() and wacom_get_hdev_data()
This ensures sibling matching remains safe and stable even if individual
siblings are dynamically added or removed.
To secure the lifecycle against concurrent probe/disconnect races:
- Switch kref_put() to kref_put_mutex() in wacom_remove_shared_data() to
serialize refcount drops with list traversal and lookup
- Modify wacom_release_shared_data() to assume the list lock is already held
Also, do not accumulate the 'device_type' capability flag during subsequent
sibling probes. Keeping only the first probed sibling's device_type exactly
preserves the original sibling matching behavior without introducing side
effects.
Fixes: 4492efffffeb ("Input: wacom - share pen info with touch of the same ID")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
drivers/hid/wacom_sys.c | 63 +++++++++++++++++++++++++----------------
1 file changed, 39 insertions(+), 24 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 26fce6d3198a..7cd0b0ff3128 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -759,27 +759,40 @@ static void wacom_retrieve_hid_descriptor(struct hid_device *hdev,
struct wacom_hdev_data {
struct list_head list;
struct kref kref;
- struct hid_device *dev;
+ char phys[64];
+ __u32 vendor;
+ __u32 product;
+ __u32 device_type;
struct wacom_shared shared;
};
+static bool wacom_compare_device_paths(struct hid_device *hdev_a,
+ const char *phys_b, char separator)
+{
+ int n1 = strrchr(hdev_a->phys, separator) - hdev_a->phys;
+ int n2 = strrchr(phys_b, separator) - phys_b;
+
+ if (n1 != n2 || n1 <= 0 || n2 <= 0)
+ return false;
+
+ return !strncmp(hdev_a->phys, phys_b, n1);
+}
+
static LIST_HEAD(wacom_udev_list);
static DEFINE_MUTEX(wacom_udev_list_lock);
static bool wacom_are_sibling(struct hid_device *hdev,
- struct hid_device *sibling)
+ struct wacom_hdev_data *data)
{
struct wacom *wacom = hid_get_drvdata(hdev);
struct wacom_features *features = &wacom->wacom_wac.features;
- struct wacom *sibling_wacom = hid_get_drvdata(sibling);
- struct wacom_features *sibling_features = &sibling_wacom->wacom_wac.features;
__u32 oVid = features->oVid ? features->oVid : hdev->vendor;
__u32 oPid = features->oPid ? features->oPid : hdev->product;
/* The defined oVid/oPid must match that of the sibling */
- if (features->oVid != HID_ANY_ID && sibling->vendor != oVid)
+ if (features->oVid != HID_ANY_ID && data->vendor != oVid)
return false;
- if (features->oPid != HID_ANY_ID && sibling->product != oPid)
+ if (features->oPid != HID_ANY_ID && data->product != oPid)
return false;
/*
@@ -787,11 +800,11 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* device path, while those with different VID/PID must share
* the same physical parent device path.
*/
- if (hdev->vendor == sibling->vendor && hdev->product == sibling->product) {
- if (!hid_compare_device_paths(hdev, sibling, '/'))
+ if (hdev->vendor == data->vendor && hdev->product == data->product) {
+ if (!wacom_compare_device_paths(hdev, data->phys, '/'))
return false;
} else {
- if (!hid_compare_device_paths(hdev, sibling, '.'))
+ if (!wacom_compare_device_paths(hdev, data->phys, '.'))
return false;
}
@@ -804,7 +817,7 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* devices.
*/
if ((features->device_type & WACOM_DEVICETYPE_DIRECT) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_DIRECT))
+ !(data->device_type & WACOM_DEVICETYPE_DIRECT))
return false;
/*
@@ -812,17 +825,17 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* devices.
*/
if (!(features->device_type & WACOM_DEVICETYPE_DIRECT) &&
- (sibling_features->device_type & WACOM_DEVICETYPE_DIRECT))
+ (data->device_type & WACOM_DEVICETYPE_DIRECT))
return false;
/* Pen devices may only be siblings of touch devices */
if ((features->device_type & WACOM_DEVICETYPE_PEN) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_TOUCH))
+ !(data->device_type & WACOM_DEVICETYPE_TOUCH))
return false;
/* Touch devices may only be siblings of pen devices */
if ((features->device_type & WACOM_DEVICETYPE_TOUCH) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_PEN))
+ !(data->device_type & WACOM_DEVICETYPE_PEN))
return false;
/*
@@ -838,7 +851,7 @@ static struct wacom_hdev_data *wacom_get_hdev_data(struct hid_device *hdev)
/* Try to find an already-probed interface from the same device */
list_for_each_entry(data, &wacom_udev_list, list) {
- if (hid_compare_device_paths(hdev, data->dev, '/')) {
+ if (wacom_compare_device_paths(hdev, data->phys, '/')) {
kref_get(&data->kref);
return data;
}
@@ -846,7 +859,7 @@ static struct wacom_hdev_data *wacom_get_hdev_data(struct hid_device *hdev)
/* Fallback to finding devices that appear to be "siblings" */
list_for_each_entry(data, &wacom_udev_list, list) {
- if (wacom_are_sibling(hdev, data->dev)) {
+ if (wacom_are_sibling(hdev, data)) {
kref_get(&data->kref);
return data;
}
@@ -860,35 +873,34 @@ static void wacom_release_shared_data(struct kref *kref)
struct wacom_hdev_data *data =
container_of(kref, struct wacom_hdev_data, kref);
- mutex_lock(&wacom_udev_list_lock);
list_del(&data->list);
- mutex_unlock(&wacom_udev_list_lock);
-
kfree(data);
}
static void wacom_remove_shared_data(void *res)
{
- struct wacom *wacom = res;
+ struct wacom *res_wacom = res;
struct wacom_hdev_data *data;
- struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+ struct wacom_wac *wacom_wac = &res_wacom->wacom_wac;
if (wacom_wac->shared) {
data = container_of(wacom_wac->shared, struct wacom_hdev_data,
shared);
mutex_lock(&wacom_udev_list_lock);
- if (wacom_wac->shared->touch == wacom->hdev) {
+ if (wacom_wac->shared->touch == res_wacom->hdev) {
wacom_wac->shared->touch = NULL;
rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
- } else if (wacom_wac->shared->pen == wacom->hdev) {
+ } else if (wacom_wac->shared->pen == res_wacom->hdev) {
wacom_wac->shared->pen = NULL;
}
mutex_unlock(&wacom_udev_list_lock);
synchronize_rcu();
- kref_put(&data->kref, wacom_release_shared_data);
+ if (kref_put_mutex(&data->kref, wacom_release_shared_data, &wacom_udev_list_lock))
+ mutex_unlock(&wacom_udev_list_lock);
+
wacom_wac->shared = NULL;
}
}
@@ -911,7 +923,10 @@ static int wacom_add_shared_data(struct hid_device *hdev)
}
kref_init(&data->kref);
- data->dev = hdev;
+ strncpy(data->phys, hdev->phys, sizeof(data->phys) - 1);
+ data->vendor = hdev->vendor;
+ data->product = hdev->product;
+ data->device_type = wacom_wac->features.device_type;
list_add_tail(&data->list, &wacom_udev_list);
}
--
2.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply related
* [PATCH v2 1/2] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: Lee Jones @ 2026-06-01 14:08 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Dmitry Torokhov, Peter Hutterer, linux-input, linux-kernel
wacom_intuos_pad() accesses wacom->shared->touch_input locklessly inside the
interrupt handler context. If the Touch sibling device is disconnected,
wacom_remove_shared_data() clears 'touch_input' outside any lock, creating a
Time-of-Check to Time-of-Use (TOCTOU) race condition where a preempted reader in
interrupt context dereferences the freed pointer, leading to a Use-After-Free.
Resolve this by introducing RCU protection for the touch_input pointer:
- Annotate 'touch_input' in wacom_shared struct with __rcu
- Wrap lockless readers in wacom_wac.c with rcu_read_lock() and rcu_dereference()
- Update writers in wacom_sys.c using rcu_assign_pointer()
- Call synchronize_rcu() in wacom_remove_shared_data() to ensure all active RCU
readers have finished before the input device is freed
To ensure the devres release order is correct and wacom_remove_shared_data()
completes its RCU synchronization BEFORE the input device is freed, move the
wacom_add_shared_data() call to the very end of wacom_parse_and_register().
Since devres actions are executed in LIFO order, this guarantees the shared
reference is cleared and synchronized before the input device resource is destroyed.
Also wrap wacom_set_shared_values() and touch/pen assignments in
wacom_add_shared_data() inside the wacom_udev_list_lock to serialize concurrent
probe assignments, and verify that 'shared->touch == hdev' before setting
touch_input to prevent concurrent sibling probe state desynchronization.
Fixes: 961794a00eab ("Input: wacom - add reporting of SW_MUTE_DEVICE events")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
drivers/hid/wacom_sys.c | 49 +++++++++++++++++++++++++++--------------
drivers/hid/wacom_wac.c | 17 +++++++++-----
drivers/hid/wacom_wac.h | 2 +-
3 files changed, 45 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 2220168bf116..26fce6d3198a 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -877,10 +877,16 @@ static void wacom_remove_shared_data(void *res)
data = container_of(wacom_wac->shared, struct wacom_hdev_data,
shared);
- if (wacom_wac->shared->touch == wacom->hdev)
+ mutex_lock(&wacom_udev_list_lock);
+ if (wacom_wac->shared->touch == wacom->hdev) {
wacom_wac->shared->touch = NULL;
- else if (wacom_wac->shared->pen == wacom->hdev)
+ rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
+ } else if (wacom_wac->shared->pen == wacom->hdev) {
wacom_wac->shared->pen = NULL;
+ }
+ mutex_unlock(&wacom_udev_list_lock);
+
+ synchronize_rcu();
kref_put(&data->kref, wacom_release_shared_data);
wacom_wac->shared = NULL;
@@ -909,6 +915,11 @@ static int wacom_add_shared_data(struct hid_device *hdev)
list_add_tail(&data->list, &wacom_udev_list);
}
+ if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
+ data->shared.touch = hdev;
+ else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
+ data->shared.pen = hdev;
+
mutex_unlock(&wacom_udev_list_lock);
wacom_wac->shared = &data->shared;
@@ -917,11 +928,6 @@ static int wacom_add_shared_data(struct hid_device *hdev)
if (retval)
return retval;
- if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
- wacom_wac->shared->touch = hdev;
- else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
- wacom_wac->shared->pen = hdev;
-
return retval;
}
@@ -2345,9 +2351,15 @@ static void wacom_release_resources(struct wacom *wacom)
static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
{
+ struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
+
+ mutex_lock(&wacom_udev_list_lock);
+
if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
- wacom_wac->shared->type = wacom_wac->features.type;
- wacom_wac->shared->touch_input = wacom_wac->touch_input;
+ if (wacom_wac->shared->touch == wacom->hdev) {
+ wacom_wac->shared->type = wacom_wac->features.type;
+ rcu_assign_pointer(wacom_wac->shared->touch_input, wacom_wac->touch_input);
+ }
}
if (wacom_wac->has_mute_touch_switch) {
@@ -2362,11 +2374,14 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
}
if (wacom_wac->shared->has_mute_touch_switch &&
- wacom_wac->shared->touch_input) {
- set_bit(EV_SW, wacom_wac->shared->touch_input->evbit);
- input_set_capability(wacom_wac->shared->touch_input, EV_SW,
- SW_MUTE_DEVICE);
+ rcu_access_pointer(wacom_wac->shared->touch_input)) {
+ struct input_dev *touch_input = rcu_dereference_protected(wacom_wac->shared->touch_input,
+ lockdep_is_held(&wacom_udev_list_lock));
+ set_bit(EV_SW, touch_input->evbit);
+ input_set_capability(touch_input, EV_SW, SW_MUTE_DEVICE);
}
+
+ mutex_unlock(&wacom_udev_list_lock);
}
static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
@@ -2442,10 +2457,6 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
goto fail;
}
- error = wacom_add_shared_data(hdev);
- if (error)
- goto fail;
-
error = wacom_setup_inputs(wacom);
if (error)
goto fail;
@@ -2496,6 +2507,10 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
}
}
+ error = wacom_add_shared_data(hdev);
+ if (error)
+ goto fail_quirks;
+
wacom_set_shared_values(wacom_wac);
devres_close_group(&hdev->dev, wacom);
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85625..c7cf1aaab903 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -650,11 +650,18 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
input_report_key(input, KEY_CONTROLPANEL, menu);
input_report_key(input, KEY_INFO, info);
- if (wacom->shared && wacom->shared->touch_input) {
- input_report_switch(wacom->shared->touch_input,
- SW_MUTE_DEVICE,
- !wacom->shared->is_touch_on);
- input_sync(wacom->shared->touch_input);
+ if (wacom->shared) {
+ struct input_dev *touch_input;
+
+ rcu_read_lock();
+ touch_input = rcu_dereference(wacom->shared->touch_input);
+ if (touch_input) {
+ input_report_switch(touch_input,
+ SW_MUTE_DEVICE,
+ !wacom->shared->is_touch_on);
+ input_sync(touch_input);
+ }
+ rcu_read_unlock();
}
input_report_abs(input, ABS_RX, strip1);
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 126bec6e5c0c..a8bbba4a6f37 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -285,7 +285,7 @@ struct wacom_shared {
/* for wireless device to access USB interfaces */
unsigned touch_max;
int type;
- struct input_dev *touch_input;
+ struct input_dev __rcu *touch_input;
struct hid_device *pen;
struct hid_device *touch;
bool has_mute_touch_switch;
--
2.54.0.823.g6e5bcc1fc9-goog
^ permalink raw reply related
* [PATCH v2 2/2] HID: wacom: use cleanup.h for wacom_wac_queue_flush() buffer management
From: Jinmo Yang @ 2026-06-01 13:41 UTC (permalink / raw)
To: linux-input, dmitry.torokhov
Cc: jikos, benjamin.tissoires, stable, jinmo44.yang
In-Reply-To: <20260601134124.1560886-1-jinmo44.yang@gmail.com>
Use __free(kfree) cleanup facility for the temporary buffer in
wacom_wac_queue_flush() to simplify error paths and ensure the buffer
is freed automatically when it goes out of scope.
Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
---
drivers/hid/wacom_sys.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 2e237bdd2..edc24fe2e 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -70,11 +70,10 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
{
while (!kfifo_is_empty(fifo)) {
int size = kfifo_peek_len(fifo);
- u8 *buf;
+ u8 *buf __free(kfree) = kzalloc(size, GFP_ATOMIC);
unsigned int count;
int err;
- buf = kzalloc(size, GFP_ATOMIC);
if (!buf) {
kfifo_skip(fifo);
continue;
@@ -87,7 +86,6 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
// to flush seems reasonable enough, however.
hid_warn(hdev, "%s: removed fifo entry with unexpected size\n",
__func__);
- kfree(buf);
continue;
}
err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, size, false);
@@ -95,8 +93,6 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
hid_warn(hdev, "%s: unable to flush event due to error %d\n",
__func__, err);
}
-
- kfree(buf);
}
}
--
2.53.0
^ permalink raw reply related
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