* Re: [PATCH 08/10] [v6 net-next] p54spi: convert to devicetree
From: sashiko-bot @ 2026-05-30 0:45 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260520183815.2510387-9-arnd@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] SPI transfers are using stack-allocated buffers for `tx_buf` and `rx_buf`, violating the SPI subsystem's DMA requirements.
--
commit 8ba2b2da7516b867a52706d993b9ce2d49c99105
Author: Arnd Bergmann <arnd@arndb.de>
p54spi: convert to devicetree
The Prism54 SPI driver hardcodes GPIO numbers and expects users to
pass them as module parameters, apparently a relic from its life as a
staging driver. This works because there is only one user, the Nokia
N8x0 tablet.
Convert this to the gpio descriptor interface and DT based probing
to improve this and simplify the code at the same time.
> diff --git a/drivers/net/wireless/intersil/p54/p54spi.c b/drivers/net/wireless/intersil/p54/p54spi.c
> index 9d66dcae54e02..d18be25450285 100644
> --- a/drivers/net/wireless/intersil/p54/p54spi.c
> +++ b/drivers/net/wireless/intersil/p54/p54spi.c
[ ... ]
> @@ -30,19 +31,6 @@
> MODULE_FIRMWARE("3826.arm");
> MODULE_FIRMWARE("3826.eeprom");
>
> -/* gpios should be handled in board files and provided via platform data,
> - * but because it's currently impossible for p54spi to have a header file
> - * in include/linux, let's use module parameters for now
> - */
> -
> -static int p54spi_gpio_power = 97;
> -module_param(p54spi_gpio_power, int, 0444);
> -MODULE_PARM_DESC(p54spi_gpio_power, "gpio number for power line");
> -
> -static int p54spi_gpio_irq = 87;
> -module_param(p54spi_gpio_irq, int, 0444);
> -MODULE_PARM_DESC(p54spi_gpio_irq, "gpio number for irq line");
> -
> static void p54spi_spi_read(struct p54s_priv *priv, u8 address,
> void *buf, size_t len)
> {
[Severity: High]
This isn't a bug introduced by this patch, but does this code use
stack-allocated buffers for SPI transfers?
It appears that p54spi_spi_read() and other similar functions pass stack
variables (e.g., addr in p54spi_spi_read(), rx_head in p54spi_rx(), and
val in p54spi_read32()) directly into spi_sync() as tx_buf and rx_buf.
If the underlying SPI controller uses DMA, the SPI core maps these buffers.
Using stack memory for DMA is unsafe because, with options like
CONFIG_VMAP_STACK enabled, the stack may not be physically contiguous.
Even without it, stack DMA leads to cache-line sharing and incoherency,
which can result in severe memory corruption or kernel panics.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520183815.2510387-1-arnd@kernel.org?part=8
^ permalink raw reply
* Re: [PATCH 10/10] gpiolib: turn off legacy interface by default
From: sashiko-bot @ 2026-05-30 0:45 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260520183815.2510387-11-arnd@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Turning off `CONFIG_GPIOLIB_LEGACY` by default will break the build for multiple architectures and platforms that still rely on the legacy GPIO interface but do not select `CONFIG_GPIOLIB_LEGACY`.
--
commit a920fbf61a1d127c3a98b41cb32f7260acd5657b
Author: Arnd Bergmann <arnd@arndb.de>
gpiolib: turn off legacy interface by default
This commit removes the default enablement of GPIOLIB_LEGACY, requiring
platforms to explicitly select it if they still use the legacy interface.
It also adds a prompt to enable it manually for compile testing.
> All users of the legacy interface now select CONFIG_GPIOLIB_LEGACY,
> so it can be turned off by default and only get built on platforms
> that still have one unconverted driver.
[Severity: Low]
Is this statement accurate? There appear to be platforms like ARM PXA
(arch/arm/mach-pxa/spitz_pm.c), ARM Orion5x, and m68k Coldfire that still
contain direct calls to legacy GPIO functions like gpio_request() but do
not select GPIOLIB_LEGACY in their Kconfig files.
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 020e51e30317a..543de15a12f63 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -4,7 +4,14 @@
> #
>
> config GPIOLIB_LEGACY
> - def_bool y
[Severity: Low]
Will removing this default cause build regressions for the unconverted
platforms mentioned above?
It seems the legacy function declarations in include/linux/gpio.h will be
hidden, leading to implicit declaration errors when compiling those
architectures.
> + bool "Legacy GPIO interfaces" if COMPILE_TEST
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520183815.2510387-1-arnd@kernel.org?part=10
^ permalink raw reply
* Re: [PATCH 02/10] [v3] input: gpio-keys: make legacy gpiolib optional
From: sashiko-bot @ 2026-05-30 0:45 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260520183815.2510387-3-arnd@kernel.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Modifying the legacy GPIO validation to check `!button->irq` breaks legacy devices that legitimately provide both a GPIO and an IRQ.
- [Medium] Conditionally removing the `gpio` field from `struct gpio_keys_button` causes build failures in drivers that assign to this field when `CONFIG_GPIOLIB_LEGACY` is disabled.
--
commit cb399daff5ceabcf54d5ea611fd309f5c0600ab4
Author: Arnd Bergmann <arnd@arndb.de>
input: gpio-keys: make legacy gpiolib optional
This commit makes the legacy gpiolib interface optional for gpio-keys and
gpio-keys-polled by adding ifdef blocks. It also modifies the gpio-keys
driver to ignore the gpio number if an IRQ is passed to keep Rohm PMIC
drivers working with and without CONFIG_GPIOLIB_LEGACY.
Link: https://lore.kernel.org/all/b3c94552-c104-42e3-be15-7e8362e8039e@gmail.com/
Link: https://lore.kernel.org/all/afJXG4_rtaj3l2Dk@google.com/
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
[ ... ]
> @@ -528,7 +528,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
> */
> bdata->gpiod = NULL;
> }
> - } else if (gpio_is_valid(button->gpio)) {
> +#ifdef CONFIG_GPIOLIB_LEGACY
> + } else if (!button->irq && gpio_is_valid(button->gpio)) {
[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.
[ ... ]
> diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
> --- a/include/linux/gpio_keys.h
> +++ b/include/linux/gpio_keys.h
[ ... ]
> @@ -25,7 +25,9 @@ struct device;
> */
> 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?
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.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520183815.2510387-1-arnd@kernel.org?part=2
^ permalink raw reply
* Re: [PATCH 01/10] [v2] [net-next] net: dsa: b53: hide legacy gpiolib usage on non-mips
From: sashiko-bot @ 2026-05-30 0:45 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260520183815.2510387-2-arnd@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Probe errors from devm_gpio_request_one() are silently ignored, failing to propagate critical errors such as -EPROBE_DEFER.
--
commit 46ca85c6cb9da93cad093c8eb342afc0ea94be33
Author: Arnd Bergmann <arnd@arndb.de>
net: dsa: b53: hide legacy gpiolib usage on non-mips
This commit moves legacy gpiolib usage inside an #ifdef block and transitions
the common parts of the driver to use modern GPIO descriptors. This allows
building the driver even when gpio_set_value is left out of the kernel.
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index 2c4131ed7e30b..d838511ae6744 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
[ ... ]
> @@ -3173,12 +3172,6 @@ static int b53_switch_init(struct b53_device *dev)
> return -ENOMEM;
>
> dev->reset_gpio = b53_switch_get_reset_gpio(dev);
> - if (dev->reset_gpio >= 0) {
> - ret = devm_gpio_request_one(dev->dev, dev->reset_gpio,
> - GPIOF_OUT_INIT_HIGH, "robo_reset");
> - if (ret)
> - return ret;
> - }
>
> return 0;
> }
[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).
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520183815.2510387-1-arnd@kernel.org?part=1
^ permalink raw reply
* Re: [PATCH 4/4] dt-bindings: input: remove obsolete matrix-keymap.txt
From: sashiko-bot @ 2026-05-30 5:23 UTC (permalink / raw)
To: Akash Sukhavasi; +Cc: dmitry.torokhov, robh, linux-input, devicetree, conor+dt
In-Reply-To: <20260529052246.4934-5-akash.sukhavasi@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The commit message inaccurately claims that 'Eight files still reference matrix-keymap.txt', when only 6 files actually do.
--
commit eb5de2cbf16470ef87ea36fcd4695758a1c30a9f
Author: Akash Sukhavasi <akash.sukhavasi@gmail.com>
dt-bindings: input: remove obsolete matrix-keymap.txt
This commit removes the obsolete matrix-keymap.txt stub and updates all
remaining references across the input and mfd binding documentation to
point directly to the matrix-keymap.yaml schema.
> Eight files still reference matrix-keymap.txt, forcing readers
> through an extra hop to reach the .yaml. The stub has not been
> touched since June 2020. Update all references across input and mfd
> binding documentation to point directly to matrix-keymap.yaml and
> remove the stub.
[Severity: Low]
This isn't a bug, but the commit message states that eight files still
reference the text file. Looking at the patch, there are eight total
references updated, but they are spread across only six files since
tc3589x.txt contains three of those references.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260529052246.4934-1-akash.sukhavasi@gmail.com?part=4
^ permalink raw reply
* [PATCH] Input: ims-pcu - only expose sysfs attributes on control interface
From: Dmitry Torokhov @ 2026-05-30 5:40 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel
When the driver was converted to use the driver core to instantiate device
attributes (via .dev_groups in the usb_driver structure), the attributes
started appearing on all interfaces bound to the driver. Since the ims-pcu
driver manually claims the secondary data interface during probe, the
driver core automatically creates the sysfs attributes for that interface
as well.
However, the driver only supports these attributes on the primary control
interface. Data interfaces lack the necessary descriptors and internal
state to handle these requests, and accessing them can lead to unexpected
behavior or crashes.
Fix this by updating the is_visible() callbacks for both the main and OFN
attribute groups to verify that the interface being accessed is indeed the
control interface.
Fixes: 204d18a7a0c6 ("Input: ims-pcu - use driver core to instantiate device attributes")
Cc: stable@vger.kernel.org
Reported-by: Sashiko bot <sashiko-bot@kernel.org>
Assisted-by: Gemini:gemini-3.1-pro
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/ims-pcu.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
index 13367ec66936..ddfb88b7c8f5 100644
--- a/drivers/input/misc/ims-pcu.c
+++ b/drivers/input/misc/ims-pcu.c
@@ -1283,6 +1283,9 @@ static umode_t ims_pcu_is_attr_visible(struct kobject *kobj,
struct ims_pcu *pcu = usb_get_intfdata(intf);
umode_t mode = attr->mode;
+ if (intf != pcu->ctrl_intf)
+ return 0;
+
if (pcu->bootloader_mode) {
if (attr != &dev_attr_update_firmware_status.attr &&
attr != &dev_attr_update_firmware.attr &&
@@ -1534,6 +1537,9 @@ static umode_t ims_pcu_ofn_is_attr_visible(struct kobject *kobj,
struct ims_pcu *pcu = usb_get_intfdata(intf);
umode_t mode = attr->mode;
+ if (intf != pcu->ctrl_intf)
+ return 0;
+
/*
* PCU-B devices, both GEN_1 and GEN_2 do not have OFN sensor.
*/
--
2.54.0.823.g6e5bcc1fc9-goog
--
Dmitry
^ permalink raw reply related
* [PATCH] Input: atlas_btns - modernize the driver
From: Dmitry Torokhov @ 2026-05-30 5:52 UTC (permalink / raw)
To: linux-input; +Cc: Rafael J. Wysocki, linux-kernel
Rework the driver to use modern style:
- Remove global state by introducing a per-device structure
- Use devm for resource management (input device allocation)
- Use dev_err_probe() for error reporting in the probe path
- Clean up unused definitions and headers.
Assisted-by: Gemini:gemini-3.1-pro
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/atlas_btns.c | 109 ++++++++++++++++----------------
1 file changed, 55 insertions(+), 54 deletions(-)
diff --git a/drivers/input/misc/atlas_btns.c b/drivers/input/misc/atlas_btns.c
index 835ad45a9d65..5805a1a3bb05 100644
--- a/drivers/input/misc/atlas_btns.c
+++ b/drivers/input/misc/atlas_btns.c
@@ -15,17 +15,18 @@
#include <linux/types.h>
#include <linux/acpi.h>
#include <linux/platform_device.h>
-#include <linux/uaccess.h>
#define ACPI_ATLAS_NAME "Atlas ACPI"
-#define ACPI_ATLAS_CLASS "Atlas"
-static unsigned short atlas_keymap[16];
-static struct input_dev *input_dev;
+struct atlas_btns {
+ struct input_dev *input_dev;
+ unsigned short keymap[16];
+};
/* button handling code */
static acpi_status acpi_atlas_button_setup(acpi_handle region_handle,
- u32 function, void *handler_context, void **return_context)
+ u32 function, void *handler_context,
+ void **return_context)
{
*return_context =
(function != ACPI_REGION_DEACTIVATE) ? handler_context : NULL;
@@ -34,33 +35,37 @@ static acpi_status acpi_atlas_button_setup(acpi_handle region_handle,
}
static acpi_status acpi_atlas_button_handler(u32 function,
- acpi_physical_address address,
- u32 bit_width, u64 *value,
- void *handler_context, void *region_context)
+ acpi_physical_address address,
+ u32 bit_width, u64 *value,
+ void *handler_context,
+ void *region_context)
{
- acpi_status status;
+ struct atlas_btns *atlas = region_context;
if (function == ACPI_WRITE) {
int code = address & 0x0f;
int key_down = !(address & 0x10);
- input_event(input_dev, EV_MSC, MSC_SCAN, code);
- input_report_key(input_dev, atlas_keymap[code], key_down);
- input_sync(input_dev);
+ input_event(atlas->input_dev, EV_MSC, MSC_SCAN, code);
+ input_report_key(atlas->input_dev, atlas->keymap[code],
+ key_down);
+ input_sync(atlas->input_dev);
- status = AE_OK;
- } else {
- pr_warn("shrugged on unexpected function: function=%x,address=%lx,value=%x\n",
- function, (unsigned long)address, (u32)*value);
- status = AE_BAD_PARAMETER;
+ return AE_OK;
}
- return status;
+ dev_warn(atlas->input_dev->dev.parent,
+ "unexpected function: function=%x,address=%lx,value=%x\n",
+ function, (unsigned long)address, (u32)*value);
+
+ return AE_BAD_PARAMETER;
}
static int atlas_acpi_button_probe(struct platform_device *pdev)
{
struct acpi_device *device;
+ struct atlas_btns *atlas;
+ struct input_dev *input_dev;
acpi_status status;
int i;
int err;
@@ -69,65 +74,62 @@ static int atlas_acpi_button_probe(struct platform_device *pdev)
if (!device)
return -ENODEV;
- input_dev = input_allocate_device();
- if (!input_dev) {
- pr_err("unable to allocate input device\n");
+ atlas = devm_kzalloc(&pdev->dev, sizeof(*atlas), GFP_KERNEL);
+ if (!atlas)
return -ENOMEM;
- }
+
+ input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!input_dev)
+ return -ENOMEM;
+
+ atlas->input_dev = input_dev;
input_dev->name = "Atlas ACPI button driver";
input_dev->phys = "ASIM0000/atlas/input0";
input_dev->id.bustype = BUS_HOST;
- input_dev->keycode = atlas_keymap;
- input_dev->keycodesize = sizeof(unsigned short);
- input_dev->keycodemax = ARRAY_SIZE(atlas_keymap);
+ input_dev->keycode = atlas->keymap;
+ input_dev->keycodesize = sizeof(atlas->keymap[0]);
+ input_dev->keycodemax = ARRAY_SIZE(atlas->keymap);
input_set_capability(input_dev, EV_MSC, MSC_SCAN);
- __set_bit(EV_KEY, input_dev->evbit);
- for (i = 0; i < ARRAY_SIZE(atlas_keymap); i++) {
+ for (i = 0; i < ARRAY_SIZE(atlas->keymap); i++) {
if (i < 9) {
- atlas_keymap[i] = KEY_F1 + i;
- __set_bit(KEY_F1 + i, input_dev->keybit);
- } else
- atlas_keymap[i] = KEY_RESERVED;
+ atlas->keymap[i] = KEY_F1 + i;
+ input_set_capability(input_dev, EV_KEY, KEY_F1 + i);
+ } else {
+ atlas->keymap[i] = KEY_RESERVED;
+ }
}
err = input_register_device(input_dev);
- if (err) {
- pr_err("couldn't register input device\n");
- input_free_device(input_dev);
- return err;
- }
+ if (err)
+ return dev_err_probe(&pdev->dev, err,
+ "couldn't register input device\n");
/* hookup button handler */
status = acpi_install_address_space_handler(device->handle,
- 0x81, &acpi_atlas_button_handler,
- &acpi_atlas_button_setup, device);
- if (ACPI_FAILURE(status)) {
- pr_err("error installing addr spc handler\n");
- input_unregister_device(input_dev);
- err = -EINVAL;
- }
+ 0x81,
+ &acpi_atlas_button_handler,
+ &acpi_atlas_button_setup,
+ atlas);
+ if (ACPI_FAILURE(status))
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "error installing addr spc handler\n");
- return err;
+ return 0;
}
static void atlas_acpi_button_remove(struct platform_device *pdev)
{
struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
- acpi_status status;
- status = acpi_remove_address_space_handler(device->handle,
- 0x81, &acpi_atlas_button_handler);
- if (ACPI_FAILURE(status))
- pr_err("error removing addr spc handler\n");
-
- input_unregister_device(input_dev);
+ acpi_remove_address_space_handler(device->handle, 0x81,
+ &acpi_atlas_button_handler);
}
static const struct acpi_device_id atlas_device_ids[] = {
- {"ASIM0000", 0},
- {"", 0},
+ { "ASIM0000" },
+ { "" }
};
MODULE_DEVICE_TABLE(acpi, atlas_device_ids);
@@ -144,4 +146,3 @@ module_platform_driver(atlas_acpi_driver);
MODULE_AUTHOR("Jaya Kumar");
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("Atlas button driver");
-
--
2.54.0.823.g6e5bcc1fc9-goog
--
Dmitry
^ permalink raw reply related
* Re: [PATCH] Input: ims-pcu - only expose sysfs attributes on control interface
From: sashiko-bot @ 2026-05-30 5:53 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <ahp23lj4_vXIeUBl@google.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [Low] The `ims_pcu_ofn_is_attr_visible()` function introduced in the patch returns `0` when `intf != pcu->ctrl_intf` for the `ofn` attribute group. Because `ofn` is a named attribute group, returning `0` suppresses the individual files but still creates an empty 'ofn' directory on the data interface.
Pre-existing issues:
- [High] The `buttons->input` pointer is assigned after `input_register_device()`, and there is no memory barrier before setting `pcu->setup_complete = true`. On weakly ordered architectures, the CPU can reorder these stores, leading to a NULL pointer dereference in the interrupt handler (`ims_pcu_handle_response`) if an event arrives and observes `setup_complete == true` before `buttons->input` is initialized.
- [High] The driver claims the secondary data interface (`pcu->data_intf`) in `ims_pcu_probe` but fails to release it in `ims_pcu_disconnect` when unbinding the control interface. The data interface remains bound with its `intfdata` pointing to the freed `pcu` structure. Subsequent sysfs accesses on the data interface will dereference this freed pointer, resulting in a use-after-free (UAF).
- [High] A malicious USB device can provide an incorrect master interface number in the CDC Union descriptor. `ims_pcu_parse_cdc_data()` uses this to set `pcu->ctrl_intf` and assigns `intfdata` to it. If this differs from the interface currently being probed, the probed interface will have `NULL` driver data. When the sysfs core subsequently calls `ims_pcu_is_attr_visible()`, `usb_get_intfdata()` returns `NULL`, causing a NULL pointer dereference when accessing `pcu->ctrl_intf`.
- [High] The sysfs attribute store function `ims_pcu_reset_device` calls `ims_pcu_execute_command()` without acquiring `pcu->cmd_mutex`. This allows concurrent sysfs operations (e.g., setting backlight brightness or updating firmware) to interleave their command preparations, causing data corruption in the shared `pcu->urb_out_buf` and race conditions on the `pcu->cmd_done` completion structure.
- [High] In `ims_pcu_buffers_free()`, the input URB DMA buffer (`pcu->urb_in_buf`) is incorrectly freed using `pcu->max_out_size` instead of the `pcu->max_in_size` used during allocation.
--
commit e7e143a0135bed02c30cf00e17a9a25adf1ab6c4
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Subject: Input: ims-pcu - only expose sysfs attributes on control interface
When the driver was converted to use the driver core to instantiate device
attributes, they started appearing on all interfaces bound to the driver.
This patch updates the is_visible callbacks to verify that the interface
being accessed is indeed the control interface.
> diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
> index 4c022a36dbe84..7c76bc82ab703 100644
> --- a/drivers/input/misc/ims-pcu.c
> +++ b/drivers/input/misc/ims-pcu.c
> @@ -1249,6 +1249,9 @@ static umode_t ims_pcu_is_attr_visible(struct kobject *kobj,
> struct ims_pcu *pcu = usb_get_intfdata(intf);
> umode_t mode = attr->mode;
>
> + if (intf != pcu->ctrl_intf)
> + return 0;
[Severity: High]
This isn't a bug introduced by this patch, but could pcu be NULL here?
If a malicious USB device provides an incorrect master interface number in the
CDC Union descriptor, ims_pcu_parse_cdc_data() might set pcu->ctrl_intf to
a different interface than the one being probed. When the sysfs core
subsequently calls ims_pcu_is_attr_visible(), usb_get_intfdata() would
return NULL, leading to a NULL pointer dereference when accessing
pcu->ctrl_intf.
[ ... ]
> @@ -1488,6 +1491,9 @@ static umode_t ims_pcu_ofn_is_attr_visible(struct kobject *kobj,
> struct ims_pcu *pcu = usb_get_intfdata(intf);
> umode_t mode = attr->mode;
>
> + if (intf != pcu->ctrl_intf)
> + return 0;
[Severity: Low]
Since ofn is a named attribute group, does returning 0 here suppress the
individual files but still leave an empty ofn directory on the data interface?
According to sysfs guidelines, should this return SYSFS_GROUP_INVISIBLE
instead to fully suppress the group directory?
I also noticed a few pre-existing issues in the surrounding code:
drivers/input/misc/ims-pcu.c:ims_pcu_init_application_mode() {
...
if (info->has_gamepad) {
error = ims_pcu_setup_gamepad(pcu);
if (error)
goto err_destroy_buttons;
}
pcu->setup_complete = true;
return 0;
}
[Severity: High]
This is a pre-existing issue, but should there be a memory barrier before
setting pcu->setup_complete = true?
On weakly ordered architectures, could the CPU reorder these stores? If an
event arrives and ims_pcu_handle_response() observes setup_complete == true
before buttons->input is fully initialized, it might lead to a NULL pointer
dereference in the interrupt handler.
drivers/input/misc/ims-pcu.c:ims_pcu_disconnect() {
...
/*
* See if we are dealing with control or data interface. The cleanup
* happens when we unbind primary (control) interface.
*/
if (alt->desc.bInterfaceClass != USB_CLASS_COMM)
return;
ims_pcu_stop_io(pcu);
...
ims_pcu_buffers_free(pcu);
kfree(pcu);
}
[Severity: High]
This isn't a bug introduced by this patch, but does this fail to release the
claimed data interface?
If the driver claims the secondary data interface (pcu->data_intf) in
ims_pcu_probe(), failing to release it via usb_driver_release_interface()
when unbinding the control interface results in an asymmetric teardown.
The data interface remains bound with its intfdata pointing to the freed
pcu structure, and reading its sysfs attributes could trigger a use-after-free.
drivers/input/misc/ims-pcu.c:ims_pcu_reset_device() {
...
dev_info(pcu->dev, "Attempting to reset device\n");
error = ims_pcu_execute_command(pcu, PCU_RESET, &reset_byte, 1);
if (error) {
...
}
[Severity: High]
This is a pre-existing issue, but shouldn't ims_pcu_reset_device() acquire
pcu->cmd_mutex before calling ims_pcu_execute_command()?
Without acquiring the lock, concurrent sysfs operations (e.g., setting
backlight brightness or updating firmware) could interleave their command
preparations. This could cause data corruption in the shared
pcu->urb_out_buf and race conditions on the pcu->cmd_done completion
structure.
drivers/input/misc/ims-pcu.c:ims_pcu_buffers_free() {
...
usb_kill_urb(pcu->urb_in);
usb_free_urb(pcu->urb_in);
usb_free_coherent(pcu->udev, pcu->max_out_size,
pcu->urb_in_buf, pcu->read_dma);
...
}
[Severity: High]
This isn't a bug introduced by this patch, but is pcu->urb_in_buf being freed
with the wrong size?
It appears the input URB DMA buffer is incorrectly freed using
pcu->max_out_size instead of the pcu->max_in_size that was used during
allocation. Depending on the DMA allocator, this could lead to memory
corruption or leaks.
--
Sashiko AI review · https://sashiko.dev/#/patchset/ahp23lj4_vXIeUBl@google.com?part=1
^ permalink raw reply
* Re: [PATCH v1] Input: Use named initializers for arrays of i2c_device_data
From: Dmitry Torokhov @ 2026-05-30 5:56 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Anshul Dalal, Michael Hennerich, Yassine Oudjana, Linus Walleij,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea,
Support Opensource, Nick Dyer, Hans de Goede, Job Noorman,
Mika Penttilä, Maxime Coquelin, Alexandre Torgue, Kees Cook,
bui duc phuc, Thorsten Blum, Yauhen Kharuzhy, Sakari Ailus,
Marco Crivellari, Minseong Kim, Ingo Molnar, Thomas Gleixner,
Oleh Kuzhylnyi, Marek Vasut, Krzysztof Kozlowski,
Geert Uytterhoeven, Josua Mayer, Michael Tretter, Jeff LaBundy,
Javier Carrasco, David Heidelberg, Petr Hodina, Svyatoslav Ryhel,
Johannes Kirchmair, Andy Shevchenko, Xichao Zhao, linux-input,
linux-kernel, linux-arm-kernel, platform-driver-x86, linux-stm32
In-Reply-To: <20260515164848.497608-2-u.kleine-koenig@baylibre.com>
Hi Uwe,
On Fri, May 15, 2026 at 06:48:47PM +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.
>
> The mentioned robustness is relevant for a planned change to struct
> i2c_device_id that replaces .driver_data by an anonymous union.
>
> This patch doesn't modify the compiled arrays, only their representation
> in source form benefits. The former was confirmed with x86 and arm64
> builds.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> ---
> Hello,
>
> the mentioned change to i2c_device_id is the following:
>
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 23ff24080dfd..aebd3a5e90af 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -477,7 +477,11 @@ struct rpmsg_device_id {
>
> struct i2c_device_id {
> char name[I2C_NAME_SIZE];
> - kernel_ulong_t driver_data; /* Data private to the driver */
> + union {
> + /* Data private to the driver */
> + kernel_ulong_t driver_data;
> + const void *driver_data_ptr;
> + };
> };
>
> /* pci_epf */
>
> and this requires that .driver_data is assigned via a named initializer
> for static data. This requirement isn't a bad one because named
> initializers are also much better readable than list initializers.
>
> The union added to struct i2c_device_id enables further cleanups like:
>
> diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
> index 66ada7ffbc80..94aa4dc002c5 100644
> --- a/drivers/input/touchscreen/ili210x.c
> +++ b/drivers/input/touchscreen/ili210x.c
> @@ -969,7 +969,7 @@ static int ili210x_i2c_probe(struct i2c_client *client)
>
> chip = device_get_match_data(dev);
> if (!chip && id)
> - chip = (const struct ili2xxx_chip *)id->driver_data;
> + chip = id->driver_data_ptr;
> if (!chip)
> return dev_err_probe(&client->dev, -ENODEV, "unknown device model\n");
>
> @@ -1049,10 +1049,10 @@ static int ili210x_i2c_probe(struct i2c_client *client)
> }
>
> static const struct i2c_device_id ili210x_i2c_id[] = {
> - { .name = "ili210x", .driver_data = (long)&ili210x_chip },
> - { .name = "ili2117", .driver_data = (long)&ili211x_chip },
> - { .name = "ili2120", .driver_data = (long)&ili212x_chip },
> - { .name = "ili251x", .driver_data = (long)&ili251x_chip },
> + { .name = "ili210x", .driver_data_ptr = &ili210x_chip },
> + { .name = "ili2117", .driver_data_ptr = &ili211x_chip },
> + { .name = "ili2120", .driver_data_ptr = &ili212x_chip },
> + { .name = "ili251x", .driver_data_ptr = &ili251x_chip },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, ili210x_i2c_id);
>
> that are an improvement for readability (again!) and it keeps some
> properties of the pointers (here: being const) without having to pay
> attention for that.
>
> My additional motivation for this effort is CHERI[1]. This is a hardware
> extension that uses 128 bit pointers but unsigned long is still 64 bit.
> So with CHERI you cannot store pointers in unsigned long variables.
I like the ability to properly set up pointers for driver data, however
I do not think we should use named initializers for name field. As long
as we are not planning on moving its position I like the brevity of just
saying
{ "ili210x", .driver_data_ptr = &ili210x_chip },
Can we keep the old style for the name field?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v1] Input: Use named initializers for arrays of i2c_device_data
From: Uwe Kleine-König (The Capable Hub) @ 2026-05-30 12:02 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Anshul Dalal, Michael Hennerich, Yassine Oudjana, Linus Walleij,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea,
Support Opensource, Nick Dyer, Hans de Goede, Job Noorman,
Mika Penttilä, Maxime Coquelin, Alexandre Torgue, Kees Cook,
bui duc phuc, Thorsten Blum, Yauhen Kharuzhy, Sakari Ailus,
Marco Crivellari, Minseong Kim, Ingo Molnar, Thomas Gleixner,
Oleh Kuzhylnyi, Marek Vasut, Krzysztof Kozlowski,
Geert Uytterhoeven, Josua Mayer, Michael Tretter, Jeff LaBundy,
Javier Carrasco, David Heidelberg, Petr Hodina, Svyatoslav Ryhel,
Johannes Kirchmair, Andy Shevchenko, Xichao Zhao, linux-input,
linux-kernel, linux-arm-kernel, platform-driver-x86, linux-stm32
In-Reply-To: <ahp7WuHiPYa7Q_pX@google.com>
[-- Attachment #1: Type: text/plain, Size: 1055 bytes --]
Hello Dmitry,
On Fri, May 29, 2026 at 10:56:48PM -0700, Dmitry Torokhov wrote:
> I like the ability to properly set up pointers for driver data, however
> I do not think we should use named initializers for name field. As long
> as we are not planning on moving its position I like the brevity of just
> saying
>
> { "ili210x", .driver_data_ptr = &ili210x_chip },
>
> Can we keep the old style for the name field?
I have no strong opinion here, my patch introduces the named initializer
for both members for consistency. Note that my patches for all the other
subsystems (and also other *_device_id) do the same and you're the first
maintainer with that expressed preference.
I even sent a few patches that adapt initializers without .driver_data
(e.g.
https://lore.kernel.org/all/177932943265.3832404.360063047381979287.git-patchwork-notify@kernel.org/)
that got positive feedback so far.
So if you're ok with having input differ in style from (up to now) all
other subsystems, I can rework the patch.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH] HID: wacom: use GFP_ATOMIC in wacom_wac_queue_flush()
From: Jinmo Yang @ 2026-05-30 15:59 UTC (permalink / raw)
To: Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires
Cc: Dmitry Torokhov, linux-input, linux-kernel, Jinmo Yang, stable
wacom_wac_queue_flush() is called via the .raw_event callback
(wacom_raw_event → wacom_wac_pen_serial_enforce → wacom_wac_queue_flush).
For USB HID devices, this callback is invoked from hid_irq_in(), which
is a URB completion handler running in atomic context. Using GFP_KERNEL
in this path can sleep, leading to a "scheduling while atomic" bug.
Use GFP_ATOMIC instead. The existing code already handles allocation
failure by skipping the fifo entry and continuing.
Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Fixes: 5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX limit")
Cc: stable@vger.kernel.org
Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
---
drivers/hid/wacom_sys.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index a32320b35..2e237bdd2 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -74,7 +74,7 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
unsigned int count;
int err;
- buf = kzalloc(size, GFP_KERNEL);
+ buf = kzalloc(size, GFP_ATOMIC);
if (!buf) {
kfifo_skip(fifo);
continue;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] HID: wacom: use GFP_ATOMIC in wacom_wac_queue_flush()
From: sashiko-bot @ 2026-05-30 16:16 UTC (permalink / raw)
To: Jinmo Yang; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260530155930.128183-1-jinmo44.yang@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] A malicious USB device can trigger a massive heap buffer overflow by sending a HID report larger than the wacom pen_fifo capacity (e.g., > PAGE_SIZE).
--
commit 2421694466917c461a3ca5695c18542757de2d44
Author: Jinmo Yang <jinmo44.yang@gmail.com>
HID: wacom: use GFP_ATOMIC in wacom_wac_queue_flush()
wacom_wac_queue_flush() is called via the .raw_event callback
(wacom_raw_event → wacom_wac_pen_serial_enforce → wacom_wac_queue_flush).
For USB HID devices, this callback is invoked from hid_irq_in(), which
is a URB completion handler running in atomic context. Using GFP_KERNEL
in this path can sleep, leading to a "scheduling while atomic" bug.
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 2220168bf1164..a25400b3e658d 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -74,7 +74,7 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
> unsigned int count;
> int err;
>
> - buf = kzalloc(size, GFP_KERNEL);
> + buf = kzalloc(size, GFP_ATOMIC);
> if (!buf) {
> kfifo_skip(fifo);
[Severity: Critical]
This is a pre-existing issue, but does this usage of kfifo_skip() point to a
similar problem in wacom_wac_queue_insert() in drivers/hid/wacom_sys.c?
If a malicious device spoofs a Wacom device with WACOM_QUIRK_TOOLSERIAL and
sends a report larger than PAGE_SIZE, wouldn't wacom_wac_queue_insert() be
called with size > PAGE_SIZE?
Since pen_fifo capacity is bounded to at most PAGE_SIZE, kfifo_avail(fifo) can
never naturally be >= size, causing the code to loop and blindly call
kfifo_skip(fifo):
drivers/hid/wacom_sys.c:wacom_wac_queue_insert() {
...
while (kfifo_avail(fifo) < size) {
if (!warned)
hid_warn(hdev, "%s: kfifo has filled, starting to drop events\n", __func__);
warned = true;
kfifo_skip(fifo);
}
kfifo_in(fifo, raw_data, size);
...
Doesn't kfifo_skip(fifo) advance fifo->out even when empty, underflowing
fifo->in - fifo->out to a massive unsigned value?
Could this trick kfifo_avail(fifo) into returning a huge number, breaking the
loop and allowing kfifo_in() to write up to 16KB out of bounds into the 4KB
fifo->data buffer via lib/kfifo.c:kfifo_copy_in()?
lib/kfifo.c:kfifo_copy_in() {
...
memcpy(fifo->data, src + l, len - l);
...
> continue;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260530155930.128183-1-jinmo44.yang@gmail.com?part=1
^ permalink raw reply
* [PATCH 6.1 060/969] HID: alps: fix NULL pointer dereference in alps_raw_event()
From: Greg Kroah-Hartman @ 2026-05-30 15:53 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, stable, Jiri Kosina,
Benjamin Tissoires, Masaki Ota, linux-input, Jiri Kosina
In-Reply-To: <20260530160300.485627683@linuxfoundation.org>
6.1-stable review patch. If anyone has any objections, please let me know.
------------------
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 1badfc4319224820d5d890f8eab6aa52e4e83339 upstream.
Commit ecfa6f34492c ("HID: Add HID_CLAIMED_INPUT guards in raw_event
callbacks missing them") attempted to fix up the HID drivers that had
missed the previous fix that was done in 2ff5baa9b527 ("HID: appleir:
Fix potential NULL dereference at raw event handle"), but the alps
driver was missed.
Fix this up by properly checking in the hid-alps driver that it had been
claimed correctly before attempting to process the raw event.
Fixes: 73196ebe134d ("HID: alps: add support for Alps T4 Touchpad device")
Cc: stable <stable@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Benjamin Tissoires <bentiss@kernel.org>
Cc: Masaki Ota <masaki.ota@jp.alps.com>
Cc: linux-input@vger.kernel.org
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/hid/hid-alps.c | 3 +++
1 file changed, 3 insertions(+)
--- a/drivers/hid/hid-alps.c
+++ b/drivers/hid/hid-alps.c
@@ -437,6 +437,9 @@ static int alps_raw_event(struct hid_dev
int ret = 0;
struct alps_dev *hdata = hid_get_drvdata(hdev);
+ if (!(hdev->claimed & HID_CLAIMED_INPUT) || !hdata->input)
+ return 0;
+
switch (hdev->product) {
case HID_PRODUCT_ID_T4_BTNLESS:
ret = t4_raw_event(hdata, data, size);
^ permalink raw reply
* [PATCH 6.1 061/969] HID: core: clamp report_size in s32ton() to avoid undefined shift
From: Greg Kroah-Hartman @ 2026-05-30 15:53 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, stable, Jiri Kosina,
Benjamin Tissoires, linux-input, Jiri Kosina
In-Reply-To: <20260530160300.485627683@linuxfoundation.org>
6.1-stable review patch. If anyone has any objections, please let me know.
------------------
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 69c02ffde6ed4d535fa4e693a9e572729cad3d0d upstream.
s32ton() shifts by n-1 where n is the field's report_size, a value that
comes directly from a HID device. The HID parser bounds report_size
only to <= 256, so a broken HID device can supply a report descriptor
with a wide field that triggers shift exponents up to 256 on a 32-bit
type when an output report is built via hid_output_field() or
hid_set_field().
Commit ec61b41918587 ("HID: core: fix shift-out-of-bounds in
hid_report_raw_event") added the same n > 32 clamp to the function
snto32(), but s32ton() was never given the same fix as I guess syzbot
hadn't figured out how to fuzz a device the same way.
Fix this up by just clamping the max value of n, just like snto32()
does.
Cc: stable <stable@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Benjamin Tissoires <bentiss@kernel.org>
Cc: linux-input@vger.kernel.org
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/hid/hid-core.c | 3 +++
1 file changed, 3 insertions(+)
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1358,6 +1358,9 @@ static u32 s32ton(__s32 value, unsigned
if (!value || !n)
return 0;
+ if (n > 32)
+ n = 32;
+
a = value >> (n - 1);
if (a && a != -1)
^ permalink raw reply
* [PATCH 5.15 056/776] HID: alps: fix NULL pointer dereference in alps_raw_event()
From: Greg Kroah-Hartman @ 2026-05-30 15:56 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, stable, Jiri Kosina,
Benjamin Tissoires, Masaki Ota, linux-input, Jiri Kosina
In-Reply-To: <20260530160240.228940103@linuxfoundation.org>
5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 1badfc4319224820d5d890f8eab6aa52e4e83339 upstream.
Commit ecfa6f34492c ("HID: Add HID_CLAIMED_INPUT guards in raw_event
callbacks missing them") attempted to fix up the HID drivers that had
missed the previous fix that was done in 2ff5baa9b527 ("HID: appleir:
Fix potential NULL dereference at raw event handle"), but the alps
driver was missed.
Fix this up by properly checking in the hid-alps driver that it had been
claimed correctly before attempting to process the raw event.
Fixes: 73196ebe134d ("HID: alps: add support for Alps T4 Touchpad device")
Cc: stable <stable@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Benjamin Tissoires <bentiss@kernel.org>
Cc: Masaki Ota <masaki.ota@jp.alps.com>
Cc: linux-input@vger.kernel.org
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/hid/hid-alps.c | 3 +++
1 file changed, 3 insertions(+)
--- a/drivers/hid/hid-alps.c
+++ b/drivers/hid/hid-alps.c
@@ -437,6 +437,9 @@ static int alps_raw_event(struct hid_dev
int ret = 0;
struct alps_dev *hdata = hid_get_drvdata(hdev);
+ if (!(hdev->claimed & HID_CLAIMED_INPUT) || !hdata->input)
+ return 0;
+
switch (hdev->product) {
case HID_PRODUCT_ID_T4_BTNLESS:
ret = t4_raw_event(hdata, data, size);
^ permalink raw reply
* [PATCH 5.15 057/776] HID: core: clamp report_size in s32ton() to avoid undefined shift
From: Greg Kroah-Hartman @ 2026-05-30 15:56 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, stable, Jiri Kosina,
Benjamin Tissoires, linux-input, Jiri Kosina
In-Reply-To: <20260530160240.228940103@linuxfoundation.org>
5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 69c02ffde6ed4d535fa4e693a9e572729cad3d0d upstream.
s32ton() shifts by n-1 where n is the field's report_size, a value that
comes directly from a HID device. The HID parser bounds report_size
only to <= 256, so a broken HID device can supply a report descriptor
with a wide field that triggers shift exponents up to 256 on a 32-bit
type when an output report is built via hid_output_field() or
hid_set_field().
Commit ec61b41918587 ("HID: core: fix shift-out-of-bounds in
hid_report_raw_event") added the same n > 32 clamp to the function
snto32(), but s32ton() was never given the same fix as I guess syzbot
hadn't figured out how to fuzz a device the same way.
Fix this up by just clamping the max value of n, just like snto32()
does.
Cc: stable <stable@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Benjamin Tissoires <bentiss@kernel.org>
Cc: linux-input@vger.kernel.org
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/hid/hid-core.c | 3 +++
1 file changed, 3 insertions(+)
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1354,6 +1354,9 @@ static u32 s32ton(__s32 value, unsigned
if (!value || !n)
return 0;
+ if (n > 32)
+ n = 32;
+
a = value >> (n - 1);
if (a && a != -1)
return value < 0 ? 1 << (n - 1) : (1 << (n - 1)) - 1;
^ permalink raw reply
* Re: [PATCH 2/4] HID: core: introduce hid_safe_input_report()
From: Carlos Llamas @ 2026-05-30 17:43 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Bastien Nocera, Benjamin Tissoires, Jiri Kosina,
Filipe Laíns, Ping Cheng, Jason Gerecke, Viresh Kumar,
Johan Hovold, Alex Elder, Greg Kroah-Hartman, Lee Jones,
linux-input, linux-kernel, greybus-dev, linux-staging, linux-usb,
stable
In-Reply-To: <CAO-hwJ+EgC0pM6L6vGFEaRFt2Nwj5b-CCf_5e5VkvrXgdHrjNg@mail.gmail.com>
On Thu, Apr 16, 2026 at 04:46:28PM +0200, Benjamin Tissoires wrote:
> On Thu, Apr 16, 2026 at 11:41 AM Bastien Nocera <hadess@hadess.net> wrote:
> >
> > On Wed, 2026-04-15 at 11:38 +0200, Benjamin Tissoires wrote:
> > > hid_input_report() is used in too many places to have a commit that
> > > doesn't cross subsystem borders. Instead of changing the API,
> > > introduce
> > > a new one when things matters in the transport layers:
> > > - usbhid
> > > - i2chid
> > >
> > > This effectively revert to the old behavior for those two transport
> > > layers.
> > >
> > > Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> > > bogus memset()")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> > > ---
> > > drivers/hid/hid-core.c | 21 +++++++++++++++++++++
> > > drivers/hid/i2c-hid/i2c-hid-core.c | 7 ++++---
> > > drivers/hid/usbhid/hid-core.c | 11 ++++++-----
> > > include/linux/hid.h | 2 ++
> > > 4 files changed, 33 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > > index a806820df7e5..cb0ad99e7a0a 100644
> > > --- a/drivers/hid/hid-core.c
> > > +++ b/drivers/hid/hid-core.c
> > > @@ -2191,6 +2191,27 @@ int hid_input_report(struct hid_device *hid,
> > > enum hid_report_type type, u8 *data
> > > }
> > > EXPORT_SYMBOL_GPL(hid_input_report);
> > >
> > > +/**
> > > + * hid_safe_input_report - report data from lower layer (usb, bt...)
> > > + *
> > > + * @hid: hid device
> > > + * @type: HID report type (HID_*_REPORT)
> > > + * @data: report contents
> > > + * @bufsize: allocated size of the data buffer
> > > + * @size: useful size of data parameter
> > > + * @interrupt: distinguish between interrupt and control transfers
> > > + *
> > > + * This is data entry for lower layers.
> >
> > You probably want to explain why it should be used instead of
> > hid_input_report() in this doc blurb, and modify the hid_input_report()
> > docs to mention that this should be used.
>
> Good point. Sending v2 ASAP.
>
> >
> > Maybe hid_input_report() should also be marked as deprecated somehow,
> > to avoid new users?
>
> Well, it's not entirely deprecated because, for instance, in uhid we
> only have the buffer with the provided size around. So we can't be
> less restrictive in that precise case, and then switching to _safe
> will not change a bit.
>
> Cheers,
> Benjamin
Hi Benjamin, our CI started failing with commit 0a3fe972a7cb ("HID:
core: Mitigate potential OOB by removing bogus memset()"), so I was
hoping your patchset would fix this.
However, I just realized our call path goes through uhid precisely,
which still triggers the EINVAL error since uhid as not converted to
hid_safe_input_report().
My vague understanding though, is that uhid_event uses a static buffer
in ev->data[UHID_DATA_MAX], so maybe we can use that through
uhid_dev_input{2}()?
I ran the following path through our CI and it fixed our issue, so I
wanted to get your thoughts on this.
Carlos Llamas
---
drivers/hid/uhid.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 524b53a3c87b..37b60c3aaf66 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -595,8 +595,8 @@ static int uhid_dev_input(struct uhid_device *uhid, struct uhid_event *ev)
if (!READ_ONCE(uhid->running))
return -EINVAL;
- hid_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input.data,
- min_t(size_t, ev->u.input.size, UHID_DATA_MAX), 0);
+ hid_safe_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input.data, UHID_DATA_MAX,
+ min_t(size_t, ev->u.input.size, UHID_DATA_MAX), 0);
return 0;
}
@@ -606,8 +606,8 @@ static int uhid_dev_input2(struct uhid_device *uhid, struct uhid_event *ev)
if (!READ_ONCE(uhid->running))
return -EINVAL;
- hid_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input2.data,
- min_t(size_t, ev->u.input2.size, UHID_DATA_MAX), 0);
+ hid_safe_input_report(uhid->hid, HID_INPUT_REPORT, ev->u.input2.data, UHID_DATA_MAX,
+ min_t(size_t, ev->u.input2.size, UHID_DATA_MAX), 0);
return 0;
}
^ permalink raw reply related
* [PATCH 5.10 047/589] HID: alps: fix NULL pointer dereference in alps_raw_event()
From: Greg Kroah-Hartman @ 2026-05-30 15:58 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, stable, Jiri Kosina,
Benjamin Tissoires, Masaki Ota, linux-input, Jiri Kosina
In-Reply-To: <20260530160224.570625122@linuxfoundation.org>
5.10-stable review patch. If anyone has any objections, please let me know.
------------------
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 1badfc4319224820d5d890f8eab6aa52e4e83339 upstream.
Commit ecfa6f34492c ("HID: Add HID_CLAIMED_INPUT guards in raw_event
callbacks missing them") attempted to fix up the HID drivers that had
missed the previous fix that was done in 2ff5baa9b527 ("HID: appleir:
Fix potential NULL dereference at raw event handle"), but the alps
driver was missed.
Fix this up by properly checking in the hid-alps driver that it had been
claimed correctly before attempting to process the raw event.
Fixes: 73196ebe134d ("HID: alps: add support for Alps T4 Touchpad device")
Cc: stable <stable@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Benjamin Tissoires <bentiss@kernel.org>
Cc: Masaki Ota <masaki.ota@jp.alps.com>
Cc: linux-input@vger.kernel.org
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/hid/hid-alps.c | 3 +++
1 file changed, 3 insertions(+)
--- a/drivers/hid/hid-alps.c
+++ b/drivers/hid/hid-alps.c
@@ -437,6 +437,9 @@ static int alps_raw_event(struct hid_dev
int ret = 0;
struct alps_dev *hdata = hid_get_drvdata(hdev);
+ if (!(hdev->claimed & HID_CLAIMED_INPUT) || !hdata->input)
+ return 0;
+
switch (hdev->product) {
case HID_PRODUCT_ID_T4_BTNLESS:
ret = t4_raw_event(hdata, data, size);
^ permalink raw reply
* [PATCH 5.10 048/589] HID: core: clamp report_size in s32ton() to avoid undefined shift
From: Greg Kroah-Hartman @ 2026-05-30 15:58 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, stable, Jiri Kosina,
Benjamin Tissoires, linux-input, Jiri Kosina
In-Reply-To: <20260530160224.570625122@linuxfoundation.org>
5.10-stable review patch. If anyone has any objections, please let me know.
------------------
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 69c02ffde6ed4d535fa4e693a9e572729cad3d0d upstream.
s32ton() shifts by n-1 where n is the field's report_size, a value that
comes directly from a HID device. The HID parser bounds report_size
only to <= 256, so a broken HID device can supply a report descriptor
with a wide field that triggers shift exponents up to 256 on a 32-bit
type when an output report is built via hid_output_field() or
hid_set_field().
Commit ec61b41918587 ("HID: core: fix shift-out-of-bounds in
hid_report_raw_event") added the same n > 32 clamp to the function
snto32(), but s32ton() was never given the same fix as I guess syzbot
hadn't figured out how to fuzz a device the same way.
Fix this up by just clamping the max value of n, just like snto32()
does.
Cc: stable <stable@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Benjamin Tissoires <bentiss@kernel.org>
Cc: linux-input@vger.kernel.org
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/hid/hid-core.c | 3 +++
1 file changed, 3 insertions(+)
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1354,6 +1354,9 @@ static u32 s32ton(__s32 value, unsigned
if (!value || !n)
return 0;
+ if (n > 32)
+ n = 32;
+
a = value >> (n - 1);
if (a && a != -1)
return value < 0 ? 1 << (n - 1) : (1 << (n - 1)) - 1;
^ permalink raw reply
* Re: [PATCH v5 2/3] HID: nintendo: Add rumble support for Switch 2 controllers
From: Silvan Jegen @ 2026-05-30 20:03 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Vicki Pfau, dmitry.torokhov, linux-input
In-Reply-To: <20260514005353.EF8D8C19425@smtp.kernel.org>
sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [High] Mixing jiffies and milliseconds in delayed work scheduling logic causes an integer underflow, permanently stalling the haptic rumble.
> - [High] A guaranteed NULL pointer dereference occurs in the error logging path of switch2_rumble_work, as well as a TOCTOU race condition.
> - [Medium] `ns2->ctlr_type` is checked before it is dynamically initialized, causing union corruption for GameCube controllers.
> - [Low] Implicit type promotion in the max() macro causes a compiler warning.
> - [Low] Kconfig documentation claims individual motor control for the Pro Controller, but the implementation identicalizes the rumble data for both motors.
> --
>
> commit d72f0f8e0dadde2cacc27050b92088c2e5cbea0e
> Author: Vicki Pfau <vi@endrift.com>
> Subject: HID: nintendo: Add rumble support for Switch 2 controllers
>
> This commit introduces rumble support for Switch 2 controllers, handling both HD
> Rumble linear resonant actuators and eccentric rotating mass haptics. It adds
> state machines for rumble scheduling and output reporting via hdev.
>
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index 26ef589f652c6..a7fe74d982475 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -850,10 +850,10 @@ config NINTENDO_FF
> > depends on HID_NINTENDO
> > select INPUT_FF_MEMLESS
> > help
> > - Say Y here if you have a Nintendo Switch controller and want to enable
> > - force feedback support for it. This works for both joy-cons, the pro
> > - controller, and the NSO N64 controller. For the pro controller, both
> > - rumble motors can be controlled individually.
> > + Say Y here if you have a Nintendo Switch or Switch 2 controller and want
> > + to enable force feedback support for it. This works for Joy-Cons, the Pro
> > + Controllers, and the NSO N64 and GameCube controller. For the Pro
> > + Controller, both rumble motors can be controlled individually.
>
> This isn't a bug, but is the help text accurate regarding individual motor
> control? The implementation in switch2_rumble_work() appears to encode the
> exact same rumble configuration to both the left and right motor offsets,
> which would make both motors vibrate identically.
As pointers to different offsets into the rumble_buffer are passed,
I assume that the vibrations will actually not be identical.
>
> > diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> > index 2193afcdefabc..9801799a90758 100644
> > --- a/drivers/hid/hid-nintendo.c
> > +++ b/drivers/hid/hid-nintendo.c
>
> [ ... ]
>
> > @@ -3103,6 +3155,107 @@ static inline bool switch2_ctlr_is_joycon(enum switch2_ctlr_type type)
> > return type == NS2_CTLR_TYPE_JCL || type == NS2_CTLR_TYPE_JCR;
> > }
> >
> > +#if IS_ENABLED(CONFIG_NINTENDO_FF)
> > +static void switch2_encode_rumble(struct switch2_hd_rumble *rumble, uint8_t buffer[5])
> > +{
> > + buffer[0] = rumble->hi_freq;
> > + buffer[1] = (rumble->hi_freq >> 8) | (rumble->hi_amp << 2);
> > + buffer[2] = (rumble->hi_amp >> 6) | (rumble->lo_freq << 4);
> > + buffer[3] = (rumble->lo_freq >> 4) | (rumble->lo_amp << 6);
> > + buffer[4] = rumble->lo_amp >> 2;
> > +}
> > +
> > +static int switch2_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
> > +{
> > + struct switch2_controller *ns2 = input_get_drvdata(dev);
> > +
> > + if (effect->type != FF_RUMBLE)
> > + return 0;
> > +
> > + guard(spinlock_irqsave)(&ns2->rumble_lock);
> > + if (ns2->ctlr_type == NS2_CTLR_TYPE_GC) {
> > + ns2->rumble.sd.amplitude = max(effect->u.rumble.strong_magnitude,
> > + effect->u.rumble.weak_magnitude >> 1);
>
> Will this max calculation trigger a compiler warning? The strict type checking
> in the max macro might fail because effect->u.rumble.weak_magnitude >> 1
> promotes to an int, while strong_magnitude is a u16.
Apparently it is not strict enough to emit a warning on my machine at
least ...
>
> > + } else {
> > + ns2->rumble.hd.hi_amp = effect->u.rumble.weak_magnitude * RUMBLE_MAX >> 16;
> > + ns2->rumble.hd.lo_amp = effect->u.rumble.strong_magnitude * RUMBLE_MAX >> 16;
> > + }
> > +
> > + if (ns2->hdev)
> > + schedule_delayed_work(&ns2->rumble_work, 0);
> > +
> > + return 0;
> > +}
> > +
> > +static void switch2_rumble_work(struct work_struct *work)
> > +{
> > + struct switch2_controller *ns2 = container_of(to_delayed_work(work),
> > + struct switch2_controller, rumble_work);
> > + unsigned long current_ms = jiffies_to_msecs(get_jiffies_64());
> > + unsigned long flags;
> > + bool active;
> > + int ret;
>
> [ ... ]
>
> > + if (active) {
> > + unsigned long interval = msecs_to_jiffies(4);
> > +
> > + if (!ns2->last_rumble_work)
> > + ns2->last_rumble_work = current_ms;
> > + else
> > + ns2->last_rumble_work += interval;
> > + schedule_delayed_work(&ns2->rumble_work,
> > + ns2->last_rumble_work + interval - current_ms);
>
> Does this code underflow the scheduled delay?
>
> The current_ms variable is retrieved in milliseconds using jiffies_to_msecs(),
> but interval is calculated in jiffies using msecs_to_jiffies(). Since
> ns2->last_rumble_work is initialized to milliseconds but incremented by
> jiffies, subtracting current_ms could yield a negative value.
>
> On systems where HZ is not 1000, could this underflow the delay parameter and
> schedule the next work millions of jiffies in the future?
It's definitely suspicious that we are adding jiffies to milliseconds. We
should probably fix that.
>
> > + } else {
> > + ns2->last_rumble_work = 0;
> > + }
> > + spin_unlock_irqrestore(&ns2->rumble_lock, flags);
> > +
> > + if (!ns2->hdev) {
> > + cancel_delayed_work(&ns2->rumble_work);
> > + ret = -ENODEV;
> > + } else {
> > + ret = hid_hw_output_report(ns2->hdev, ns2->rumble_buffer, 64);
> > + }
> > +
> > + if (ret < 0)
> > + hid_dbg(ns2->hdev, "Failed to send output report ret=%d\n", ret);
>
> If ns2->hdev is NULL, we set ret = -ENODEV and then reach this hid_dbg call.
> Could this result in a NULL pointer dereference, as the hid_dbg macro passes
> ns2->hdev to dev_dbg()?
Since we set `ret = -ENODEV` when ns2->hdev is NULL we would indeed end
up calling hid_dbg() with a NULL argument.
>
> Also, since ns2->hdev is accessed locklessly and without READ_ONCE(), is there
> a time-of-check to time-of-use race where the compiler might reload the pointer
> between the check and hid_hw_output_report()?
>
> [ ... ]
>
> > @@ -3881,6 +4048,14 @@ static int switch2_probe(struct hid_device *hdev, const struct hid_device_id *id
> >
> > switch2_leds_create(ns2);
> >
> > +#if IS_ENABLED(CONFIG_NINTENDO_FF)
> > + if (ns2->ctlr_type != NS2_CTLR_TYPE_GC) {
> > + ns2->rumble.hd.hi_freq = RUMBLE_HI_FREQ;
> > + ns2->rumble.hd.lo_freq = RUMBLE_LO_FREQ;
> > + }
>
> Are we checking ns2->ctlr_type before it is dynamically initialized?
>
> During probe, ns2->ctlr_type appears to be 0 since the struct is freshly
> allocated and the type is discovered later during the init state machine.
> This would cause the high and low frequencies to always be written.
As switch2_init_controller is called from the other driver's probe
function, this might be fine. To avoid writing these values if the
initialisation has not happened yet, we should probably guard against
a 0 value.
Cheers,
Silvan
>
> Since hd and sd share a union, could this corrupt the sd.error and
> sd.amplitude fields for actual GameCube controllers?
>
> > + spin_lock_init(&ns2->rumble_lock);
> > + INIT_DELAYED_WORK(&ns2->rumble_work, switch2_rumble_work);
> > +#endif
> > hid_set_drvdata(hdev, ns2);
^ permalink raw reply
* [PATCH] HID: logitech-hidpp: remove excess kernel-doc member in hidpp_scroll_counter
From: Rosen Penev @ 2026-05-31 0:01 UTC (permalink / raw)
To: linux-input
Cc: Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires, Hans de Goede, open list
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
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/hid/hid-logitech-hidpp.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index ccbf28869a96..1990ba5b26ea 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -164,7 +164,6 @@ struct hidpp_battery {
/**
* struct hidpp_scroll_counter - Utility class for processing high-resolution
* scroll events.
- * @dev: the input device for which events should be reported.
* @wheel_multiplier: the scalar multiplier to be applied to each wheel event
* @remainder: counts the number of high-resolution units moved since the last
* low-resolution event (REL_WHEEL or REL_HWHEEL) was sent. Should
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] HID: wacom: use GFP_ATOMIC in wacom_wac_queue_flush()
From: Dmitry Torokhov @ 2026-05-31 4:25 UTC (permalink / raw)
To: Jinmo Yang
Cc: Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
linux-input, linux-kernel, stable
In-Reply-To: <20260530155930.128183-1-jinmo44.yang@gmail.com>
Hi Jinmo,
On Sun, May 31, 2026 at 12:59:30AM +0900, Jinmo Yang wrote:
> wacom_wac_queue_flush() is called via the .raw_event callback
> (wacom_raw_event → wacom_wac_pen_serial_enforce → wacom_wac_queue_flush).
> For USB HID devices, this callback is invoked from hid_irq_in(), which
> is a URB completion handler running in atomic context. Using GFP_KERNEL
> in this path can sleep, leading to a "scheduling while atomic" bug.
>
> Use GFP_ATOMIC instead. The existing code already handles allocation
> failure by skipping the fifo entry and continuing.
>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
If you want to give credit this should be "Reported-by: Sashiko-bot <...>".
> Fixes: 5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX limit")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
> ---
> drivers/hid/wacom_sys.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index a32320b35..2e237bdd2 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -74,7 +74,7 @@ static void wacom_wac_queue_flush(struct hid_device *hdev,
> unsigned int count;
> int err;
>
> - buf = kzalloc(size, GFP_KERNEL);
> + buf = kzalloc(size, GFP_ATOMIC);
> if (!buf) {
> kfifo_skip(fifo);
> continue;
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
As a followup please consider changing 'buf' management to use cleanup
facilities:
u8 *buf __free(kfree) = kzalloc(size, GFP_ATOMIC);
if (!buf) {
kfifo_skiip(fifo);
continue;
}
...
Thanks.
--
Dmitry
^ permalink raw reply
* [git pull] Input updates for v7.1-rc5
From: Dmitry Torokhov @ 2026-05-31 5:09 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-input
Hi Linus,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v7.1-rc5
to receive updates for the input subsystem. You will get:
- updates to Elan I2C touchpad driver to handle a new IC type and to
validate size of supplied firmware to prevent OOB access
- updates to Xpad controller driver to recognize ASUS ROG RAIKIRI II and
"Nova 2 Lite" from GameSir controllers as well as a fix to prevent a
potential OOB access when handling "Share" button
- an update to Synaptics touchpad driver to use RMI mode for touchpad in
Thinkpad E490
- updates to Atmel MXT driver adding checks to prevent potential OOB
accesses
- a fix to IMS PCU driver to free correct amount of memory when
tearing it down
- a fixup to the recent change to Atlas buttons driver
- a small cleanup in fm801-fp for PCI IDs table initialisation.
Changelog:
---------
Dmitriy Zharov (1):
Input: xpad - add support for ASUS ROG RAIKIRI II
Dmitry Torokhov (4):
Input: xpad - fix out-of-bounds access for Share button
Input: elan_i2c - validate firmware size before use
Input: atmel_mxt_ts - fix boundary check in mxt_prepare_cfg_mem
Input: atmel_mxt_ts - check mem_size before calculating config memory size
Greg Kroah-Hartman (1):
Input: usbtouchscreen - clamp NEXIO data_len/x_len to URB buffer size
Jingle Wu 吳金國 (2):
Input: elan_i2c - add ic type 0x19
Input: elan_i2c - increase device reset wait timeout after update FW
Nicolás Bazaes (1):
Input: synaptics - add LEN2058 to SMBus passlist for ThinkPad E490
Qbeliw Tanaka (1):
Input: xpad - add "Nova 2 Lite" from GameSir
Rafael J. Wysocki (1):
Input: atlas - check ACPI_COMPANION() against NULL
Thomas Fourier (1):
Input: ims-pcu - fix usb_free_coherent() size in ims_pcu_buffers_free()
Uwe Kleine-König (The Capable Hub) (1):
Input: fm801-gp - simplify initialisation of pci_device_id array
Diffstat:
--------
drivers/input/gameport/fm801-gp.c | 4 ++--
drivers/input/joystick/xpad.c | 14 ++++++++++----
drivers/input/misc/atlas_btns.c | 6 +++++-
drivers/input/misc/ims-pcu.c | 2 +-
drivers/input/mouse/elan_i2c_core.c | 8 ++++++++
drivers/input/mouse/elan_i2c_i2c.c | 2 +-
drivers/input/mouse/synaptics.c | 1 +
drivers/input/touchscreen/atmel_mxt_ts.c | 13 ++++++++++---
drivers/input/touchscreen/usbtouchscreen.c | 5 +++++
9 files changed, 43 insertions(+), 12 deletions(-)
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v1] Input: Use named initializers for arrays of i2c_device_data
From: Dmitry Torokhov @ 2026-05-31 5:13 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Anshul Dalal, Michael Hennerich, Yassine Oudjana, Linus Walleij,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea,
Support Opensource, Nick Dyer, Hans de Goede, Job Noorman,
Mika Penttilä, Maxime Coquelin, Alexandre Torgue, Kees Cook,
bui duc phuc, Thorsten Blum, Yauhen Kharuzhy, Sakari Ailus,
Marco Crivellari, Minseong Kim, Ingo Molnar, Thomas Gleixner,
Oleh Kuzhylnyi, Marek Vasut, Krzysztof Kozlowski,
Geert Uytterhoeven, Josua Mayer, Michael Tretter, Jeff LaBundy,
Javier Carrasco, David Heidelberg, Petr Hodina, Svyatoslav Ryhel,
Johannes Kirchmair, Andy Shevchenko, Xichao Zhao, linux-input,
linux-kernel, linux-arm-kernel, platform-driver-x86, linux-stm32
In-Reply-To: <ahrP4STTFdUsBp0O@monoceros>
On Sat, May 30, 2026 at 02:02:20PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> Hello Dmitry,
>
> On Fri, May 29, 2026 at 10:56:48PM -0700, Dmitry Torokhov wrote:
> > I like the ability to properly set up pointers for driver data, however
> > I do not think we should use named initializers for name field. As long
> > as we are not planning on moving its position I like the brevity of just
> > saying
> >
> > { "ili210x", .driver_data_ptr = &ili210x_chip },
> >
> > Can we keep the old style for the name field?
>
> I have no strong opinion here, my patch introduces the named initializer
> for both members for consistency. Note that my patches for all the other
> subsystems (and also other *_device_id) do the same and you're the first
> maintainer with that expressed preference.
>
> I even sent a few patches that adapt initializers without .driver_data
> (e.g.
> https://lore.kernel.org/all/177932943265.3832404.360063047381979287.git-patchwork-notify@kernel.org/)
> that got positive feedback so far.
>
> So if you're ok with having input differ in style from (up to now) all
> other subsystems, I can rework the patch.
OK, I guess I am too late to voice my opinion. I do not want to have
different style, in this particular case at least, so I applied the
patch.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: iqs5xx - Drop unused i2c driver_data
From: Dmitry Torokhov @ 2026-05-31 5:14 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Andrew Morton, Randy Dunlap, Andy Shevchenko, linux-input,
linux-kernel
In-Reply-To: <20260515165135.498505-2-u.kleine-koenig@baylibre.com>
On Fri, May 15, 2026 at 06:51:34PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> The driver doesn't make use of the value that was explicitly assigned to
> the .driver_data members. Drop the assignment. While touching the array,
> convert it to use named initialization which is easier to understand.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox