* [PATCH] Input: rmi4 - release F54 queue on video registration failure
From: Myeonghun Pak @ 2026-05-24 18:23 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, linux-kernel, Myeonghun Pak, stable, Ijae Kim
rmi_f54_probe() initializes the videobuf2 queue before registering the
video device. If video_register_device() fails, probe only unregisters
the V4L2 device and leaves the initialized queue unwound by neither
remove nor file release paths.
Release the queue before continuing through the existing probe error
path.
This issue was identified during our ongoing static-analysis research while
reviewing kernel code.
Fixes: 3a762dbd5347 ("[media] Input: synaptics-rmi4 - add support for F54 diagnostics")
Cc: stable@vger.kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
drivers/input/rmi4/rmi_f54.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
index 61909e1a39..fca7b9fec5 100644
--- a/drivers/input/rmi4/rmi_f54.c
+++ b/drivers/input/rmi4/rmi_f54.c
@@ -722,6 +722,7 @@ static int rmi_f54_probe(struct rmi_function *fn)
ret = video_register_device(&f54->vdev, VFL_TYPE_TOUCH, -1);
if (ret) {
dev_err(&fn->dev, "Unable to register video subdevice.");
+ vb2_queue_release(&f54->queue);
goto remove_v4l2;
}
--
2.47.1
^ permalink raw reply related
* [PATCH] HID: wacom: stop hardware after post-start probe failures
From: Myeonghun Pak @ 2026-05-24 17:53 UTC (permalink / raw)
To: Ping Cheng, Jason Gerecke
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
Myeonghun Pak, stable, Ijae Kim
wacom_parse_and_register() starts HID hardware before registering inputs
and initializing pad LEDs/remotes. Those later steps can fail, but their
error paths currently release Wacom resources without stopping the HID
hardware.
Route post-hid_hw_start() failures through hid_hw_stop() before
releasing driver resources.
This issue was identified during our ongoing static-analysis research while
reviewing kernel code.
Fixes: c1d6708bf0d3 ("HID: wacom: Do not register input devices until after hid_hw_start")
Cc: stable@vger.kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
drivers/hid/wacom_sys.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 0d1c6d90fe..c824d9c224 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2456,16 +2456,16 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
error = wacom_register_inputs(wacom);
if (error)
- goto fail;
+ goto fail_hw_stop;
if (wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PAD) {
error = wacom_initialize_leds(wacom);
if (error)
- goto fail;
+ goto fail_hw_stop;
error = wacom_initialize_remotes(wacom);
if (error)
- goto fail;
+ goto fail_hw_stop;
}
if (!wireless) {
@@ -2496,6 +2496,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
return 0;
fail_quirks:
+fail_hw_stop:
hid_hw_stop(hdev);
fail:
wacom_release_resources(wacom);
--
2.47.1
^ permalink raw reply related
* Re: [PATCH 1/1] HID: wacom: validate report size before kfifo insert
From: sashiko-bot @ 2026-05-24 14:31 UTC (permalink / raw)
To: Jinmo Yang; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260524135203.1996265-2-jinmo44.yang@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Critical] Incomplete vulnerability fix / off-by-two error allowing kfifo state corruption and massive out-of-bounds slab read.
Pre-existing issues:
- [High] Sleeping in atomic context due to kzalloc(GFP_KERNEL) in wacom_wac_queue_flush().
--
commit 077dd88850a2e1126c613aafa226a5f028a98bb9
Author: Jinmo Yang <jinmo44.yang@gmail.com>
HID: wacom: validate report size before kfifo insert
This commit adds a check in wacom_wac_queue_insert() to reject reports
that exceed the total kfifo capacity. This aims to prevent oversized
reports from causing out-of-bounds writes during kfifo insertion.
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index a32320b351e3e..cc82c6f36901a 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -54,6 +54,12 @@ static void wacom_wac_queue_insert(struct hid_device *hdev,
> {
> bool warned = false;
>
> + if (size > kfifo_size(fifo)) {
[Severity: Critical]
Does this validation correctly account for the record header overhead?
Since the driver uses a record-based kfifo (kfifo_rec_ptr_2), each inserted
record is prefixed with a 2-byte length header. This means the maximum
available payload space in an empty kfifo is kfifo_size(fifo) - 2.
If an attacker sends a report with a size exactly equal to kfifo_size(fifo)
or kfifo_size(fifo) - 1, this check falsely passes.
> + hid_warn(hdev, "%s: report too large (%d > %u) for kfifo\n",
> + __func__, size, kfifo_size(fifo));
> + return;
> + }
> +
> while (kfifo_avail(fifo) < size) {
If the payload requires more space than the empty kfifo can provide,
the kfifo_avail() check will be smaller than size, and the loop will
execute on an empty buffer:
> if (!warned)
> hid_warn(hdev, "%s: kfifo has filled, starting to drop events\n", __func__);
> warned = true;
>
> kfifo_skip(fifo);
> }
Could calling kfifo_skip() on an empty kfifo underflow the internal
pointers and corrupt the ring buffer's state?
If this allows the subsequent kfifo_in() to write an oversized payload
and overwrite the 2-byte record header with attacker-controlled data,
would this lead to a massive out-of-bounds read later when
wacom_wac_queue_flush() processes the corrupted record?
[Severity: High]
This is a pre-existing issue, but does the related flush function
wacom_wac_queue_flush() safely allocate memory?
It appears wacom_wac_queue_flush() calls kzalloc() with GFP_KERNEL.
Since this queue processing path is invoked from wacom_raw_event()
(which executes in an atomic context from the USB URB completion handler),
could this GFP_KERNEL allocation sleep and trigger a 'scheduling while
atomic' panic? Should this use GFP_ATOMIC instead?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260524135203.1996265-2-jinmo44.yang@gmail.com?part=1
^ permalink raw reply
* [PATCH 1/1] HID: wacom: validate report size before kfifo insert
From: Jinmo Yang @ 2026-05-24 13:52 UTC (permalink / raw)
To: Jason Gerecke, Ping Cheng
Cc: Jinmo Yang, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel, stable
In-Reply-To: <20260524135203.1996265-1-jinmo44.yang@gmail.com>
wacom_wac_queue_insert() passes the report size directly to kfifo_in()
without checking whether the report fits in the kfifo buffer.
Since commit 5e013ad20689 ("HID: wacom: Remove static WACOM_PKGLEN_MAX
limit"), the kfifo is sized dynamically as min(PAGE_SIZE, 10 * pktlen),
which can be as small as 256 bytes. However, reports received via
UHID_INPUT2 can be up to UHID_DATA_MAX (4096) bytes. When such an
oversized report reaches wacom_wac_queue_insert(), the existing
kfifo_avail() loop cannot make room for a record larger than the total
buffer, causing kfifo_copy_in() to memcpy up to 3840 bytes past the
slab allocation.
Add a size check at the top of wacom_wac_queue_insert() to reject
reports that exceed the kfifo capacity.
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 | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index a32320b..cc82c6f 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -54,6 +54,12 @@ static void wacom_wac_queue_insert(struct hid_device *hdev,
{
bool warned = false;
+ if (size > kfifo_size(fifo)) {
+ hid_warn(hdev, "%s: report too large (%d > %u) for kfifo\n",
+ __func__, size, kfifo_size(fifo));
+ return;
+ }
+
while (kfifo_avail(fifo) < size) {
if (!warned)
hid_warn(hdev, "%s: kfifo has filled, starting to drop events\n", __func__);
--
2.53.0
^ permalink raw reply related
* [PATCH 0/1] HID: wacom: fix slab-out-of-bounds write in kfifo_copy_in
From: Jinmo Yang @ 2026-05-24 13:52 UTC (permalink / raw)
To: Jason Gerecke, Ping Cheng
Cc: Jinmo Yang, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel, stable
Hi,
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
Allocated by task 4174:
kasan_save_stack+0x30/0x50 mm/kasan/common.c:57
kasan_save_track+0x14/0x30 mm/kasan/common.c:78
poison_kmalloc_redzone mm/kasan/common.c:398 [inline]
__kasan_kmalloc+0x8f/0xa0 mm/kasan/common.c:415
kasan_kmalloc include/linux/kasan.h:263 [inline]
__do_kmalloc_node mm/slub.c:5309 [inline]
__kmalloc_node_noprof+0x19a/0x4e0 mm/slub.c:5315
_kmalloc_array_node_noprof include/linux/slab.h:1269 [inline]
__kfifo_alloc_node+0x11e/0x260 lib/kfifo.c:44
__kfifo_alloc include/linux/kfifo.h:932 [inline]
wacom_devm_kfifo_alloc drivers/hid/wacom_sys.c:1315 [inline]
wacom_parse_and_register+0x2b4/0x5640 drivers/hid/wacom_sys.c:2381
wacom_probe+0x8d5/0xc40 drivers/hid/wacom_sys.c:2880
The buggy address belongs to the object at ffff888009179000
which belongs to the cache kmalloc-256 of size 256
The buggy address is located 0 bytes inside of
allocated 256-byte region [ffff888009179000, ffff888009179100)
Memory state around the buggy address:
ffff888009179000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff888009179080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff888009179100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
^
ffff888009179180: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff888009179200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
This is a regression from commit 5e013ad20689 ("HID: wacom: Remove
static WACOM_PKGLEN_MAX limit"), first present in v6.15-rc1. Before
that commit, wacom_raw_event() rejected reports exceeding
WACOM_PKGLEN_MAX (361 bytes) and the kfifo was sized at 512 bytes
(361 rounded up). After the commit, the size cap was removed and the
kfifo is dynamically sized as min(PAGE_SIZE, 10 * pktlen), which can
be as small as 256 bytes.
wacom_wac_queue_insert() passes the report size directly to kfifo_in()
without validating that it fits. When a UHID_INPUT2 event delivers a
report up to 4096 bytes (UHID_DATA_MAX), kfifo_copy_in() writes up to
3840 bytes past the end of the kmalloc-256 slab object.
The fix adds a bounds check in wacom_wac_queue_insert() to reject
reports that exceed the kfifo capacity.
Thanks,
Jinmo
Jinmo Yang (1):
HID: wacom: validate report size before kfifo insert
drivers/hid/wacom_sys.c | 6 ++++++
1 file changed, 6 insertions(+)
--
2.53.0
^ permalink raw reply
* Re: [PATCH] HID: core: Fix bsize printk specifier in hid_report_raw_event()
From: kernel test robot @ 2026-05-24 1:03 UTC (permalink / raw)
To: yaolu, jikos, bentiss
Cc: oe-kbuild-all, linux-input, linux-kernel, Lu Yao, k2ci
In-Reply-To: <20260522072718.186707-1-yaolu@kylinos.cn>
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on hid/for-next]
[also build test WARNING on v7.1-rc4]
[cannot apply to linus/master next-20260522]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/yaolu-kylinos-cn/HID-core-Fix-bsize-printk-specifier-in-hid_report_raw_event/20260522-152828
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20260522072718.186707-1-yaolu%40kylinos.cn
patch subject: [PATCH] HID: core: Fix bsize printk specifier in hid_report_raw_event()
config: alpha-defconfig (https://download.01.org/0day-ci/archive/20260524/202605240823.YKX23Xjr-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260524/202605240823.YKX23Xjr-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605240823.YKX23Xjr-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:15,
from include/linux/input.h:19,
from drivers/hid/hid-core.c:25:
drivers/hid/hid-core.c: In function 'hid_report_raw_event':
>> drivers/hid/hid-core.c:2053:43: warning: format '%u' expects argument of type 'unsigned int', but argument 5 has type 'size_t' {aka 'long unsigned int'} [-Wformat=]
2053 | hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %u)\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:156:61: note: in expansion of macro 'dev_fmt'
156 | dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
include/linux/dev_printk.h:215:17: note: in expansion of macro 'dev_warn'
215 | dev_level(dev, fmt, ##__VA_ARGS__); \
| ^~~~~~~~~
include/linux/dev_printk.h:227:9: note: in expansion of macro 'dev_level_ratelimited'
227 | dev_level_ratelimited(dev_warn, dev, fmt, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~
include/linux/hid.h:1340:9: note: in expansion of macro 'dev_warn_ratelimited'
1340 | dev_warn_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~
drivers/hid/hid-core.c:2053:17: note: in expansion of macro 'hid_warn_ratelimited'
2053 | hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %u)\n",
| ^~~~~~~~~~~~~~~~~~~~
drivers/hid/hid-core.c:2053:90: note: format string is defined here
2053 | hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %u)\n",
| ~^
| |
| unsigned int
| %lu
drivers/hid/hid-core.c:2075:43: warning: format '%u' expects argument of type 'unsigned int', but argument 5 has type 'size_t' {aka 'long unsigned int'} [-Wformat=]
2075 | hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %u)\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:156:61: note: in expansion of macro 'dev_fmt'
156 | dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
include/linux/dev_printk.h:215:17: note: in expansion of macro 'dev_warn'
215 | dev_level(dev, fmt, ##__VA_ARGS__); \
| ^~~~~~~~~
include/linux/dev_printk.h:227:9: note: in expansion of macro 'dev_level_ratelimited'
227 | dev_level_ratelimited(dev_warn, dev, fmt, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~
include/linux/hid.h:1340:9: note: in expansion of macro 'dev_warn_ratelimited'
1340 | dev_warn_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~
drivers/hid/hid-core.c:2075:17: note: in expansion of macro 'hid_warn_ratelimited'
2075 | hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %u)\n",
| ^~~~~~~~~~~~~~~~~~~~
drivers/hid/hid-core.c:2075:91: note: format string is defined here
2075 | hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %u)\n",
| ~^
| |
| unsigned int
| %lu
vim +2053 drivers/hid/hid-core.c
2035
2036 int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *data,
2037 size_t bufsize, u32 size, int interrupt)
2038 {
2039 struct hid_report_enum *report_enum = hid->report_enum + type;
2040 struct hid_report *report;
2041 struct hid_driver *hdrv;
2042 int max_buffer_size = HID_MAX_BUFFER_SIZE;
2043 u32 rsize, csize = size;
2044 size_t bsize = bufsize;
2045 u8 *cdata = data;
2046 int ret = 0;
2047
2048 report = hid_get_report(report_enum, data);
2049 if (!report)
2050 return 0;
2051
2052 if (unlikely(bsize < csize)) {
> 2053 hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %u)\n",
2054 report->id, csize, bsize);
2055 return -EINVAL;
2056 }
2057
2058 if (report_enum->numbered) {
2059 cdata++;
2060 csize--;
2061 bsize--;
2062 }
2063
2064 rsize = hid_compute_report_size(report);
2065
2066 if (hid->ll_driver->max_buffer_size)
2067 max_buffer_size = hid->ll_driver->max_buffer_size;
2068
2069 if (report_enum->numbered && rsize >= max_buffer_size)
2070 rsize = max_buffer_size - 1;
2071 else if (rsize > max_buffer_size)
2072 rsize = max_buffer_size;
2073
2074 if (bsize < rsize) {
2075 hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %u)\n",
2076 report->id, rsize, bsize);
2077 return -EINVAL;
2078 }
2079
2080 if (csize < rsize) {
2081 dbg_hid("report %d is too short, (%d < %d)\n", report->id,
2082 csize, rsize);
2083 memset(cdata + csize, 0, rsize - csize);
2084 }
2085
2086 if ((hid->claimed & HID_CLAIMED_HIDDEV) && hid->hiddev_report_event)
2087 hid->hiddev_report_event(hid, report);
2088 if (hid->claimed & HID_CLAIMED_HIDRAW) {
2089 ret = hidraw_report_event(hid, data, size);
2090 if (ret)
2091 return ret;
2092 }
2093
2094 if (hid->claimed != HID_CLAIMED_HIDRAW && report->maxfield) {
2095 hid_process_report(hid, report, cdata, interrupt);
2096 hdrv = hid->driver;
2097 if (hdrv && hdrv->report)
2098 hdrv->report(hid, report);
2099 }
2100
2101 if (hid->claimed & HID_CLAIMED_INPUT)
2102 hidinput_report_event(hid, report);
2103
2104 return ret;
2105 }
2106 EXPORT_SYMBOL_GPL(hid_report_raw_event);
2107
2108
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [syzbot] [input?] possible deadlock in tasklet_action_common (2)
From: Hillf Danton @ 2026-05-24 0:45 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: syzbot, dmitry.torokhov, linux-input, Tetsuo Handa, linux-kernel,
syzkaller-bugs
In-Reply-To: <20260522073508.VfBubDl8@linutronix.de>
On Fri, 22 May 2026 09:35:08 +0200 Sebastian Andrzej Siewior wrote:
> On 2026-05-22 15:21:43 [+0800], Hillf Danton wrote:
> > > > On RT spinlock is replaced with mutex, and softirq can be raised in the
> > > > irq that could come any moment after spin_lock_irqsave().
> > >
> > > That is true on the other hand. That means having raised another tasklet
> > > could lead to the backtrace. But it would have been two different locks,
> > > not blocking on each other.
> > >
> > The last question, by two different locks, do you mean that the
> > tasklet_sync_callback.cb_lock is per cpu?
>
> Yes, it is but this does not matter. kbd_led_trigger_activate() does
> tasklet_disable(&keyboard_tasklet) so you can't have kbd_bh() running
> and led_set_brightness() which would kick the softirq again. Not from
> kbd_led_trigger_activate().
>
> Even if another component would raise a softirq in that window, you
> could run tasklets, yes, but never kbd_bh(). Not in this window.
>
After a look at fd4e876f59b7 ("softirq: Provide a handshake for canceling
tasklets via polling") add a shameless question (obvious the above one is
not last), any chance for cutting tasklet_sync_callback off if wait_on_bit()
for TASKLET_STATE_RUN is used in tasklet_unlock_spin_wait()?
^ permalink raw reply
* Re: [PATCH v5 1/3] HID: nintendo: Add preliminary Switch 2 controller driver
From: Silvan Jegen @ 2026-05-23 8:57 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Vicki Pfau, linux-input, dmitry.torokhov
In-Reply-To: <20260514001510.964E7C19425@smtp.kernel.org>
Heyhey!
Below some comments regarding the findings (note that I am not an expert
on neither Linux kernel drivers in general nor USB HID specifically).
sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider:
> - [High] Sleeping in atomic context and AB-BA deadlock in `switch2_usb_disconnect`
> - [High] Race condition in IN URB tracking causes permanent starvation
> - [High] Dangling pointers and use-after-free on probe failure
> - [High] Memory corruption and double free due to incorrect devm input lifecycle management
> - [High] NULL pointer dereference in USB probe error path
> - [High] Division by zero crash from unvalidated trigger calibration data
> - [High] BUG_ON kernel panic when `ida_free` is called with U32_MAX
> - [High] Out-of-bounds read when parsing NS2_CMD_FW_INFO response
> - [High] Race condition due to URB submission before initialization of synchronization primitives
> - [Medium] Infinite USB retry storm on protocol errors
> - [Medium] Unaligned memory access on byte array casts
> --
>
> commit 00cb7265ad5983144c6923702e2ee2c3c047d29e
> Author: Vicki Pfau <vi@endrift.com>
> Subject: HID: nintendo: Add preliminary Switch 2 controller driver
>
> This adds a new driver for the Switch 2 controllers. The Switch 2 uses an
> unusual split-interface design such that input and rumble occur on the main
> HID interface, but all other communication occurs over a "configuration"
> interface.
>
> > diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> > index 29008c2cc5304..2193afcdefabc 100644
> > --- a/drivers/hid/hid-nintendo.c
> > +++ b/drivers/hid/hid-nintendo.c
> [ ... ]
> > +static int switch2_init_input(struct switch2_controller *ns2)
> > +{
> [ ... ]
> > + input = devm_input_allocate_device(&hdev->dev);
> > + if (!input)
> > + return -ENOMEM;
> [ ... ]
> > + ret = input_register_device(input);
> > + if (ret < 0) {
> > + hid_err(ns2->hdev, "Failed to register input; ret=%d\n", ret);
> > + input_free_device(input);
>
> Calling input_free_device or input_unregister_device on a devm-managed
> input device can cause double frees when devres cleanup fires. Can this
> lead to memory corruption?
I think sashiko is actually correct here, unfortunately. The documentation
for devm_input_allocate_device states that
Managed input devices do not need to be explicitly unregistered or
freed as it will be done automatically when owner device unbinds from
its driver (or binding fails). ...
Looks like we can remove all the input_unregister_device and
input_free_device calls here.
>
> > + return ret;
> > + }
> [ ... ]
> > +static void switch2_controller_put(struct switch2_controller *ns2)
> > +{
> > + struct input_dev *input;
> > + bool do_free;
> [ ... ]
> > + if (input)
> > + input_unregister_device(input);
>
> Calling input_unregister_device on a devm-managed input device can cause
> issues. If the input device remains alive (tied to the HID device devres)
> and its child LEDs are not released, will reconnecting the USB cable
> allocate a new input device and corrupt the LED lists by re-registering
> the exact same ns2->leds structures?
This input_unregister_device can also be removed.
>
> > +
> > + if (do_free) {
> [ ... ]
> > +static void switch2_report_trigger(struct input_dev *input, uint8_t zero, int abs, uint8_t data)
> > +{
> > + int value = (NS2_TRIGGER_RANGE + 1) * (data - zero) / (232 - zero);
>
> The zero variable is loaded directly from the controller's factory calibration
> flash. If a device reports a zero point of exactly 232, will this trigger a
> division by zero exception and crash the kernel?
Not sure this is an issue in practice, but we might want to protect the
code in any case by doing something like the below perhaps (untested).
int value = (NS2_TRIGGER_RANGE + 1) * (data - zero) / max_t(int, (232 - zero), 1);
>
> > +
> > + input_report_abs(input, abs, clamp(value, 0, NS2_TRIGGER_RANGE));
> > +}
> [ ... ]
> > +static int switch2_read_flash(struct switch2_controller *ns2, uint32_t address,
> > + uint8_t size)
> > +{
> > + uint8_t message[8] = { size, 0x7e };
> > +
> > + if (!ns2->cfg)
> > + return -ENOTCONN;
> > + *(__le32 *)&message[4] = __cpu_to_le32(address);
>
> The driver directly casts &message[4] to __le32. Since message is a uint8_t
> array, it is only guaranteed to be 1-byte aligned. On strict architectures,
> will this unaligned memory write trigger a hardware alignment fault?
I assume this could be an issue and we should just use something like
the below (only compile-tested).
put_unaligned_le32(__cpu_to_le32(address), &message[4]);
>
> > + return ns2->cfg->send_command(NS2_CMD_FLASH, NS2_SUBCMD_FLASH_READ, message,
> > + sizeof(message), ns2->cfg);
> > +}
> [ ... ]
> > +int switch2_receive_command(struct switch2_controller *ns2,
> > + const uint8_t *message, size_t length)
> > +{
> [ ... ]
> > + header = (const struct switch2_cmd_header *)message;
> > + if (!(header->flags & NS2_FLAG_OK)) {
> > + ret = -EIO;
> > + goto exit;
> > + }
> > + message = &message[8];
> > + switch (header->command) {
> [ ... ]
> > + case NS2_CMD_FW_INFO:
> > + if (header->subcommand == NS2_SUBCMD_FW_INFO_GET) {
> > + if (length < sizeof(ns2->version)) {
> > + ret = -EINVAL;
> > + goto exit;
> > + }
> > + memcpy(&ns2->version, message, sizeof(ns2->version));
>
> The message pointer is advanced by 8 bytes to skip the header, but length
> is not decremented. The bounds check compares length against
> sizeof(ns2->version). If length is exactly sizeof(ns2->version), the check
> passes, but the advanced message pointer only has length - 8 valid bytes
> remaining. Will the memcpy over-read into uninitialized memory?
Sounds like something that we should fix by either decreasing length or
increasing the size in the check.
>
> > + ns2->ctlr_type = ns2->version.ctlr_type;
> > + switch2_init_step_done(ns2, NS2_INIT_GET_FIRMWARE_INFO);
> > + }
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > +exit:
> > + if (ns2->init_step < NS2_INIT_DONE)
> > + switch2_init_controller(ns2);
>
> If a command response has the NACK flag set or encounters another error,
> the code jumps to exit and invokes switch2_init_controller. Since
> ns2->init_step has not advanced, the state machine will immediately re-send
> the exact same command. Can this cause an infinite tight feedback loop of
> failures and retries, flooding the USB bus?
We have discussed this before and it doesn't seem to be an issue in
practice.
If we want to be on the safe side, we could pass a retry count and abort
on too many error retries, perhaps?
> > +
> > + return ret;
> > +}
> [ ... ]
> > +int switch2_controller_attach_cfg(const char *phys, struct switch2_cfg_intf *cfg)
> > +{
> [ ... ]
> > + guard(mutex)(&ns2->lock);
> > + WARN_ON(ns2->cfg);
> > + ns2->cfg = cfg;
> > +
> > + if (ns2->hdev)
> > + return switch2_init_controller(ns2);
>
> If switch2_init_controller fails here or in switch2_probe, the error is
> returned immediately without unrolling the state. Will ns2->cfg or ns2->hdev
> remain set, potentially leading to a use-after-free when subsequent device
> events occur?
AFAICT, since we are always checking for both ns2->cfg and ns2->hdev
before doing any processing in switch2_init_controller, there should
never be a use-after-free
It *is* true that we don't call hid_hw_{close,stop} in switch2_probe,
if the switch2_init_controller call fails though. It might make sense
to return a specific error and use it to check whether we want to give
up and call these?
>
> > + return 0;
> > +}
> [ ... ]
> > +static void switch2_remove(struct hid_device *hdev)
> > +{
> > + struct switch2_controller *ns2 = hid_get_drvdata(hdev);
> > +
> > + hid_hw_close(hdev);
> > + mutex_lock(&ns2->lock);
> > + WARN_ON(ns2->hdev != hdev);
> > + ns2->hdev = NULL;
> > + mutex_unlock(&ns2->lock);
> > + ida_free(&nintendo_player_id_allocator, ns2->player_id);
>
> If ida_alloc fails during probe, ns2->player_id remains at its default
> value of U32_MAX. When ida_free is called, the IDA framework enforces a
> check on the ID. Casting U32_MAX to a signed integer yields -1. Will this
> trigger the BUG_ON and cause a kernel panic?
ida_free seems to return early on
if ((int)id < 0)
return;
so that should be fine as is.
>
> > + switch2_controller_put(ns2);
> > + hid_hw_stop(hdev);
> > +}
> [ ... ]
> > diff --git a/drivers/input/joystick/nintendo-switch2-usb.c b/drivers/input/joystick/nintendo-switch2-usb.c
> > new file mode 100644
> > index 0000000000000..ebd89d852e21a
> > --- /dev/null
> > +++ b/drivers/input/joystick/nintendo-switch2-usb.c
> [ ... ]
> > +static void switch2_bulk_in(struct urb *urb)
> > +{
> [ ... ]
> > + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
> > + for (i = 0; i < NS2_IN_URBS; i++) {
> > + int err;
> > + struct switch2_urb *ns2_urb;
> > +
> > + if (ns2_usb->bulk_in[i].urb == urb) {
> > + ns2_usb->message_in = i;
>
> If multiple URBs complete in quick succession before the work item executes,
> the message_in index is overwritten. Will this cause the work item to only
> process the last completed URB, leaving earlier URBs indefinitely active
> and leading to starvation of the URB pool?
>
> > + continue;
> > + }
> [ ... ]
> > +static int switch2_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
> > +{
> [ ... ]
> > + ns2_usb->bulk_in[0].active = true;
> > + ret = usb_submit_urb(ns2_usb->bulk_in[0].urb, GFP_ATOMIC);
> > + if (ret < 0)
> > + goto err_free_out;
> > +
> > + init_usb_anchor(&ns2_usb->bulk_out_anchor);
> > + spin_lock_init(&ns2_usb->bulk_out_lock);
> > + init_usb_anchor(&ns2_usb->bulk_in_anchor);
> > + spin_lock_init(&ns2_usb->bulk_in_lock);
> > + INIT_WORK(&ns2_usb->message_in_work, switch2_usb_message_in_work);
>
> usb_submit_urb is invoked for bulk_in[0] before the initialization of the
> spinlocks, anchors, and work struct. If the URB completes immediately, the
> completion handler will execute and attempt to use uninitialized
> synchronization primitives. Can this cause a crash?
As the documentation for usb_submit_urb mentions that the call is async,
I assume that this is indeed a theoretical possibility.
>
> > +
> > + usb_set_intfdata(intf, ns2_usb);
> [ ... ]
> > +err_kill_urb:
> > + usb_kill_urb(ns2_usb->bulk_in[0].urb);
> > +err_free_out:
> > + for (i = 0; i < NS2_OUT_URBS; i++) {
> > + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_out[i].data,
> > + ns2_usb->bulk_out[i].urb->transfer_dma);
> > + usb_free_urb(ns2_usb->bulk_out[i].urb);
> > + }
> > +err_free_in:
> > + for (i = 0; i < NS2_IN_URBS; i++) {
> > + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_in[i].data,
> > + ns2_usb->bulk_in[i].urb->transfer_dma);
>
> If usb_alloc_urb fails during the loop, it jumps to err_free_in. During
> cleanup, it accesses ns2_usb->bulk_in[i].urb->transfer_dma. Since urb is
> NULL for the failed index, will this trigger a NULL pointer dereference?
Yes, most likely we would need a new label for this case.
>
> > + usb_free_urb(ns2_usb->bulk_in[i].urb);
> > + }
> > + devm_kfree(&intf->dev, ns2_usb);
> > +
> > + return ret;
> > +}
> > +
> > +static void switch2_usb_disconnect(struct usb_interface *intf)
> > +{
> > + struct switch2_usb *ns2_usb = usb_get_intfdata(intf);
> > + unsigned long flags;
> > + int i;
> > +
> > + spin_lock_irqsave(&ns2_usb->bulk_out_lock, flags);
> > + usb_kill_anchored_urbs(&ns2_usb->bulk_out_anchor);
>
> Both usb_kill_anchored_urbs and cancel_work_sync can sleep. Is it safe to
> call them while holding a spinlock?
According to some googling, this indeed seems to be an isuse. The
non-sleeping way to cancel transfer requests would be to use
https://www.kernel.org/doc/html/v6.1/driver-api/usb/usb.html#c.usb_unlink_anchored_urbs
>
> > + for (i = 0; i < NS2_OUT_URBS; i++) {
> > + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_out[i].data,
> > + ns2_usb->bulk_out[i].urb->transfer_dma);
> > + usb_free_urb(ns2_usb->bulk_out[i].urb);
> > + }
> > + spin_unlock_irqrestore(&ns2_usb->bulk_out_lock, flags);
> > +
> > + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
> > + usb_kill_anchored_urbs(&ns2_usb->bulk_in_anchor);
> > + cancel_work_sync(&ns2_usb->message_in_work);
>
> Furthermore, cancel_work_sync waits for message_in_work which attempts to
> acquire bulk_in_lock. Does this cause a deadlock since the lock is already
> held by the disconnecting thread?
Yes, this looks like a real concern as well.
Cheers,
Silvan
>
> > + for (i = 0; i < NS2_IN_URBS; i++) {
^ permalink raw reply
* Re: [PATCH v2 2/2] Input: atmel_mxt_ts: Allow per-board config via DT compatible
From: sashiko-bot @ 2026-05-23 16:18 UTC (permalink / raw)
To: Hendrik Noack; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260523152332.86864-2-hendrik-noack@gmx.de>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The OF device node reference for the root node is leaked when falling back to the generic configuration loader.
- [Medium] Mismatch between the constructed firmware filename and the filename documented in the commit message.
--
commit e3439c3fb5cf6c8c740fae5a27659ea3cb9e7436
Author: Hendrik Noack <hendrik-noack@gmx.de>
Input: atmel_mxt_ts: Allow per-board config via DT compatible
This commit modifies the Atmel maXTouch driver to load a board-specific
configuration file based on the device tree compatible string. If the specific
configuration file is not found, it falls back to the original generic
configuration behavior.
> Extend the driver to support per-board config files keyed by the device
> tree compatible string. For DT-based systems, the driver now looks up the
> primary compatible of the board and searches for a config file named:
>
> atmel/maxtouch,<compatible>.cfg
[Severity: Medium]
The documentation in the commit message uses a comma before the compatible
string, but the code implementation constructs the filename using a dot.
Is it intended to use a dot here, and if so, could the commit message
be updated to match the code?
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 07690c3bff06e..a9e9f151d0424 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
[ ... ]
> @@ -2233,6 +2235,64 @@ static void mxt_config_cb(const struct firmware *cfg, void *ctx)
[ ... ]
> +static int mxt_invoke_config_loader(struct mxt_data *data, bool device_specific)
> +{
> + struct device_node *root;
> + char *board_type = NULL;
> + char *fw_name;
> + void (*cb)(const struct firmware *fw, void *context);
> + int error;
> +
> + root = of_find_node_by_path("/");
> + if (device_specific && root) {
> + const char *tmp;
> +
> + if (!of_property_read_string_index(root, "compatible", 0, &tmp))
> + board_type = kstrdup(tmp, GFP_KERNEL);
> +
> + of_node_put(root);
> + }
[Severity: Medium]
Does this leak the root device node reference when device_specific is false?
The call to of_find_node_by_path() unconditionally acquires a reference,
but of_node_put() is only called inside the if block. The fallback
configuration path triggered from mxt_board_config_cb() passes false
for device_specific, which would bypass the release.
> +
> + if (board_type) {
> + /* get rid of '/' in the compatible string to be able to find the FW */
> + strreplace(board_type, '/', '-');
> +
> + fw_name = kasprintf(GFP_KERNEL, "%s/%s.%s%s", MXT_CFG_FOLDER, MXT_CFG_NAME,
> + board_type, MXT_CFG_EXTENSION);
[Severity: Medium]
Does this format string construct the filename with a dot instead of the comma
described in the commit message?
This format string expands to atmel/maxtouch.<compatible>.cfg which might lead
to unexpected firmware loading failures for users following the naming scheme
documented in the commit.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260523152332.86864-1-hendrik-noack@gmx.de?part=2
^ permalink raw reply
* Re: [PATCH v2 1/2] Input: atmel_mxt_ts: Fix async config use-after-free
From: sashiko-bot @ 2026-05-23 16:01 UTC (permalink / raw)
To: Hendrik Noack; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260523152332.86864-1-hendrik-noack@gmx.de>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [Critical] The newly added `config_lock` mutex in `struct mxt_data` is used without being initialized.
- [High] If `mxt_initialize()` aborts due to `shutting_down == true`, it consumes the completion but fails to restore it, leading to a permanent hang.
- [High] The `config_lock` mutex is held across a potentially unbounded wait, defeating the purpose of the `shutting_down` flag.
- [Medium] A time-of-check to time-of-use (TOCTOU) race allows asynchronous firmware loading to start after device removal has begun.
Pre-existing issues:
- [High] Concurrent sysfs writes to the `update_fw` attribute can trigger a double free of the input device.
--
commit a6627582981aef52fbfe146ee37db19f67d402dd
Author: Hendrik Noack <hendrik-noack@gmx.de>
Input: atmel_mxt_ts: Fix async config use-after-free
This commit attempts to fix a use-after-free by adding a mutex and a
completion to serialize the asynchronous firmware loading and the device
removal process.
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index a9e86ad7ed5ef..07690c3bff06e 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
[ ... ]
> @@ -2271,12 +2279,22 @@ static int mxt_initialize(struct mxt_data *data)
> if (error)
> return error;
>
> + scoped_guard(mutex, &data->config_lock) {
> + wait_for_completion(&data->config_completion);
[Severity: High]
Is it safe to hold the config_lock mutex across this wait_for_completion()
call?
Since the lock is held while waiting for a potentially long firmware load,
and mxt_remove() needs this same lock to set shutting_down to true, would
this cause mxt_remove() to block indefinitely, defeating the early shutdown
check?
> +
> + if (data->shutting_down)
> + return -EBUSY;
[Severity: High]
If shutting_down is true, the wait_for_completion() above has already
decremented the completion's counter.
Returning -EBUSY here without calling complete(&data->config_completion)
leaves the completion consumed. When mxt_remove() later calls
wait_for_completion(), will it hang forever waiting for a completion that
is never restored?
> +
> + reinit_completion(&data->config_completion);
> + }
> +
> error = request_firmware_nowait(THIS_MODULE, true, MXT_CFG_NAME,
> &client->dev, GFP_KERNEL, data,
> mxt_config_cb);
[Severity: Medium]
Because request_firmware_nowait() is called after the config_lock is
released, is there a race condition here?
If mxt_remove() runs immediately after the lock is dropped, it will acquire
the lock and set shutting_down to true, but request_firmware_nowait() will
still be invoked, starting a new asynchronous load while the device is
actively being removed.
[ ... ]
> @@ -3237,6 +3255,9 @@ static int mxt_probe(struct i2c_client *client)
> init_completion(&data->bl_completion);
> init_completion(&data->reset_completion);
> init_completion(&data->crc_completion);
> + init_completion(&data->config_completion);
> + complete(&data->config_completion);
> + data->shutting_down = false;
[Severity: Critical]
Does the config_lock mutex need to be initialized here?
The mxt_data structure is allocated with devm_kzalloc(), which
zero-initializes the memory. Using an uninitialized mutex with
scoped_guard() will cause a NULL pointer dereference when contention
occurs. Should there be a call to mutex_init(&data->config_lock)?
[ ... ]
> @@ -3342,6 +3363,12 @@ static void mxt_remove(struct i2c_client *client)
> {
> struct mxt_data *data = i2c_get_clientdata(client);
>
> + scoped_guard(mutex, &data->config_lock) {
> + data->shutting_down = true;
> + }
> +
> + wait_for_completion(&data->config_completion);
> +
> disable_irq(data->irq);
> mxt_free_input_device(data);
[Severity: High]
This is a pre-existing issue, but looking at mxt_free_input_device(), it
checks if data->input_dev is non-NULL and then unregisters it.
If concurrent sysfs writes to the update_fw attribute occur, multiple
threads can enter mxt_load_fw() and call mxt_free_input_device() at the
same time. Since sysfs doesn't serialize these writes and there is no
internal locking, could both threads pass the NULL check before either
sets it to NULL, resulting in a use-after-free and double-free of the
input device?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260523152332.86864-1-hendrik-noack@gmx.de?part=1
^ permalink raw reply
* Re: [PATCH v2] HID: wacom: fix NULL pointer dereference in wacom_intuos_pad()
From: sashiko-bot @ 2026-05-23 15:50 UTC (permalink / raw)
To: Jinmo Yang; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260523150619.615565-1-jinmo44.yang@gmail.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] The patch's early return in `wacom_intuos_pad()` causes valid pad reports to fall through to pen handlers when `pad_input` is NULL.
Pre-existing issues:
- [High] The patch addresses a NULL pointer dereference for `pad_input` but completely misses identical vulnerabilities for `pen_input` and `touch_input`.
- [High] Concurrent execution of `wacom_mode_change_work()` and `wacom_raw_event()` can lead to a Use-After-Free of input devices.
- [High] A dangling pointer in `wacom->shared->touch_input` leads to Use-After-Free when the Touch interface unbinds while the Pad interface remains active.
--
commit e74a772fd06994c14eb8d49710acd3ad8740df1f
Author: Jinmo Yang <jinmo44.yang@gmail.com>
HID: wacom: fix NULL pointer dereference in wacom_intuos_pad()
This commit adds a NULL check for pad_input at the top of
wacom_intuos_pad() to bail out early when the pad input device was not set
up, preventing a general protection fault.
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index da1f0ea85625d..251ddda3ecb3d 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -515,7 +515,6 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
> struct wacom_features *features = &wacom->features;
> unsigned char *data = wacom->data;
> struct input_dev *input = wacom->pad_input;
> - int i;
> int buttons = 0, nbuttons = features->numbered_buttons;
> int keys = 0, nkeys = 0;
> int ring1 = 0, ring2 = 0;
> @@ -523,6 +522,10 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
> bool prox = false;
> bool wrench = false, keyboard = false, mute_touch = false, menu = false,
> info = false;
> + int i;
> +
> + if (!input)
> + return 0;
[Severity: High]
Does returning 0 here allow the event to fall through to other handlers?
wacom_intuos_irq() relies on a non-zero return to halt processing. If this
returns 0, a crafted pad packet might fall through to wacom_intuos_inout().
Since wacom_intuos_inout() only checks data[1] and not the report ID, it
could misinterpret a pad report as a pen proximity event. For example, if
data[1] == 0x80, could it trigger wacom_exit_report() and either inject a
spurious pen exit event or cause a NULL pointer dereference if pen_input
is also NULL?
Would it be better to place this check after verifying the report ID, and
return 1 to securely drop the packet?
[Severity: High]
This isn't a bug introduced by this patch, but does the same NULL pointer
vulnerability exist for pen_input and touch_input?
A crafted UHID descriptor can omit pen and touch capabilities, leaving
wacom->pen_input and wacom->touch_input as NULL.
If valid pen or touch packets are subsequently received, handlers like
wacom_exit_report(), wacom_intuos_general(), and wacom_bpt3_touch_msg()
appear to blindly dereference these NULL pointers.
[Severity: High]
This is a pre-existing issue, but can concurrent execution of
wacom_mode_change_work() and wacom_raw_event() lead to a use-after-free
of the input devices?
wacom_mode_change_work() frees input devices asynchronously via
wacom_release_resources() and devres_release_group(), without holding the
lock that serializes input report processing.
Concurrently, a raw input report could enter wacom_intuos_pad(), read the
wacom->pad_input pointer, successfully pass this new if (!input) check,
and then access the input device right after it gets freed.
>
> /* pad packets. Works as a second tool and is always in prox */
> if (!(data[0] == WACOM_REPORT_INTUOSPAD || data[0] == WACOM_REPORT_INTUOS5PAD ||
[Severity: High]
This is a pre-existing issue, but can wacom->shared->touch_input become a
dangling pointer if the touch interface unbinds while the pad interface
remains active?
When a touch interface is unbound, its input devices are freed via
devres_release_group(), but wacom_remove_shared_data() does not appear to
set wacom->shared->touch_input to NULL.
If a pad event is received later, the pad event handler invokes
input_report_switch() and input_sync() on the dangling pointer:
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);
}
Could this result in a use-after-free?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260523150619.615565-1-jinmo44.yang@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH] HID: wacom: fix NULL pointer dereference in wacom_intuos_pad()
From: sashiko-bot @ 2026-05-23 15:45 UTC (permalink / raw)
To: Jinmo Yang; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260523150101.611473-1-jinmo44.yang@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The added early return `if (!input) return 0;` in `wacom_intuos_pad()` fails to consume unhandled pad packets, causing them to fall through and be misinterpreted as pen events.
Pre-existing issues:
- [High] Identical NULL pointer dereference vulnerabilities exist for `pen_input` and `touch_input` in other event handlers.
--
commit f0fe30c63ed06d22404bffeeaa7efc7e1da7f7c0
Author: Jinmo Yang <jinmo44.yang@gmail.com>
HID: wacom: fix NULL pointer dereference in wacom_intuos_pad()
This commit adds a NULL check for pad_input at the top of wacom_intuos_pad()
to bail out early when the pad input device was not set up. This prevents
a general protection fault in input_get_drvdata() when a Wacom tablet is
created via UHID with missing interfaces.
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index da1f0ea85625d..251ddda3ecb3d 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -523,6 +522,10 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
> bool prox = false;
> bool wrench = false, keyboard = false, mute_touch = false, menu = false,
> info = false;
> + int i;
> +
> + if (!input)
> + return 0;
[Severity: High]
Does returning 0 here instead of 1 when input is NULL cause the pad packet
to be treated as unhandled?
If wacom_intuos_irq() receives a 0, it falls through to wacom_intuos_inout().
Since wacom_intuos_inout() does not validate the report ID in data[0],
could a pad packet coincidentally matching proximity masks be incorrectly
processed as a pen event, causing spurious events and state corruption?
Should the !input check be moved after the data[0] check, and return 1
to consume and discard the unhandled packet?
[Severity: High]
This is a pre-existing issue, but are there identical NULL pointer dereference
vulnerabilities for wacom->pen_input and wacom->touch_input?
If an attacker uses /dev/uhid to create a virtual device without PEN or TOUCH
collections, pen_input and touch_input remain NULL.
If packets for these missing interfaces are sent, will handlers like
wacom_bpt_touch(), wacom_bpt_pen(), or wacom_intuos_inout() unconditionally
dereference these NULL pointers during input_event() calls or when reporting
out-of-proximity events?
>
> /* pad packets. Works as a second tool and is always in prox */
> if (!(data[0] == WACOM_REPORT_INTUOSPAD || data[0] == WACOM_REPORT_INTUOS5PAD ||
> data[0] == WACOM_REPORT_CINTIQPAD))
> return 0;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260523150101.611473-1-jinmo44.yang@gmail.com?part=1
^ permalink raw reply
* [PATCH v2 2/2] Input: atmel_mxt_ts: Allow per-board config via DT compatible
From: Hendrik Noack @ 2026-05-23 15:23 UTC (permalink / raw)
To: Nick Dyer, Dmitry Torokhov; +Cc: Hendrik Noack, linux-input, linux-kernel
In-Reply-To: <20260523152332.86864-1-hendrik-noack@gmx.de>
The Atmel maXTouch config is board-dependent: different panels and layouts
require different configs even when the same controller and firmware is
used.
Extend the driver to support per-board config files keyed by the device
tree compatible string. For DT-based systems, the driver now looks up the
primary compatible of the board and searches for a config file named:
atmel/maxtouch,<compatible>.cfg
If such a file exists, it is used as the board-specific config. If the
system is not DT-based or if the per-board file is not present, the driver
reverts to the original behaviour and searches for a config file named:
maxtouch.cfg
Signed-off-by: Hendrik Noack <hendrik-noack@gmx.de>
---
Changes in v2:
- fix leak of fw_name (thanks sashiko.dev)
- adopt to added patch
---
drivers/input/touchscreen/atmel_mxt_ts.c | 68 ++++++++++++++++++++++--
1 file changed, 63 insertions(+), 5 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 07690c3bff06..a9e9f151d042 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -36,7 +36,9 @@
/* Firmware files */
#define MXT_FW_NAME "maxtouch.fw"
-#define MXT_CFG_NAME "maxtouch.cfg"
+#define MXT_CFG_FOLDER "atmel"
+#define MXT_CFG_NAME "maxtouch"
+#define MXT_CFG_EXTENSION ".cfg"
#define MXT_CFG_MAGIC "OBP_RAW V1"
/* Registers */
@@ -2233,6 +2235,64 @@ static void mxt_config_cb(const struct firmware *cfg, void *ctx)
complete(&data->config_completion);
}
+static int mxt_invoke_config_loader(struct mxt_data *data, bool device_specific);
+
+static void mxt_board_config_cb(const struct firmware *cfg, void *ctx)
+{
+ if (!cfg) {
+ struct mxt_data *data = ctx;
+ int error;
+
+ error = mxt_invoke_config_loader(data, false);
+ /* request_firmware_nowait() succeeded and mxt_config_cb() will be called at end */
+ if (!error)
+ return;
+
+ dev_err(&data->client->dev, "Failed to invoke general config loader: %d\n", error);
+ }
+
+ mxt_config_cb(cfg, ctx);
+}
+
+static int mxt_invoke_config_loader(struct mxt_data *data, bool device_specific)
+{
+ struct device_node *root;
+ char *board_type = NULL;
+ char *fw_name;
+ void (*cb)(const struct firmware *fw, void *context);
+ int error;
+
+ root = of_find_node_by_path("/");
+ if (device_specific && root) {
+ const char *tmp;
+
+ if (!of_property_read_string_index(root, "compatible", 0, &tmp))
+ board_type = kstrdup(tmp, GFP_KERNEL);
+
+ of_node_put(root);
+ }
+
+ if (board_type) {
+ /* get rid of '/' in the compatible string to be able to find the FW */
+ strreplace(board_type, '/', '-');
+
+ fw_name = kasprintf(GFP_KERNEL, "%s/%s.%s%s", MXT_CFG_FOLDER, MXT_CFG_NAME,
+ board_type, MXT_CFG_EXTENSION);
+ cb = mxt_board_config_cb;
+ kfree(board_type);
+ } else {
+ fw_name = kasprintf(GFP_KERNEL, "%s%s", MXT_CFG_NAME, MXT_CFG_EXTENSION);
+ cb = mxt_config_cb;
+ }
+ if (!fw_name)
+ return -ENOMEM;
+
+ error = request_firmware_nowait(THIS_MODULE, true, fw_name, &data->client->dev, GFP_KERNEL,
+ data, cb);
+ kfree(fw_name);
+ return error;
+}
+
static int mxt_initialize(struct mxt_data *data)
{
struct i2c_client *client = data->client;
@@ -2288,11 +2348,9 @@ static int mxt_initialize(struct mxt_data *data)
reinit_completion(&data->config_completion);
}
- error = request_firmware_nowait(THIS_MODULE, true, MXT_CFG_NAME,
- &client->dev, GFP_KERNEL, data,
- mxt_config_cb);
+ error = mxt_invoke_config_loader(data, true);
if (error) {
- dev_err(&client->dev, "Failed to invoke firmware loader: %d\n",
+ dev_err(&client->dev, "Failed to invoke config loader: %d\n",
error);
complete(&data->config_completion);
return error;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/2] Input: atmel_mxt_ts: Fix async config use-after-free
From: Hendrik Noack @ 2026-05-23 15:23 UTC (permalink / raw)
To: Nick Dyer, Dmitry Torokhov; +Cc: Hendrik Noack, linux-input, linux-kernel
The Atmel maXTouch driver starts asynchronous config loading from both
probe and a sysfs trigger via request_firmware_nowait() and passes the
devm-managed struct mxt_data as the callback context. If the driver is
removed while an asynchronous request is still pending, devres will free
mxt_data before the firmware callback runs. When the callback eventually
executes, it dereferences the freed mxt_data pointer, leading to a
use-after-free.
This is resolved by introducing a lock and a completion that serialise the
request_firmware_nowait() and remove():
Before calling request_firmware_nowait(), a mutex guards the waiting and
reinitialisation of the completion. If remove() has started, the
asynchronous loading will no longer be started. The completion is
completed at the end of the callback or if request_firmware_nowait()
fails.
In mxt_remove(), the same mutex guards the start of remove(). Afterwards,
the completion of pending request_firmware_nowait() is awaited.
Signed-off-by: Hendrik Noack <hendrik-noack@gmx.de>
---
Changes in v2:
- this fix is added to the patch series (thanks sashiko.dev)
---
drivers/input/touchscreen/atmel_mxt_ts.c | 29 +++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index a9e86ad7ed5e..07690c3bff06 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -347,6 +347,11 @@ struct mxt_data {
/* for config update handling */
struct completion crc_completion;
+ /* for loading config */
+ struct completion config_completion;
+ struct mutex config_lock;
+ bool shutting_down;
+
u32 *t19_keymap;
unsigned int t19_num_keys;
@@ -2221,8 +2226,11 @@ static int mxt_configure_objects(struct mxt_data *data,
static void mxt_config_cb(const struct firmware *cfg, void *ctx)
{
- mxt_configure_objects(ctx, cfg);
+ struct mxt_data *data = ctx;
+
+ mxt_configure_objects(data, cfg);
release_firmware(cfg);
+ complete(&data->config_completion);
}
static int mxt_initialize(struct mxt_data *data)
@@ -2271,12 +2279,22 @@ static int mxt_initialize(struct mxt_data *data)
if (error)
return error;
+ scoped_guard(mutex, &data->config_lock) {
+ wait_for_completion(&data->config_completion);
+
+ if (data->shutting_down)
+ return -EBUSY;
+
+ reinit_completion(&data->config_completion);
+ }
+
error = request_firmware_nowait(THIS_MODULE, true, MXT_CFG_NAME,
&client->dev, GFP_KERNEL, data,
mxt_config_cb);
if (error) {
dev_err(&client->dev, "Failed to invoke firmware loader: %d\n",
error);
+ complete(&data->config_completion);
return error;
}
@@ -3237,6 +3255,9 @@ static int mxt_probe(struct i2c_client *client)
init_completion(&data->bl_completion);
init_completion(&data->reset_completion);
init_completion(&data->crc_completion);
+ init_completion(&data->config_completion);
+ complete(&data->config_completion);
+ data->shutting_down = false;
data->suspend_mode = dmi_check_system(chromebook_T9_suspend_dmi) ?
MXT_SUSPEND_T9_CTRL : MXT_SUSPEND_DEEP_SLEEP;
@@ -3342,6 +3363,12 @@ static void mxt_remove(struct i2c_client *client)
{
struct mxt_data *data = i2c_get_clientdata(client);
+ scoped_guard(mutex, &data->config_lock) {
+ data->shutting_down = true;
+ }
+
+ wait_for_completion(&data->config_completion);
+
disable_irq(data->irq);
mxt_free_input_device(data);
mxt_free_object_table(data);
--
2.43.0
^ permalink raw reply related
* [PATCH v2] HID: wacom: fix NULL pointer dereference in wacom_intuos_pad()
From: Jinmo Yang @ 2026-05-23 15:06 UTC (permalink / raw)
To: Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Jinmo Yang
In-Reply-To: <20260523150101.611473-1-jinmo44.yang@gmail.com>
wacom_intuos_pad() dereferences wacom->pad_input without a NULL check.
When a Wacom tablet is created via UHID with parameters that route
through wacom_bpt_irq() -> wacom_intuos_irq() -> wacom_intuos_pad(),
but probe did not allocate pad_input, the call to
wacom_report_numbered_buttons() passes a NULL input_dev, causing a
general protection fault in input_get_drvdata().
Add a NULL check for pad_input at the top of wacom_intuos_pad() to
bail out early when the pad input device was not set up.
The bug was found by syzkaller on linux-next 7.1.0-rc4 (next-20260522):
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000052
KASAN: null-ptr-deref in range [0x0000000000000290-0x0000000000000297]
RIP: 0010:input_get_drvdata include/linux/input.h:390 [inline]
RIP: 0010:wacom_report_numbered_buttons+0x37/0x210 drivers/hid/wacom_wac.c:4210
Call Trace:
wacom_intuos_pad drivers/hid/wacom_wac.c:643 [inline]
wacom_intuos_irq+0x29a/0x32b0 drivers/hid/wacom_wac.c:1042
wacom_bpt_irq drivers/hid/wacom_wac.c:3275 [inline]
wacom_wac_irq+0x12ed/0xaef0 drivers/hid/wacom_wac.c:3545
wacom_raw_event+0x677/0xa90 drivers/hid/wacom_sys.c:184
__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
Also confirmed on a Pixel 9 Pro (Android 16, kernel 6.1.124) where
it causes an immediate kernel panic and reboot via /dev/uhid without
requiring root privileges:
KP: Oops: Fatal exception: comm:wacom_27qhdt
Reboot reason: 0xbaba - Kernel PANIC
Reproducer (unprivileged):
open("/dev/uhid", O_RDWR)
write(fd, UHID_CREATE2{vendor=0x056a, product=0x0020})
write(fd, UHID_INPUT2{report_id=0x0c, size=10})
Fixes: c7f0522a1ad1 ("HID: wacom: Slim down wacom_intuos_pad processing")
Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
---
drivers/hid/wacom_wac.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85..251ddda3e 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -515,7 +515,6 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
struct wacom_features *features = &wacom->features;
unsigned char *data = wacom->data;
struct input_dev *input = wacom->pad_input;
- int i;
int buttons = 0, nbuttons = features->numbered_buttons;
int keys = 0, nkeys = 0;
int ring1 = 0, ring2 = 0;
@@ -523,6 +522,10 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
bool prox = false;
bool wrench = false, keyboard = false, mute_touch = false, menu = false,
info = false;
+ int i;
+
+ if (!input)
+ return 0;
/* pad packets. Works as a second tool and is always in prox */
if (!(data[0] == WACOM_REPORT_INTUOSPAD || data[0] == WACOM_REPORT_INTUOS5PAD ||
--
2.53.0
^ permalink raw reply related
* [PATCH] HID: wacom: fix NULL pointer dereference in wacom_intuos_pad()
From: Jinmo Yang @ 2026-05-23 15:01 UTC (permalink / raw)
To: Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Jinmo Yang
wacom_intuos_pad() dereferences wacom->pad_input without a NULL check.
When a Wacom tablet is created via UHID with parameters that route
through wacom_bpt_irq() -> wacom_intuos_irq() -> wacom_intuos_pad(),
but probe did not allocate pad_input, the call to
wacom_report_numbered_buttons() passes a NULL input_dev, causing a
general protection fault in input_get_drvdata().
Add a NULL check for pad_input at the top of wacom_intuos_pad() to
bail out early when the pad input device was not set up.
The bug was found by syzkaller and confirmed on a Pixel 9 Pro
(Android 16, kernel 6.1.124) where it causes an immediate kernel
panic and reboot via /dev/uhid without requiring root privileges:
KP: Oops: Fatal exception: comm:wacom_27qhdt
Reboot reason: 0xbaba - Kernel PANIC
Reproducer (unprivileged):
open("/dev/uhid", O_RDWR)
write(fd, UHID_CREATE2{vendor=0x056a, product=0x0020})
write(fd, UHID_INPUT2{report_id=0x0c, size=10})
Fixes: c7f0522a1ad1 ("HID: wacom: Slim down wacom_intuos_pad processing")
Signed-off-by: Jinmo Yang <jinmo44.yang@gmail.com>
---
drivers/hid/wacom_wac.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85..251ddda3e 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -515,7 +515,6 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
struct wacom_features *features = &wacom->features;
unsigned char *data = wacom->data;
struct input_dev *input = wacom->pad_input;
- int i;
int buttons = 0, nbuttons = features->numbered_buttons;
int keys = 0, nkeys = 0;
int ring1 = 0, ring2 = 0;
@@ -523,6 +522,10 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
bool prox = false;
bool wrench = false, keyboard = false, mute_touch = false, menu = false,
info = false;
+ int i;
+
+ if (!input)
+ return 0;
/* pad packets. Works as a second tool and is always in prox */
if (!(data[0] == WACOM_REPORT_INTUOSPAD || data[0] == WACOM_REPORT_INTUOS5PAD ||
--
2.53.0
^ permalink raw reply related
* [dtor-input:for-linus] BUILD SUCCESS dab48a7e74e6a394f3aa0461a2b1fb0c7b38fcb8
From: kernel test robot @ 2026-05-23 12:42 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
branch HEAD: dab48a7e74e6a394f3aa0461a2b1fb0c7b38fcb8 Input: ims-pcu - fix usb_free_coherent() size in ims_pcu_buffers_free()
elapsed time: 1166m
configs tested: 69
configs skipped: 6
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc-15.2.0
arc allmodconfig gcc-15.2.0
arc allnoconfig gcc-15.2.0
arc allyesconfig gcc-15.2.0
arm allnoconfig clang-23
arm allyesconfig gcc-15.2.0
arm64 allmodconfig clang-19
arm64 allnoconfig gcc-15.2.0
csky allmodconfig gcc-15.2.0
csky allnoconfig gcc-15.2.0
hexagon allnoconfig clang-23
hexagon randconfig-001-20260523 clang-23
hexagon randconfig-002-20260523 clang-23
i386 allmodconfig gcc-14
i386 allnoconfig gcc-14
i386 allyesconfig gcc-14
loongarch allmodconfig clang-19
loongarch allnoconfig clang-23
loongarch randconfig-001-20260523 gcc-15.2.0
loongarch randconfig-002-20260523 gcc-15.2.0
m68k allmodconfig gcc-15.2.0
m68k allnoconfig gcc-15.2.0
m68k allyesconfig gcc-15.2.0
microblaze allnoconfig gcc-15.2.0
microblaze allyesconfig gcc-15.2.0
mips allmodconfig gcc-15.2.0
mips allnoconfig gcc-15.2.0
mips allyesconfig gcc-15.2.0
nios2 allmodconfig gcc-11.5.0
nios2 allnoconfig gcc-11.5.0
nios2 randconfig-001-20260523 gcc-11.5.0
nios2 randconfig-002-20260523 gcc-9.5.0
openrisc allnoconfig gcc-15.2.0
parisc allmodconfig gcc-15.2.0
parisc allnoconfig gcc-15.2.0
parisc allyesconfig gcc-15.2.0
parisc randconfig-002-20260523 gcc-9.5.0
powerpc allmodconfig gcc-15.2.0
powerpc allnoconfig gcc-15.2.0
powerpc64 randconfig-002-20260523 clang-20
riscv allmodconfig clang-23
riscv allnoconfig gcc-15.2.0
riscv allyesconfig clang-16
riscv randconfig-001-20260523 gcc-8.5.0
riscv randconfig-002-20260523 gcc-8.5.0
s390 allmodconfig clang-18
s390 allnoconfig clang-23
s390 allyesconfig gcc-15.2.0
s390 randconfig-001-20260523 clang-23
s390 randconfig-002-20260523 clang-23
sh allnoconfig gcc-15.2.0
sh allyesconfig gcc-15.2.0
sh defconfig gcc-15.2.0
sh randconfig-001-20260523 gcc-15.2.0
sh randconfig-002-20260523 gcc-11.5.0
sparc allnoconfig gcc-15.2.0
sparc64 allmodconfig clang-23
sparc64 defconfig clang-20
um allmodconfig clang-19
um allnoconfig clang-23
um defconfig clang-23
um i386_defconfig gcc-14
um x86_64_defconfig clang-23
x86_64 allmodconfig clang-20
x86_64 allnoconfig clang-20
x86_64 allyesconfig clang-20
x86_64 defconfig gcc-14
x86_64 rhel-9.4-rust clang-20
xtensa allnoconfig gcc-15.2.0
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2] HID: core: demote warning to debug level
From: Matteo Croce @ 2026-05-23 12:11 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260523110447.F10D41F000E9@smtp.kernel.org>
Il giorno sab 23 mag 2026 alle ore 13:04 <sashiko-bot@kernel.org> ha scritto:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Demoting the `bsize < rsize` warning to debug level masks legacy API misuse and silently drops valid short reports.
> --
>
> commit c89b6cb8fc074729ec292f2ffe95d958722d1447
> Author: Matteo Croce <teknoraver@meta.com>
>
> HID: core: demote warning to debug level
>
> This commit demotes the "Event data for report was too short" message from
> a warning to a debug log to prevent syslog flooding from devices that
> regularly send short reports.
>
> > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > index 41a79e43c82b3..503b9bfa5d379 100644
> > --- a/drivers/hid/hid-core.c
> > +++ b/drivers/hid/hid-core.c
> > @@ -2072,8 +2072,8 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
> > rsize = max_buffer_size;
> >
> > if (bsize < rsize) {
> > - hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %zu)\n",
> > - report->id, rsize, bsize);
> > + hid_dbg_ratelimited(hid, "Event data for report %d was too short (%d vs %zu)\n",
> > + report->id, rsize, bsize);
> > return -EINVAL;
> > }
>
> [Severity: High]
> Does demoting this warning to debug level mask legacy API misuse and silently
> drop valid short reports?
>
> The bsize < rsize check correctly returns -EINVAL to reject events where the
> allocated buffer is too small, preventing out-of-bounds memory accesses.
>
> If the UPS device triggers this warning because a core API or legacy driver
> incorrectly uses the old hid_input_report() function, the legacy API
> implicitly sets the allocated buffer size (bsize) equal to the received
> payload size (csize). When the device sends a valid short report
> (csize < rsize), bsize also becomes less than rsize, causing the HID core
> to incorrectly conclude the buffer is too small, drop the report, and print
> the warning.
>
> Silencing this warning fails to fix the root cause. The valid short reports
> are still incorrectly dropped, but now they are dropped silently, potentially
> leaving the device broken. Furthermore, demoting this warning hides genuine
> buffer under-allocation vulnerabilities in other HID drivers.
>
> Should the caller be updated to use hid_safe_input_report() with the actual
> allocated buffer length, so the short reports can be successfully processed
> via zero-padding instead of returning -EINVAL?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260523105545.68079-1-technoboy85@gmail.com?part=1
Commit 0a3fe972a7cb changed the level from debug to warning.
This is too noisy, I'm just reverting to the original behaviour.
--
Matteo Croce
perl -e 'for($t=0;;$t++){print chr($t*($t>>8|$t>>13)&255)}' |aplay
^ permalink raw reply
* Re: [PATCH v2] HID: core: demote warning to debug level
From: sashiko-bot @ 2026-05-23 11:04 UTC (permalink / raw)
To: Matteo Croce; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260523105545.68079-1-technoboy85@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Demoting the `bsize < rsize` warning to debug level masks legacy API misuse and silently drops valid short reports.
--
commit c89b6cb8fc074729ec292f2ffe95d958722d1447
Author: Matteo Croce <teknoraver@meta.com>
HID: core: demote warning to debug level
This commit demotes the "Event data for report was too short" message from
a warning to a debug log to prevent syslog flooding from devices that
regularly send short reports.
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 41a79e43c82b3..503b9bfa5d379 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2072,8 +2072,8 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
> rsize = max_buffer_size;
>
> if (bsize < rsize) {
> - hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %zu)\n",
> - report->id, rsize, bsize);
> + hid_dbg_ratelimited(hid, "Event data for report %d was too short (%d vs %zu)\n",
> + report->id, rsize, bsize);
> return -EINVAL;
> }
[Severity: High]
Does demoting this warning to debug level mask legacy API misuse and silently
drop valid short reports?
The bsize < rsize check correctly returns -EINVAL to reject events where the
allocated buffer is too small, preventing out-of-bounds memory accesses.
If the UPS device triggers this warning because a core API or legacy driver
incorrectly uses the old hid_input_report() function, the legacy API
implicitly sets the allocated buffer size (bsize) equal to the received
payload size (csize). When the device sends a valid short report
(csize < rsize), bsize also becomes less than rsize, causing the HID core
to incorrectly conclude the buffer is too small, drop the report, and print
the warning.
Silencing this warning fails to fix the root cause. The valid short reports
are still incorrectly dropped, but now they are dropped silently, potentially
leaving the device broken. Furthermore, demoting this warning hides genuine
buffer under-allocation vulnerabilities in other HID drivers.
Should the caller be updated to use hid_safe_input_report() with the actual
allocated buffer length, so the short reports can be successfully processed
via zero-padding instead of returning -EINVAL?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260523105545.68079-1-technoboy85@gmail.com?part=1
^ permalink raw reply
* [PATCH v2] HID: core: demote warning to debug level
From: Matteo Croce @ 2026-05-23 10:55 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Lee Jones
Cc: linux-input, linux-kernel, Matteo Croce
From: Matteo Croce <teknoraver@meta.com>
The log level for short messages was changed from debug to warning,
flooding syslog on systems with devices that regularly send
short reports, in my case an UPS:
$ dmesg |grep -c 'Event data for report .* was too short'
35
Demote it back to debug level.
Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing bogus memset()")
Signed-off-by: Matteo Croce <teknoraver@meta.com>
---
drivers/hid/hid-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 41a79e43c82b..503b9bfa5d37 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2072,8 +2072,8 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
rsize = max_buffer_size;
if (bsize < rsize) {
- hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %zu)\n",
- report->id, rsize, bsize);
+ hid_dbg_ratelimited(hid, "Event data for report %d was too short (%d vs %zu)\n",
+ report->id, rsize, bsize);
return -EINVAL;
}
--
2.50.1
^ permalink raw reply related
* Re: [PATCH] HID: core: demote data shortness warning to debug level
From: sashiko-bot @ 2026-05-23 10:45 UTC (permalink / raw)
To: Matteo Croce; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260523103806.60480-1-technoboy85@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch modifies the wrong warning message, failing to fix the stated issue and inappropriately masking a different error.
--
commit a5df696ba5cc949fba165e62139d8984b9884331
Author: Matteo Croce <teknoraver@meta.com>
HID: core: demote data shortness warning to debug level
This commit intends to reduce syslog flooding from devices that regularly
send short reports by demoting the "was too short" warning to a debug print.
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 41a79e43c82b3..76cfed8b93c15 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2050,8 +2050,8 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
> return 0;
>
> if (unlikely(bsize < csize)) {
> - hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %zu)\n",
> - report->id, csize, bsize);
> + hid_dbg_ratelimited(hid, "Event data for report %d is incorrect (%d vs %zu)\n",
> + report->id, csize, bsize);
[Severity: Medium]
Does this change modify the intended warning message?
The commit message notes the intention to silence the "was too short" warning.
However, this patch modifies the "is incorrect" warning instead, which triggers
when the reported size claims to be larger than the actual buffer size
(bsize < csize).
The target "was too short" warning appears further down in the function during
the bsize < rsize check.
As a result, this might leave the original syslog flooding issue unresolved
while potentially hiding a different transport driver condition.
> return -EINVAL;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260523103806.60480-1-technoboy85@gmail.com?part=1
^ permalink raw reply
* [PATCH] HID: core: demote data shortness warning to debug level
From: Matteo Croce @ 2026-05-23 10:38 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Lee Jones
Cc: linux-input, linux-kernel, Matteo Croce
From: Matteo Croce <teknoraver@meta.com>
Commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
bogus memset()") changed the log level of the short report message from
debug to warning, flooding syslog on systems with devices that
regularly send short reports, in my case an UPS:
$ dmesg |grep -c 'Event data for report .* was too short'
35
Demote it back to debug level.
Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing bogus memset()")
Signed-off-by: Matteo Croce <teknoraver@meta.com>
---
drivers/hid/hid-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 41a79e43c82b..76cfed8b93c1 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2050,8 +2050,8 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
return 0;
if (unlikely(bsize < csize)) {
- hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %zu)\n",
- report->id, csize, bsize);
+ hid_dbg_ratelimited(hid, "Event data for report %d is incorrect (%d vs %zu)\n",
+ report->id, csize, bsize);
return -EINVAL;
}
--
2.50.1
^ permalink raw reply related
* [PATCH v2 0/2] Introduce OnePlus 6/6T touchscreen compatible
From: David Heidelberg via B4 Relay @ 2026-05-23 9:45 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jason A. Donenfeld, Matthias Schiffer, Vincent Huang,
Bjorn Andersson, Konrad Dybcio
Cc: linux-input, devicetree, linux-kernel, linux-arm-msm, phone-devel,
David Heidelberg, Krzysztof Kozlowski, Konrad Dybcio
Mostly related to the
https://codeberg.org/sdm845/linux/commits/branch/b4/synaptics-rmi4
series, but independent on other changes which I trying to upstream for
more than one year.
Signed-off-by: David Heidelberg <david@ixit.cz>
---
Changes in v2:
- Add R-b tag.
- Link to v1: https://lore.kernel.org/r/20260408-synaptics-rmi4-dt-v1-0-2d32bacce673@ixit.cz
---
David Heidelberg (2):
dt-bindings: input: syna,rmi4: Document syna,rmi4-s3706b
arm64: dts: qcom: sdm845-oneplus: Update compatible to include model
Documentation/devicetree/bindings/input/syna,rmi4.yaml | 11 ++++++++---
arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi | 2 +-
2 files changed, 9 insertions(+), 4 deletions(-)
---
base-commit: 5200f5f493f79f14bbdc349e402a40dfb32f23c8
change-id: 20260408-synaptics-rmi4-dt-8aebf31790dc
Best regards,
--
David Heidelberg <david@ixit.cz>
^ permalink raw reply
* [PATCH v2 1/2] dt-bindings: input: syna,rmi4: Document syna,rmi4-s3706b
From: David Heidelberg via B4 Relay @ 2026-05-23 9:45 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jason A. Donenfeld, Matthias Schiffer, Vincent Huang,
Bjorn Andersson, Konrad Dybcio
Cc: linux-input, devicetree, linux-kernel, linux-arm-msm, phone-devel,
David Heidelberg, Krzysztof Kozlowski
In-Reply-To: <20260523-synaptics-rmi4-dt-v2-0-0645122babdc@ixit.cz>
From: David Heidelberg <david@ixit.cz>
Mostly irrelevant for authentic Synaptics touchscreens, but very important
for applying workarounds to cheap TS knockoffs.
These knockoffs work well with the downstream driver, and since the user
has no way to distinguish them, later in this patch set, we introduce
workarounds to ensure they function as well as possible.
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
Documentation/devicetree/bindings/input/syna,rmi4.yaml | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/syna,rmi4.yaml b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
index 8685ef4481f4a..fb4804ac3544d 100644
--- a/Documentation/devicetree/bindings/input/syna,rmi4.yaml
+++ b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
@@ -13,19 +13,24 @@ maintainers:
description: |
The Synaptics RMI4 (Register Mapped Interface 4) core is able to support RMI4
devices using different transports (I2C, SPI) and different functions (e.g.
Function 1, 2D sensors using Function 11 or 12).
properties:
compatible:
- enum:
- - syna,rmi4-i2c
- - syna,rmi4-spi
+ oneOf:
+ - enum:
+ - syna,rmi4-i2c
+ - syna,rmi4-spi
+ - items:
+ - enum:
+ - syna,rmi4-s3706b # OnePlus 6/6T
+ - const: syna,rmi4-i2c
reg:
maxItems: 1
'#address-cells':
const: 1
'#size-cells':
--
2.53.0
^ permalink raw reply related
* [PATCH v2 2/2] arm64: dts: qcom: sdm845-oneplus: Update compatible to include model
From: David Heidelberg via B4 Relay @ 2026-05-23 9:45 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jason A. Donenfeld, Matthias Schiffer, Vincent Huang,
Bjorn Andersson, Konrad Dybcio
Cc: linux-input, devicetree, linux-kernel, linux-arm-msm, phone-devel,
David Heidelberg, Krzysztof Kozlowski, Konrad Dybcio
In-Reply-To: <20260523-synaptics-rmi4-dt-v2-0-0645122babdc@ixit.cz>
From: David Heidelberg <david@ixit.cz>
We know the driver is reporting s3706b, introduce the compatible so we
can more easily introduce quirks for weird touchscreen replacements in
followup series.
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
index 6b7378cf4d493..148164d456a5a 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
@@ -475,17 +475,17 @@ bq27441_fg: bq27441-battery@55 {
};
};
&i2c12 {
status = "okay";
clock-frequency = <400000>;
synaptics-rmi4-i2c@20 {
- compatible = "syna,rmi4-i2c";
+ compatible = "syna,rmi4-s3706b", "syna,rmi4-i2c";
reg = <0x20>;
#address-cells = <1>;
#size-cells = <0>;
interrupts-extended = <&tlmm 125 IRQ_TYPE_EDGE_FALLING>;
pinctrl-names = "default";
pinctrl-0 = <&ts_default_pins>;
--
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