* [PATCH] HID: Intel-thc-hid: Intel-thc: Use str_true_false() helper
From: liu.xuemei1 @ 2025-07-24 2:36 UTC (permalink / raw)
To: jikos, bentiss
Cc: even.xu, xinpeng.sun, srinivas.pandruvada, liu.song13,
linux-input, linux-kernel
From: Liu Song <liu.song13@zte.com.cn>
Remove hard-coded strings by using the str_true_false() helper function.
Signed-off-by: Liu Song <liu.song13@zte.com.cn>
---
drivers/hid/intel-thc-hid/intel-thc/intel-thc-dev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dev.c b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dev.c
index 6f2263869b20..2b794bb481a0 100644
--- a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dev.c
+++ b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dev.c
@@ -4,6 +4,7 @@
#include <linux/bitfield.h>
#include <linux/math.h>
#include <linux/regmap.h>
+#include <linux/string_choices.h>
#include "intel-thc-dev.h"
#include "intel-thc-hw.h"
@@ -664,7 +665,7 @@ int thc_interrupt_quiesce(const struct thc_device *dev, bool int_quiesce)
if (ret) {
dev_err_once(dev->dev,
"Timeout while waiting THC idle, target quiesce state = %s\n",
- int_quiesce ? "true" : "false");
+ str_true_false(int_quiesce));
return ret;
}
--
2.27.0
^ permalink raw reply related
* Re: [PATCH 00/11] HID: Implement haptic forcepad support
From: Jonathan Denose @ 2025-07-23 19:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov, Jonathan Corbet,
Henrik Rydberg
Cc: linux-input, linux-kernel, linux-doc, Angela Czubak,
Sean O'Brien
In-Reply-To: <20250714-support-forcepads-v1-0-71c7c05748c9@google.com>
On Mon, Jul 14, 2025 at 10:09 AM Jonathan Denose <jdenose@google.com> wrote:
>
> Hello,
>
> This is an updated implementation of the interface for controlling haptic
> touchpads.
>
> Below is an updated design proposal for the userspace and HID interfaces,
> modified from what one of my colleagues submitted in 2019 [0].
>
> We would appreciate any feedback you might have.
>
> Thank you,
>
> Jonathan Denose
> Chromium OS Team
>
> Background
> ==========
>
> There are multiple independent projects to develop a touchpad with force sensors
> and haptic actuators, instead of a traditional button. These haptic touchpads
> have several advantages and potential uses; they allow clicking across the
> entire touchpad surface, adjusting the force requirement for clicks, haptic
> feedback initiated by UI, etc. Supporting these features will potentially
> require two new communication channels at the kernel level:
> * Control of haptic motor by the host
> * Force sensor data from device to host
>
> This document includes two related proposals:
> 1. HID design proposal, that hardware makers would need to implement
> 2. Kernel design proposal
>
> Objective
> ==========
>
> Develop a standard protocol to allow userspace applications to communicate with
> haptic touchpads, and minimize duplicated code and effort.
>
> Requirements:
> 1. Support UI-initiated haptic feedback.
> 2. Allow userspace to control when button press and button release haptic
> effects are triggered. (Useful when detecting a false click, changing force
> thresholds, or sending context-dependent effects).
> 3. Reveal force sensor readings to userspace applications.
> 4. Only allow OS-controlled haptic feedback for those systems which support it.
>
> Proposal
> ========
>
> In order to minimize duplicated effort, we propose standardized haptic touchpad
> support in the linux kernel.
>
> HID API
> -------
>
> Modes
> .....
>
> The haptic touchpad should be able to operate under two different modes.
>
> 1. Device-controlled mode
>
> The haptic touchpad should start up in "device-controlled mode"
> (HID_HAPTIC_MODE_DEVICE), meaning it acts as a normal touchpad. This means it
> should perform the press and release haptic feedback autonomously at predefined
> force thresholds, and send the appropriate BTN_* events.
>
> 2. Host-controlled mode
>
> Once the touchpad has been confirmed as supporting haptics (described in more
> detail in the the "Click and release control" section below), the device should
> enter "host-controlled mode" (HID_HAPTIC_MODE_HOST). In this mode userspace
> should take control. From here, userspace will take control over
> press/release haptic feedback, relying on the effects sent by the kernel.
>
> Multitouch
> ..........
>
> The HID API for multitouch reports should follow the Microsoft precision
> touchpad spec [1], with the following changes:
> * A tip pressure field [2] should be used to report the force. The physical unit
> Type (Newtons or grams), exponent, and limits should be reported in the
> report descriptor for the force field.
> * The device will always report the button state according to its predefined
> force thresholds, even when not in device-controlled mode.
> * The device must expose a "simple haptic controller" logical collection
> alongside the touchpad collection.
>
> Haptic control
> ..............
>
> The HID protocol described in HUTRR63[3] must be used.
>
> The following waveforms should be supported:
>
> | WAVEFORMNONE | Implicit waveforms required by protocol |
> | WAVEFORMSTOP | |
> | ------------------------ | ------------------------------------------------- |
> | WAVEFORMPRESS | To be used to simulate button press. In device- |
> | | controlled mode, it will also be used to simulate |
> | | button release. |
> | ------------------------ | ------------------------------------------------- |
> | WAVEFORMRELEASE | To be used to simulate button release. |
>
> All waveforms will have an associated duration; continuous waveforms will be
> ignored by the kernel.
>
> Triggers & Mode switching
> .........................
>
> The “auto trigger waveform” should be set to WAVEFORM_PRESS by default, and the
> button from the touchpad collection should be set as the “auto trigger
> associated control”.
>
> The kernel can trigger the different modes in the following ways:
> * Device-controlled mode can be enabled by setting the “auto trigger waveform” to
> WAVEFORM_PRESS.
> * Host-controlled mode can be enabled by setting the "auto trigger waveform" to
> WAVEFORM_STOP.
>
> The device must also support manual triggering. If intensity modification for
> waveforms is supported by the device, the intensity control should be included
> in the manual trigger output report. This allows modification of the intensity
> on a per-waveform basis. Retriggering does not need to be supported by the
> device.
>
> Userspace API
> -------------
>
> Multitouch protocol
> ...................
>
> ABS_MT_PRESSURE will be used to report force. The resolution of ABS_MT_PRESSURE
> should also be defined and reported in force units of grams or Newtons.
> ABS_PRESSURE should be reported as the total force applied to the touchpad.
> When the kernel is in host-controlled mode, it should always forward the button
> press and release events to userspace.
>
> Use Force Feedback protocol to request pre-defined effects
> ..........................................................
>
> The force feedback protocol [4] should be used to control predefined effects.
>
> Typical use of the force feedback protocol requires loading effects to the
> driver by describing the output waveform, and then requesting those effects
> using an ID provided by the driver. However, for haptic touchpads we do not want
> to describe the output waveform explicitly, but use a set of predefined effects,
> which are identified by HID usage.
>
> The force feedback protocol will need to be extended to allow requests for HID
> haptic effects. This requires a new feedback effect type:
>
> /**
> * struct ff_hid_effect
> * @hid_usage: hid_usage according to Haptics page (WAVEFORM_CLICK, etc.)
> * @vendor_id: the waveform vendor ID if hid_usage is in the vendor-defined
> * range
> * @vendor_id: the vendor waveform page if hid_usage is in the vendor-defined
> * range
> * @intensity: strength of the effect
> * @repeat_count: number of times to retrigger effect
> * @retrigger_period: time before effect is retriggered (in ms)
> */
> struct ff_hid_effect {
> __u16 hid_usage;
> __u16 vendor_id;
> __u8 vendor_waveform_page;
> __s16 intensity;
> __u16 repeat_count;
> __u16 retrigger_period;
> }
>
> Since the standard waveform id namespace does not overlap with the vendor
> waveform id namespace, the vendor id and page can be ignored for standard
> waveforms.
>
> Click and release control
> .........................
>
> Haptic functionality shall be gated behind the HID_MULTITOUCH_HAPTIC kernel
> configuration option, and this kernel configuration option should only be
> enabled if userspace will support haptic capabilities. Haptic functionality will
> only be initialized and used if HID_MULTITOUCH_HAPTIC is enabled, and if the
> following conditions have been met:
> * ABS_MT_PRESSURE is defined and reporting force units of Newtons or grams.
> * The device supports haptic effects according to the hid protocol defined in
> HUTRR63 [3].
> These checks will happen when the driver probes and initializes the multitouch
> device.
>
> In the case when the kernel configuration option has been set and the device
> reports pressure and haptic effects as defined above, the kernel will initialize
> the haptic device and configure the haptic driver to signal that the touchpad is
> haptic-compatible. To signal to userspace that the touchpad is haptic-compatible
> the kernel will mark INPUT_PROP_HAPTIC_TOUCHPAD.
>
> With userspace willing and able to take control, the kernel will signal to the
> device to exit device-controlled mode once a WAVEFORMPRESS or WAVEFORMRELEASE
> event is uploaded. From here, userspace will take control over press/release
> haptic feedback, relying on the effects sent by the kernel.
>
> In all other cases, the driver will take no action to enable haptic
> functionality.
>
> Summary of normal use-case
> 1. The kernel waits for userspace to upload WAVEFORMPRESS or
> WAVEFORMRELEASE.
> 2. Userspace determines when a click has been performed based on its own
> criteria and tells the touchpad to perform a haptic effect.
> 3. When userspace erases the WAVEFORMPRESS or WAVEFORMRELEASE effect, signal the
> device to return to device-controlled mode.
>
> [0]: https://www.spinics.net/lists/linux-input/msg60938.html
> [1]: https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-devices
> [2]: Usage ID 0x30 of HID usage table 0x0D. See chapter 16:
> https://www.usb.org/sites/default/files/documents/hut1_12v2.pdf
> [3]: https://www.usb.org/sites/default/files/hutrr63b_-_haptics_page_redline_0.pdf
> [4]: https://www.kernel.org/doc/html/v4.20/input/ff.html
>
> Signed-off-by: Jonathan Denose <jdenose@google.com>
> ---
> Angela Czubak (11):
> HID: add haptics page defines
> Input: add FF_HID effect type
> Input: add INPUT_PROP_HAPTIC_TOUCHPAD
> HID: haptic: introduce hid_haptic_device
> HID: input: allow mapping of haptic output
> HID: haptic: initialize haptic device
> HID: input: calculate resolution for pressure
> HID: haptic: add functions handling events
> Input: MT - add INPUT_MT_TOTAL_FORCE flags
> HID: haptic: add hid_haptic_switch_mode
> HID: multitouch: add haptic multitouch support
>
> Documentation/input/event-codes.rst | 14 +
> drivers/hid/Kconfig | 20 ++
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-haptic.c | 580 +++++++++++++++++++++++++++++++++
> drivers/hid/hid-haptic.h | 131 ++++++++
> drivers/hid/hid-input.c | 18 +-
> drivers/hid/hid-multitouch.c | 136 +++++++-
> drivers/input/input-mt.c | 14 +-
> include/linux/hid.h | 29 ++
> include/linux/input/mt.h | 1 +
> include/uapi/linux/input-event-codes.h | 1 +
> include/uapi/linux/input.h | 22 +-
> 12 files changed, 959 insertions(+), 8 deletions(-)
> ---
> base-commit: 86731a2a651e58953fc949573895f2fa6d456841
> change-id: 20250625-support-forcepads-0b4f74fd3d0a
>
> Best regards,
> --
> Jonathan Denose <jdenose@google.com>
>
Hello,
Gently bumping this patch series for initial review if anyone has some
bandwidth to give it a look.
Please feel free to let me know if you have any questions!
Thanks,
--
Jonathan
^ permalink raw reply
* [PATCH] HID: multitouch: fix integer overflow in set_abs()
From: Qasim Ijaz @ 2025-07-23 17:36 UTC (permalink / raw)
To: jikos, bentiss; +Cc: linux-input, linux-kernel, stable
It is possible for a malicious HID device to trigger a signed integer
overflow (undefined behaviour) in set_abs() in the following expression
by supplying bogus logical maximum and minimum values:
int fuzz = snratio ? (fmax - fmin) / snratio : 0;
For example, if the logical_maximum is INT_MAX and logical_minimum is -1
then (fmax - fmin) resolves to INT_MAX + 1, which does not fit in a 32-bit
signed int, so the subtraction overflows. Fix this by computing the
difference in a 64 bit context.
Fixes: 5519cab477b6 ("HID: hid-multitouch: support for PixCir-based panels")
Cc: stable@vger.kernel.org
Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
---
drivers/hid/hid-multitouch.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 22c6314a8843..687638ed6d0f 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -540,7 +540,8 @@ static void set_abs(struct input_dev *input, unsigned int code,
{
int fmin = field->logical_minimum;
int fmax = field->logical_maximum;
- int fuzz = snratio ? (fmax - fmin) / snratio : 0;
+ s64 diff = (s64)fmax - (s64)fmin;
+ int fuzz = snratio ? (int)div_s64(diff, snratio) : 0;
input_set_abs_params(input, code, fmin, fmax, fuzz, 0);
input_abs_set_res(input, code, hidinput_calc_abs_res(field, code));
}
--
2.39.5
^ permalink raw reply related
* Re: [PATCH V2] Input: synaptics-rmi4- Add a new feature for Forcepad.
From: Dmitry Torokhov @ 2025-07-23 16:30 UTC (permalink / raw)
To: Marge Yang
Cc: linux-input, linux-kernel, david.chiu, derek.cheng, sam.tsai,
vincent.huang
In-Reply-To: <20250716033648.1785509-1-marge.yang@tw.synaptics.com>
Hi Marge,
On Wed, Jul 16, 2025 at 03:36:48AM +0000, Marge Yang wrote:
> + f21->sensor_count = fn->fd.query_base_addr & (BIT(0) | BIT(1) | BIT(2) | BIT(3));
We could either use GENMASK or just 0x0f. BIT() is for individual bits.
> +
> + if (fn->fd.query_base_addr & BIT(5)) {
> + if (fn->fd.query_base_addr & BIT(6))
> + f21->query15_offset = 2;
> + else
> + f21->query15_offset = 1;
> +
> + rmi_read_block(fn->rmi_dev, fn->fd.query_base_addr + f21->query15_offset,
> + f21->data_regs, 1);
> + f21->max_number_Of_finger = f21->data_regs[0] & 0x0F;
> + } else {
> + dev_info(&fn->dev, "f21_query15 doesn't support.\n");
> + f21->query15_offset = 0;
> + f21->max_number_Of_finger = 5;
> + }
> +
> + if (fn->fd.query_base_addr & BIT(6)) {
Just double-checking - should it be BIT(5) give that reading of number of fingers
is gated by BIT(5) in the block above.
> + dev_info(&fn->dev, "Support new F21 feature.\n");
> + /*Each finger uses one byte, and the button state uses one byte.*/
> + f21->attn_data_size = f21->max_number_Of_finger + 1;
> + f21->attn_data_index_for_button = f21->attn_data_size - 1;
> + /*
> + * Each sensor uses two bytes, the button state uses one byte,
> + * and each finger uses two bytes.
> + */
> + f21->data_reg_size = f21->sensor_count * 2 + 1 +
> + f21->max_number_Of_finger * 2;
> + f21->data_reg_index_for_button = f21->sensor_count * 2;
> + } else {
> + dev_info(&fn->dev, "Support old F21 feature.\n");
> + /*Each finger uses two bytes, and the button state uses one byte.*/
> + f21->attn_data_size = f21->sensor_count * 2 + 1;
> + f21->attn_data_index_for_button = f21->attn_data_size - 1;
> + /*Each finger uses two bytes, and the button state uses one byte.*/
> + f21->data_reg_size = f21->sensor_count * 2 + 1;
> + f21->data_reg_index_for_button = f21->data_reg_size - 1;
The block is duplicated?
No need to resubmit the patch, please just provide the answer to the
above questions.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: ilitek_ts_i2c: report key event for palm
From: dmitry.torokhov @ 2025-07-23 16:09 UTC (permalink / raw)
To: Joe Hung (洪銘陽)
Cc: francesco.dolcini@toradex.com, emanuele.ghidoli@toradex.com,
linux-input, linux-kernel@vger.kernel.org,
Luca Hsu (徐嘉鍊)
In-Reply-To: <b372a99d01c14d1690afba4ceedd0936@ilitek.com>
Hi Joe,
On Mon, Jul 14, 2025 at 09:51:42AM +0000, Joe Hung (洪銘陽) wrote:
> From ec0d80214fee6acc0b38f33ad0b6b487098963bc Mon Sep 17 00:00:00 2001
> From: Joe Hong <joe_hung@ilitek.com>
> Date: Mon, 14 Jul 2025 17:20:11 +0800
> Subject: [PATCH] input: ilitek_ts_i2c: report key event for palm
>
> Add support for reporting user-defined key event while getting palm event.
Palm contacts should be reported via MT_TOOL_PALM contact type and not a
fake key event.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: ilitek_ts_i2c: report key event for palm
From: Francesco Dolcini @ 2025-07-23 15:57 UTC (permalink / raw)
To: Joe Hung (洪銘陽)
Cc: dmitry.torokhov@gmail.com, francesco.dolcini@toradex.com,
emanuele.ghidoli@toradex.com, linux-input,
linux-kernel@vger.kernel.org, Luca Hsu (徐嘉鍊)
In-Reply-To: <b372a99d01c14d1690afba4ceedd0936@ilitek.com>
On Mon, Jul 14, 2025 at 09:51:42AM +0000, Joe Hung (洪銘陽) wrote:
> From ec0d80214fee6acc0b38f33ad0b6b487098963bc Mon Sep 17 00:00:00 2001
> From: Joe Hong <joe_hung@ilitek.com>
> Date: Mon, 14 Jul 2025 17:20:11 +0800
> Subject: [PATCH] input: ilitek_ts_i2c: report key event for palm
something wrong on this headers, check your setup
> Add support for reporting user-defined key event while getting palm event.
>
> Signed-off-by: Joe Hong <joe_hung@ilitek.com>
> ---
> drivers/input/touchscreen/ilitek_ts_i2c.c | 42 +++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
> index 0dd632724a00..fdcb4ab66fbb 100644
> --- a/drivers/input/touchscreen/ilitek_ts_i2c.c
> +++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
> @@ -39,8 +39,13 @@
> #define ILITEK_TP_I2C_REPORT_ID 0x48
>
> #define REPORT_COUNT_ADDRESS 61
> +#define ALGO_MODE_ADDRESS 62
> #define ILITEK_SUPPORT_MAX_POINT 40
>
> +static uint palm_key;
> +module_param(palm_key, uint, 0664);
> +MODULE_PARM_DESC(palm_key, "Set palm key code when palm is detected");
I do not think that putting some kind of configuration in a kernel
module param is an option
Francesco
^ permalink raw reply
* [syzbot] [input?] [usb?] [io-uring?] INFO: task hung in io_wq_put_and_exit (5)
From: syzbot @ 2025-07-23 14:44 UTC (permalink / raw)
To: anna-maria, asml.silence, axboe, frederic, io-uring, linux-input,
linux-kernel, linux-usb, syzkaller-bugs, tglx
Hello,
syzbot found the following issue on:
HEAD commit: bf61759db409 Merge tag 'sched_ext-for-6.16-rc6-fixes' of g..
git tree: upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=12b877d4580000
kernel config: https://syzkaller.appspot.com/x/.config?x=415e83411fefd73f
dashboard link: https://syzkaller.appspot.com/bug?extid=e328767eafd849df0a78
compiler: gcc (Debian 12.2.0-14+deb12u1) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=110b938c580000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1622a38c580000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/22c5f1286a72/disk-bf61759d.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/cc79af4d966c/vmlinux-bf61759d.xz
kernel image: https://storage.googleapis.com/syzbot-assets/b2e6d621f424/bzImage-bf61759d.xz
The issue was bisected to:
commit e5598d6ae62626d261b046a2f19347c38681ff51
Author: Pavel Begunkov <asml.silence@gmail.com>
Date: Thu Aug 24 22:53:31 2023 +0000
io_uring: compact SQ/CQ heads/tails
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=12c92b82580000
final oops: https://syzkaller.appspot.com/x/report.txt?x=11c92b82580000
console output: https://syzkaller.appspot.com/x/log.txt?x=16c92b82580000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+e328767eafd849df0a78@syzkaller.appspotmail.com
Fixes: e5598d6ae626 ("io_uring: compact SQ/CQ heads/tails")
INFO: task syz-executor971:5849 blocked for more than 143 seconds.
Not tainted 6.16.0-rc6-syzkaller-00279-gbf61759db409 #0
Blocked by coredump.
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:syz-executor971 state:D stack:26488 pid:5849 tgid:5849 ppid:5844 task_flags:0x400148 flags:0x00024002
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5397 [inline]
__schedule+0x116a/0x5de0 kernel/sched/core.c:6786
__schedule_loop kernel/sched/core.c:6864 [inline]
schedule+0xe7/0x3a0 kernel/sched/core.c:6879
schedule_timeout+0x257/0x290 kernel/time/sleep_timeout.c:75
do_wait_for_common kernel/sched/completion.c:95 [inline]
__wait_for_common+0x2ff/0x4e0 kernel/sched/completion.c:116
io_wq_exit_workers io_uring/io-wq.c:1319 [inline]
io_wq_put_and_exit+0x271/0x8d0 io_uring/io-wq.c:1347
io_uring_clean_tctx+0x10d/0x190 io_uring/tctx.c:203
io_uring_cancel_generic+0x69c/0x9a0 io_uring/io_uring.c:3212
io_uring_files_cancel include/linux/io_uring.h:19 [inline]
do_exit+0x2ce/0x2bd0 kernel/exit.c:911
do_group_exit+0xd3/0x2a0 kernel/exit.c:1105
__do_sys_exit_group kernel/exit.c:1116 [inline]
__se_sys_exit_group kernel/exit.c:1114 [inline]
__x64_sys_exit_group+0x3e/0x50 kernel/exit.c:1114
x64_sys_call+0x1530/0x1730 arch/x86/include/generated/asm/syscalls_64.h:232
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xcd/0x4c0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f141ec08e39
RSP: 002b:00007ffcd1b0b6e8 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f141ec08e39
RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000
RBP: 00007f141ec843b0 R08: ffffffffffffffb8 R09: 0000000000000000
R10: 000000000000000e R11: 0000000000000246 R12: 00007f141ec843b0
R13: 0000000000000000 R14: 00007f141ec880c0 R15: 00007f141ebd7020
</TASK>
INFO: task syz-executor971:5850 blocked for more than 143 seconds.
Not tainted 6.16.0-rc6-syzkaller-00279-gbf61759db409 #0
Blocked by coredump.
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:syz-executor971 state:D stack:26488 pid:5850 tgid:5850 ppid:5846 task_flags:0x400148 flags:0x00024002
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5397 [inline]
__schedule+0x116a/0x5de0 kernel/sched/core.c:6786
__schedule_loop kernel/sched/core.c:6864 [inline]
schedule+0xe7/0x3a0 kernel/sched/core.c:6879
schedule_timeout+0x257/0x290 kernel/time/sleep_timeout.c:75
do_wait_for_common kernel/sched/completion.c:95 [inline]
__wait_for_common+0x2ff/0x4e0 kernel/sched/completion.c:116
io_wq_exit_workers io_uring/io-wq.c:1319 [inline]
io_wq_put_and_exit+0x271/0x8d0 io_uring/io-wq.c:1347
io_uring_clean_tctx+0x10d/0x190 io_uring/tctx.c:203
io_uring_cancel_generic+0x69c/0x9a0 io_uring/io_uring.c:3212
io_uring_files_cancel include/linux/io_uring.h:19 [inline]
do_exit+0x2ce/0x2bd0 kernel/exit.c:911
do_group_exit+0xd3/0x2a0 kernel/exit.c:1105
__do_sys_exit_group kernel/exit.c:1116 [inline]
__se_sys_exit_group kernel/exit.c:1114 [inline]
__x64_sys_exit_group+0x3e/0x50 kernel/exit.c:1114
x64_sys_call+0x1530/0x1730 arch/x86/include/generated/asm/syscalls_64.h:232
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xcd/0x4c0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f141ec08e39
RSP: 002b:00007ffcd1b0b6e8 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f141ec08e39
RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000
RBP: 00007f141ec843b0 R08: ffffffffffffffb8 R09: 0000000000000000
R10: 000000000000000e R11: 0000000000000246 R12: 00007f141ec843b0
R13: 0000000000000000 R14: 00007f141ec880c0 R15: 00007f141ebd7020
</TASK>
INFO: task syz-executor971:5851 blocked for more than 143 seconds.
Not tainted 6.16.0-rc6-syzkaller-00279-gbf61759db409 #0
Blocked by coredump.
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:syz-executor971 state:D stack:27256 pid:5851 tgid:5851 ppid:5845 task_flags:0x400148 flags:0x00024002
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5397 [inline]
__schedule+0x116a/0x5de0 kernel/sched/core.c:6786
__schedule_loop kernel/sched/core.c:6864 [inline]
schedule+0xe7/0x3a0 kernel/sched/core.c:6879
schedule_timeout+0x257/0x290 kernel/time/sleep_timeout.c:75
do_wait_for_common kernel/sched/completion.c:95 [inline]
__wait_for_common+0x2ff/0x4e0 kernel/sched/completion.c:116
io_wq_exit_workers io_uring/io-wq.c:1319 [inline]
io_wq_put_and_exit+0x271/0x8d0 io_uring/io-wq.c:1347
io_uring_clean_tctx+0x10d/0x190 io_uring/tctx.c:203
io_uring_cancel_generic+0x69c/0x9a0 io_uring/io_uring.c:3212
io_uring_files_cancel include/linux/io_uring.h:19 [inline]
do_exit+0x2ce/0x2bd0 kernel/exit.c:911
do_group_exit+0xd3/0x2a0 kernel/exit.c:1105
__do_sys_exit_group kernel/exit.c:1116 [inline]
__se_sys_exit_group kernel/exit.c:1114 [inline]
__x64_sys_exit_group+0x3e/0x50 kernel/exit.c:1114
x64_sys_call+0x1530/0x1730 arch/x86/include/generated/asm/syscalls_64.h:232
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xcd/0x4c0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f141ec08e39
RSP: 002b:00007ffcd1b0b6e8 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f141ec08e39
RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000
RBP: 00007f141ec843b0 R08: ffffffffffffffb8 R09: 0000000000000000
R10: 000000000000000e R11: 0000000000000246 R12: 00007f141ec843b0
R13: 0000000000000000 R14: 00007f141ec880c0 R15: 00007f141ebd7020
</TASK>
INFO: task syz-executor971:5852 blocked for more than 144 seconds.
Not tainted 6.16.0-rc6-syzkaller-00279-gbf61759db409 #0
Blocked by coredump.
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:syz-executor971 state:D stack:26312 pid:5852 tgid:5852 ppid:5848 task_flags:0x400148 flags:0x00024002
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5397 [inline]
__schedule+0x116a/0x5de0 kernel/sched/core.c:6786
__schedule_loop kernel/sched/core.c:6864 [inline]
schedule+0xe7/0x3a0 kernel/sched/core.c:6879
schedule_timeout+0x257/0x290 kernel/time/sleep_timeout.c:75
do_wait_for_common kernel/sched/completion.c:95 [inline]
__wait_for_common+0x2ff/0x4e0 kernel/sched/completion.c:116
io_wq_exit_workers io_uring/io-wq.c:1319 [inline]
io_wq_put_and_exit+0x271/0x8d0 io_uring/io-wq.c:1347
io_uring_clean_tctx+0x10d/0x190 io_uring/tctx.c:203
io_uring_cancel_generic+0x69c/0x9a0 io_uring/io_uring.c:3212
io_uring_files_cancel include/linux/io_uring.h:19 [inline]
do_exit+0x2ce/0x2bd0 kernel/exit.c:911
do_group_exit+0xd3/0x2a0 kernel/exit.c:1105
__do_sys_exit_group kernel/exit.c:1116 [inline]
__se_sys_exit_group kernel/exit.c:1114 [inline]
__x64_sys_exit_group+0x3e/0x50 kernel/exit.c:1114
x64_sys_call+0x1530/0x1730 arch/x86/include/generated/asm/syscalls_64.h:232
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xcd/0x4c0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f141ec08e39
RSP: 002b:00007ffcd1b0b6e8 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f141ec08e39
RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000
RBP: 00007f141ec843b0 R08: ffffffffffffffb8 R09: 0000000000000000
R10: 000000000000000e R11: 0000000000000246 R12: 00007f141ec843b0
R13: 0000000000000000 R14: 00007f141ec880c0 R15: 00007f141ebd7020
</TASK>
INFO: task syz-executor971:5853 blocked for more than 144 seconds.
Not tainted 6.16.0-rc6-syzkaller-00279-gbf61759db409 #0
Blocked by coredump.
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:syz-executor971 state:D stack:27080 pid:5853 tgid:5853 ppid:5847 task_flags:0x400148 flags:0x00024002
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5397 [inline]
__schedule+0x116a/0x5de0 kernel/sched/core.c:6786
__schedule_loop kernel/sched/core.c:6864 [inline]
schedule+0xe7/0x3a0 kernel/sched/core.c:6879
schedule_timeout+0x257/0x290 kernel/time/sleep_timeout.c:75
do_wait_for_common kernel/sched/completion.c:95 [inline]
__wait_for_common+0x2ff/0x4e0 kernel/sched/completion.c:116
io_wq_exit_workers io_uring/io-wq.c:1319 [inline]
io_wq_put_and_exit+0x271/0x8d0 io_uring/io-wq.c:1347
io_uring_clean_tctx+0x10d/0x190 io_uring/tctx.c:203
io_uring_cancel_generic+0x69c/0x9a0 io_uring/io_uring.c:3212
io_uring_files_cancel include/linux/io_uring.h:19 [inline]
do_exit+0x2ce/0x2bd0 kernel/exit.c:911
do_group_exit+0xd3/0x2a0 kernel/exit.c:1105
__do_sys_exit_group kernel/exit.c:1116 [inline]
__se_sys_exit_group kernel/exit.c:1114 [inline]
__x64_sys_exit_group+0x3e/0x50 kernel/exit.c:1114
x64_sys_call+0x1530/0x1730 arch/x86/include/generated/asm/syscalls_64.h:232
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xcd/0x4c0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f141ec08e39
RSP: 002b:00007ffcd1b0b6e8 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f141ec08e39
RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000
RBP: 00007f141ec843b0 R08: ffffffffffffffb8 R09: 0000000000000000
R10: 000000000000000e R11: 0000000000000246 R12: 00007f141ec843b0
R13: 0000000000000000 R14: 00007f141ec880c0 R15: 00007f141ebd7020
</TASK>
Showing all locks held in the system:
1 lock held by khungtaskd/31:
#0: ffffffff8e5c4e00 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire include/linux/rcupdate.h:331 [inline]
#0: ffffffff8e5c4e00 (rcu_read_lock){....}-{1:3}, at: rcu_read_lock include/linux/rcupdate.h:841 [inline]
#0: ffffffff8e5c4e00 (rcu_read_lock){....}-{1:3}, at: debug_show_all_locks+0x36/0x1c0 kernel/locking/lockdep.c:6770
2 locks held by getty/5594:
#0: ffff888032cc90a0 (&tty->ldisc_sem){++++}-{0:0}, at: tty_ldisc_ref_wait+0x24/0x80 drivers/tty/tty_ldisc.c:243
#1: ffffc900036cb2f0 (&ldata->atomic_read_lock){+.+.}-{4:4}, at: n_tty_read+0x41b/0x14f0 drivers/tty/n_tty.c:2222
=============================================
NMI backtrace for cpu 1
CPU: 1 UID: 0 PID: 31 Comm: khungtaskd Not tainted 6.16.0-rc6-syzkaller-00279-gbf61759db409 #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/07/2025
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:94 [inline]
dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:120
nmi_cpu_backtrace+0x27b/0x390 lib/nmi_backtrace.c:113
nmi_trigger_cpumask_backtrace+0x29c/0x300 lib/nmi_backtrace.c:62
trigger_all_cpu_backtrace include/linux/nmi.h:158 [inline]
check_hung_uninterruptible_tasks kernel/hung_task.c:307 [inline]
watchdog+0xf70/0x12c0 kernel/hung_task.c:470
kthread+0x3c5/0x780 kernel/kthread.c:464
ret_from_fork+0x5d4/0x6f0 arch/x86/kernel/process.c:148
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
</TASK>
Sending NMI from CPU 1 to CPUs 0:
NMI backtrace for cpu 0
CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.16.0-rc6-syzkaller-00279-gbf61759db409 #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/07/2025
RIP: 0010:pv_native_safe_halt+0xf/0x20 arch/x86/kernel/paravirt.c:82
Code: 7b 6d 02 e9 83 fb 02 00 0f 1f 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 66 90 0f 00 2d 43 99 23 00 fb f4 <c3> cc cc cc cc 66 2e 0f 1f 84 00 00 00 00 00 66 90 90 90 90 90 90
RSP: 0018:ffffffff8e207e08 EFLAGS: 000002c6
RAX: 00000000000d6725 RBX: 0000000000000000 RCX: ffffffff8b867c99
RDX: 0000000000000000 RSI: ffffffff8de2fff6 RDI: ffffffff8c157460
RBP: fffffbfff1c52ef0 R08: 0000000000000001 R09: ffffed1017086645
R10: ffff8880b843322b R11: 0000000000000001 R12: 0000000000000000
R13: ffffffff8e297780 R14: ffffffff90a94550 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff888124720000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000055e50c02d168 CR3: 000000000e382000 CR4: 00000000003526f0
Call Trace:
<TASK>
arch_safe_halt arch/x86/include/asm/paravirt.h:107 [inline]
default_idle+0x13/0x20 arch/x86/kernel/process.c:749
default_idle_call+0x6d/0xb0 kernel/sched/idle.c:117
cpuidle_idle_call kernel/sched/idle.c:185 [inline]
do_idle+0x391/0x510 kernel/sched/idle.c:325
cpu_startup_entry+0x4f/0x60 kernel/sched/idle.c:423
rest_init+0x16b/0x2b0 init/main.c:745
start_kernel+0x3ee/0x4d0 init/main.c:1102
x86_64_start_reservations+0x18/0x30 arch/x86/kernel/head64.c:307
x86_64_start_kernel+0x130/0x190 arch/x86/kernel/head64.c:288
common_startup_64+0x13e/0x148
</TASK>
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply
* [PATCH] HID: core: Harden s32ton() against conversion to 0 bits
From: Alan Stern @ 2025-07-23 14:37 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: syzbot, jikos, linux-input, linux-kernel, linux-usb,
syzkaller-bugs
In-Reply-To: <750377a5-b5df-4b2b-8e38-0001bfbeb30f@rowland.harvard.edu>
Testing by the syzbot fuzzer showed that the HID core gets a
shift-out-of-bounds exception when it tries to convert a 32-bit
quantity to a 0-bit quantity. Ideally this should never occur, but
there are buggy devices and some might have a report field with size
set to zero; we shouldn't reject the report or the device just because
of that.
Instead, harden the s32ton() routine so that it returns a reasonable
result instead of crashing when it is called with the number of bits
set to 0 -- the same as what snto32() does.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: syzbot+b63d677d63bcac06cf90@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/linux-usb/68753a08.050a0220.33d347.0008.GAE@google.com/
Tested-by: syzbot+b63d677d63bcac06cf90@syzkaller.appspotmail.com
Fixes: dde5845a529f ("[PATCH] Generic HID layer - code split")
Cc: stable@vger.kernel.org
---
The commit listed in the Fixes tag is not really the right one. But
code motion made tracking it back any further more difficult than I
wanted to deal with, so I stopped there. That commit is from 2006,
which is already far enough in the past.
drivers/hid/hid-core.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
Index: usb-devel/drivers/hid/hid-core.c
===================================================================
--- usb-devel.orig/drivers/hid/hid-core.c
+++ usb-devel/drivers/hid/hid-core.c
@@ -66,8 +66,12 @@ static s32 snto32(__u32 value, unsigned
static u32 s32ton(__s32 value, unsigned int n)
{
- s32 a = value >> (n - 1);
+ s32 a;
+ if (!value || !n)
+ return 0;
+
+ a = value >> (n - 1);
if (a && a != -1)
return value < 0 ? 1 << (n - 1) : (1 << (n - 1)) - 1;
return value & ((1 << n) - 1);
^ permalink raw reply
* Re: [PATCH] HID: core: Reject report fields with a size or count of 0
From: Alan Stern @ 2025-07-23 14:34 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: syzbot, jikos, linux-input, linux-kernel, linux-usb,
syzkaller-bugs
In-Reply-To: <34ks6futbrmunsq2tbz75jwqg64lpk4pg6udbbk3yo2exm657b@3fivbjjdcyl4>
On Wed, Jul 23, 2025 at 11:32:14AM +0200, Benjamin Tissoires wrote:
> On Jul 21 2025, Alan Stern wrote:
> > On Mon, Jul 21, 2025 at 03:05:58PM +0200, Benjamin Tissoires wrote:
> > > > So then would it be better to accept these report fields (perhaps with a
> > > > warning) and instead, harden the core HID code so that it doesn't choke
> > > > when it runs across one of them?
> > > >
> > >
> > > Yeah, that seems like the best plan forward.
> > >
> > > [sorry on reduced setup for the next 3 weeks, so I can't really debug
> > > the entire thing now.]
> > >
> > > Though, we should probably not annoy users unless we are trying to do
> > > something that won't be needed. I doubt that Saitek gamepad will ever
> > > call the faulty functions, so why putting an error in the logs when it's
> > > working fine?
> >
> > All right.
> >
> > Probably the best way to do this is simply to revert the commit that's
> > already applied and then merge a new patch to harden the core. Would
> > you like me to post the reversion patch or do you prefer to do it
> > yourself?
>
> Given that the faulty commit is on top of for-6.17/core, I can simply
> force push to the parent, and also force push the for-next branch. That
> should do the trick.
>
> Can you post your s32ton fix on top of that then?
Sure. Patch coming up shortly...
Alan Stern
^ permalink raw reply
* [PATCH v2] HID: multitouch: fix slab out-of-bounds access in mt_report_fixup()
From: Qasim Ijaz @ 2025-07-23 11:00 UTC (permalink / raw)
To: jikos, bentiss; +Cc: envelsavinds, jirislaby, linux-input, linux-kernel, stable
A malicious HID device can trigger a slab out-of-bounds during
mt_report_fixup() by passing in report descriptor smaller than
607 bytes. mt_report_fixup() attempts to patch byte offset 607
of the descriptor with 0x25 by first checking if byte offset
607 is 0x15 however it lacks bounds checks to verify if the
descriptor is big enough before conducting this check. Fix
this bug by ensuring the descriptor size is at least 608
bytes before accessing it.
Below is the KASAN splat after the out of bounds access happens:
[ 13.671954] ==================================================================
[ 13.672667] BUG: KASAN: slab-out-of-bounds in mt_report_fixup+0x103/0x110
[ 13.673297] Read of size 1 at addr ffff888103df39df by task kworker/0:1/10
[ 13.673297]
[ 13.673297] CPU: 0 UID: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.15.0-00005-gec5d573d83f4-dirty #3
[ 13.673297] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/04
[ 13.673297] Call Trace:
[ 13.673297] <TASK>
[ 13.673297] dump_stack_lvl+0x5f/0x80
[ 13.673297] print_report+0xd1/0x660
[ 13.673297] kasan_report+0xe5/0x120
[ 13.673297] __asan_report_load1_noabort+0x18/0x20
[ 13.673297] mt_report_fixup+0x103/0x110
[ 13.673297] hid_open_report+0x1ef/0x810
[ 13.673297] mt_probe+0x422/0x960
[ 13.673297] hid_device_probe+0x2e2/0x6f0
[ 13.673297] really_probe+0x1c6/0x6b0
[ 13.673297] __driver_probe_device+0x24f/0x310
[ 13.673297] driver_probe_device+0x4e/0x220
[ 13.673297] __device_attach_driver+0x169/0x320
[ 13.673297] bus_for_each_drv+0x11d/0x1b0
[ 13.673297] __device_attach+0x1b8/0x3e0
[ 13.673297] device_initial_probe+0x12/0x20
[ 13.673297] bus_probe_device+0x13d/0x180
[ 13.673297] device_add+0xe3a/0x1670
[ 13.673297] hid_add_device+0x31d/0xa40
[...]
Fixes: c8000deb6836 ("HID: multitouch: Add support for GT7868Q")
Cc: stable@vger.kernel.org
Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
---
v2:
- Simplify fix with a if-size check after discussion with Jiri Slaby
- Change explanation of bug to reflect inclusion of a if-size check
drivers/hid/hid-multitouch.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 294516a8f541..22c6314a8843 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1503,6 +1503,14 @@ static const __u8 *mt_report_fixup(struct hid_device *hdev, __u8 *rdesc,
if (hdev->vendor == I2C_VENDOR_ID_GOODIX &&
(hdev->product == I2C_DEVICE_ID_GOODIX_01E8 ||
hdev->product == I2C_DEVICE_ID_GOODIX_01E9)) {
+ if (*size < 608) {
+ dev_info(
+ &hdev->dev,
+ "GT7868Q fixup: report descriptor is only %u bytes, skipping\n",
+ *size);
+ return rdesc;
+ }
+
if (rdesc[607] == 0x15) {
rdesc[607] = 0x25;
dev_info(
--
2.39.5
^ permalink raw reply related
* Re: [PATCH RESEND] HID: multitouch: fix slab out-of-bounds access in mt_report_fixup()
From: Qasim Ijaz @ 2025-07-23 10:24 UTC (permalink / raw)
To: Jiri Slaby
Cc: jikos, bentiss, linux-input, linux-kernel, stable, Dmitry Savin
In-Reply-To: <1a2d0220-5862-4cba-8d0f-2eb7556c9620@kernel.org>
On Wed, Jul 23, 2025 at 06:40:52AM +0200, Jiri Slaby wrote:
> On 22. 07. 25, 13:19, Qasim Ijaz wrote:
> > On Tue, Jul 22, 2025 at 11:16:15AM +0200, Jiri Slaby wrote:
> > > On 22. 07. 25, 10:00, Qasim Ijaz wrote:
> > > > A malicious HID device can trigger a slab out-of-bounds during
> > > > mt_report_fixup() by passing in report descriptor smaller than
> > > > 607 bytes. mt_report_fixup() attempts to patch byte offset 607
> > > > of the descriptor with 0x25 by first checking if byte offset
> > > > 607 is 0x15 however it lacks bounds checks to verify if the
> > > > descriptor is big enough before conducting this check. Fix
> > > > this vulnerability by ensuring the descriptor size is
> > > > greater than or equal to 608 before accessing it.
> > > >
> > > > Below is the KASAN splat after the out of bounds access happens:
> > > >
> > > > [ 13.671954] ==================================================================
> > > > [ 13.672667] BUG: KASAN: slab-out-of-bounds in mt_report_fixup+0x103/0x110
> > > > [ 13.673297] Read of size 1 at addr ffff888103df39df by task kworker/0:1/10
> > > > [ 13.673297]
> > > > [ 13.673297] CPU: 0 UID: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.15.0-00005-gec5d573d83f4-dirty #3
> > > > [ 13.673297] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/04
> > > > [ 13.673297] Call Trace:
> > > > [ 13.673297] <TASK>
> > > > [ 13.673297] dump_stack_lvl+0x5f/0x80
> > > > [ 13.673297] print_report+0xd1/0x660
> > > > [ 13.673297] kasan_report+0xe5/0x120
> > > > [ 13.673297] __asan_report_load1_noabort+0x18/0x20
> > > > [ 13.673297] mt_report_fixup+0x103/0x110
> > > > [ 13.673297] hid_open_report+0x1ef/0x810
> > > > [ 13.673297] mt_probe+0x422/0x960
> > > > [ 13.673297] hid_device_probe+0x2e2/0x6f0
> > > > [ 13.673297] really_probe+0x1c6/0x6b0
> > > > [ 13.673297] __driver_probe_device+0x24f/0x310
> > > > [ 13.673297] driver_probe_device+0x4e/0x220
> > > > [ 13.673297] __device_attach_driver+0x169/0x320
> > > > [ 13.673297] bus_for_each_drv+0x11d/0x1b0
> > > > [ 13.673297] __device_attach+0x1b8/0x3e0
> > > > [ 13.673297] device_initial_probe+0x12/0x20
> > > > [ 13.673297] bus_probe_device+0x13d/0x180
> > > > [ 13.673297] device_add+0xe3a/0x1670
> > > > [ 13.673297] hid_add_device+0x31d/0xa40
> > > > [...]
> > > >
> > > > Fixes: c8000deb6836 ("HID: multitouch: Add support for GT7868Q")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
> > > > Reviewed-by: Dmitry Savin <envelsavinds@gmail.com>
> > > > ---
> > > > drivers/hid/hid-multitouch.c | 25 ++++++++++++++++---------
> > > > 1 file changed, 16 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> > > > index 7ac8e16e6158..af4abe3ba410 100644
> > > > --- a/drivers/hid/hid-multitouch.c
> > > > +++ b/drivers/hid/hid-multitouch.c
> > > > @@ -1461,18 +1461,25 @@ static const __u8 *mt_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> > > > if (hdev->vendor == I2C_VENDOR_ID_GOODIX &&
> > > > (hdev->product == I2C_DEVICE_ID_GOODIX_01E8 ||
> > > > hdev->product == I2C_DEVICE_ID_GOODIX_01E9)) {
> > > > - if (rdesc[607] == 0x15) {
> > > > - rdesc[607] = 0x25;
> > > > - dev_info(
> > > > - &hdev->dev,
> > > > - "GT7868Q report descriptor fixup is applied.\n");
> > > > + if (*size >= 608) {
> > > > + if (rdesc[607] == 0x15) {
> > > > + rdesc[607] = 0x25;
> > > > + dev_info(
> > > > + &hdev->dev,
> > > > + "GT7868Q report descriptor fixup is applied.\n");
> > > > + } else {
> > > > + dev_info(
> > > > + &hdev->dev,
> > > > + "The byte is not expected for fixing the report descriptor. \
> > > > + It's possible that the touchpad firmware is not suitable for applying the fix. \
> > > > + got: %x\n",
> > >
> > > This is wrong. You have all the spaces/tabs in the string now. Drop all the
> > > backslashes, and open and close the string on every line.
> > >
> > > > + rdesc[607]);
> > > > + }
> > >
> > > As this is superlong and superindented, perhaps introduce a new function for
> > > these devices?
> > >
> > > > } else {
> > > > dev_info(
> > > > &hdev->dev,
> > > > - "The byte is not expected for fixing the report descriptor. \
> > > > -It's possible that the touchpad firmware is not suitable for applying the fix. \
> > > > -got: %x\n",
> > >
> > > This was horrid too, yeah.
> > >
> > > > - rdesc[607]);
> > > > + "GT7868Q fixup: report descriptor only %u bytes, skipping\n",
> > >
> > > A predicate missing. Eg. "has only", or "is only".
> > >
> >
> > Thanks for the feedback Jiri, I took the advice on board, is something
> > like this better?
>
> Definitely.
>
> > static const __u8 *mt_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> > unsigned int *size)
> > {
> > if (hdev->vendor == I2C_VENDOR_ID_GOODIX &&
> > (hdev->product == I2C_DEVICE_ID_GOODIX_01E8 ||
> > hdev->product == I2C_DEVICE_ID_GOODIX_01E9)) {
> > if (*size < 608) {
> > dev_info(
> > &hdev->dev,
> > "GT7868Q fixup: report descriptor is only %u bytes, skipping\n",
> > *size);
> > return rdesc;
> > }
> > if (rdesc[607] == 0x15) {
> > rdesc[607] = 0x25;
> > dev_info(
> > &hdev->dev,
> > "GT7868Q fixup: report descriptor fixup is applied.\n");
> > } else {
> > dev_info(&hdev->dev,
> > "GT7868Q fixup: offset 607 is %x (expected 0x15), "
> > "descriptor may be malformed, skipping\n",
> > rdesc[607]);
> > }
> > }
> > return rdesc;
> > }
> >
> > the key changes I made are:
> >
> > - Move size check to the top, this way the indentation level is decent
> > - get rid of message backslashes
> > - shorten the fixup failure message when rdesc[607] is not 0x15 and make
> > it a bit clearer since this message was the longest - just a minor
> > cleanup
> > - added "is only %u bytes" as you suggested
> >
> > if this is all good I can send v2.
>
> I would invert the conditions. So the code would look like:
>
> static bool goodix_needs_fixup(hdev)
> {
> if (hdev->vendor != I2C_VENDOR_ID_GOODIX)
> return false;
>
> return hdev->product == I2C_DEVICE_ID_GOODIX_01E8 ||
> hdev->product == I2C_DEVICE_ID_GOODIX_01E9;
> }
>
> static const __u8 *mt_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> unsigned int *size)
> {
> if (!goodix_needs_fixup(hdev))
> return rdesc;
>
> if (*size < 608) {
> dev_info(&hdev->dev,
> "GT7868Q fixup: report descriptor is only %u bytes,
> skipping\n",
> *size);
> return rdesc;
> }
>
> if (rdesc[607] != 0x15) {
> dev_info(&hdev->dev,
> "GT7868Q fixup: offset 607 is %x (expected 0x15), descriptor may be
> malformed, skipping\n",
> rdesc[607]);
> return rdesc;
> }
>
> rdesc[607] = 0x25;
> dev_info(&hdev->dev,
> "GT7868Q fixup: report descriptor fixup is applied.\n");
>
> return rdesc;
> }
Thanks Jiri, this looks good. I think Ill split this into 2 patches
one for fixing the OOB with a size check and the second for a general
function cleanup. Hope that sounds good.
Thanks
qasim
>
> thanks,
> --
> js
> suse labs
^ permalink raw reply
* Re: (subset) [syzbot] [input?] [usb?] UBSAN: shift-out-of-bounds in s32ton (2)
From: Benjamin Tissoires @ 2025-07-23 9:36 UTC (permalink / raw)
To: syzbot, Alan Stern
Cc: jikos, linux-input, linux-kernel, linux-usb, syzkaller-bugs
In-Reply-To: <175285492024.272050.11219945704830043047.b4-ty@kernel.org>
On Jul 18 2025, Benjamin Tissoires wrote:
> On Tue, 15 Jul 2025 15:29:25 -0400, Alan Stern wrote:
> > On Mon, Jul 14, 2025 at 10:10:32AM -0700, syzbot wrote:
> > > Hello,
> > >
> > > syzbot found the following issue on:
> > >
> > > HEAD commit: b4b4dbfa96de media: stk1160: use usb_alloc_noncoherent/usb..
> > > git tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=15a830f0580000
> > > kernel config: https://syzkaller.appspot.com/x/.config?x=28729dff5d03ad1
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=b63d677d63bcac06cf90
> > > compiler: gcc (Debian 12.2.0-14+deb12u1) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
> > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1614418c580000
> > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1257dd82580000
> > >
> > > Downloadable assets:
> > > disk image: https://storage.googleapis.com/syzbot-assets/7301552ad828/disk-b4b4dbfa.raw.xz
> > > vmlinux: https://storage.googleapis.com/syzbot-assets/c559b38fa1b6/vmlinux-b4b4dbfa.xz
> > > kernel image: https://storage.googleapis.com/syzbot-assets/9c1da8b2a83f/bzImage-b4b4dbfa.xz
> > >
> > > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > > Reported-by: syzbot+b63d677d63bcac06cf90@syzkaller.appspotmail.com
> > >
> > > usb 4-1: config 0 interface 0 altsetting 0 has 1 endpoint descriptor, different from the interface descriptor's value: 9
> > > usb 4-1: New USB device found, idVendor=045e, idProduct=07da, bcdDevice= 0.00
> > > usb 4-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
> > > usb 4-1: config 0 descriptor??
> > > microsoft 0003:045E:07DA.0001: ignoring exceeding usage max
> > > microsoft 0003:045E:07DA.0001: unsupported Resolution Multiplier 0
> > > ------------[ cut here ]------------
> > > UBSAN: shift-out-of-bounds in drivers/hid/hid-core.c:69:16
> > > shift exponent 4294967295 is too large for 32-bit type 'int'
> > > CPU: 0 UID: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.16.0-rc4-syzkaller-00314-gb4b4dbfa96de #0 PREEMPT(voluntary)
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/07/2025
> > > Workqueue: usb_hub_wq hub_event
> > > Call Trace:
> > > <TASK>
> > > __dump_stack lib/dump_stack.c:94 [inline]
> > > dump_stack_lvl+0x16c/0x1f0 lib/dump_stack.c:120
> > > ubsan_epilogue lib/ubsan.c:233 [inline]
> > > __ubsan_handle_shift_out_of_bounds+0x27f/0x420 lib/ubsan.c:494
> > > s32ton.cold+0x37/0x9c drivers/hid/hid-core.c:69
> > > hid_output_field drivers/hid/hid-core.c:1841 [inline]
> > > hid_output_report+0x36f/0x4a0 drivers/hid/hid-core.c:1874
> > > __hid_request+0x1e0/0x3c0 drivers/hid/hid-core.c:1987
> > > hidinput_change_resolution_multipliers drivers/hid/hid-input.c:1950 [inline]
> > > hidinput_connect+0x1ada/0x2bd0 drivers/hid/hid-input.c:2327
> >
> > [...]
>
> Applied to hid/hid.git (for-6.17/core), thanks!
>
> [1/1] HID: core: Reject report fields with a size or count of 0
> https://git.kernel.org/hid/hid/c/bcf266ca2779
FTR, the patch has been dropped from for-next, it broke existing
devices. A better patch is being worked on.
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH] HID: core: Reject report fields with a size or count of 0
From: Benjamin Tissoires @ 2025-07-23 9:32 UTC (permalink / raw)
To: Alan Stern
Cc: syzbot, jikos, linux-input, linux-kernel, linux-usb,
syzkaller-bugs
In-Reply-To: <bd033800-53f0-4d5a-a52b-b0e01ac48c12@rowland.harvard.edu>
On Jul 21 2025, Alan Stern wrote:
> On Mon, Jul 21, 2025 at 03:05:58PM +0200, Benjamin Tissoires wrote:
> > > So then would it be better to accept these report fields (perhaps with a
> > > warning) and instead, harden the core HID code so that it doesn't choke
> > > when it runs across one of them?
> > >
> >
> > Yeah, that seems like the best plan forward.
> >
> > [sorry on reduced setup for the next 3 weeks, so I can't really debug
> > the entire thing now.]
> >
> > Though, we should probably not annoy users unless we are trying to do
> > something that won't be needed. I doubt that Saitek gamepad will ever
> > call the faulty functions, so why putting an error in the logs when it's
> > working fine?
>
> All right.
>
> Probably the best way to do this is simply to revert the commit that's
> already applied and then merge a new patch to harden the core. Would
> you like me to post the reversion patch or do you prefer to do it
> yourself?
Given that the faulty commit is on top of for-6.17/core, I can simply
force push to the parent, and also force push the for-next branch. That
should do the trick.
Can you post your s32ton fix on top of that then?
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense
From: Cristian Ciocaltea @ 2025-07-23 7:17 UTC (permalink / raw)
To: Roderick Colenbrander
Cc: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg, kernel, linux-input, linux-kernel
In-Reply-To: <CAEc3jaDsX8OSVskO6-Rsvn12BbV2-8ZjhV+tPaRpu9Nai3czEg@mail.gmail.com>
Hi Roderick,
On 7/23/25 7:04 AM, Roderick Colenbrander wrote:
> Hi Cristian,
>
> Thanks for the information on the audio patches in the sound tree. We
> weren't familiar with that part.
I've actually mentioned that in the cover letter and changelog of this
revision. Couldn't do this previously because I submitted the HID series
before the USB audio one.
> I talked a bit with my team members as well. In general audio is
> getting some bigger attention (will see where that goes). I'm getting
> a bit worried that the HID and usb driver need much closer coupling,
> the current coupling not being enough.
I think we should keep things simple for now, at least until we land the
basic support, also considering the USB part has been already merged.
> I don't know the USB audio spec too well, but it is more on the
> digital interface and a DAC. I'm not sure on the exact circuitry on
> the DualSense, but there is a lot of logic in the console drivers for
> volume handling where adjustment of the volume talks to the HID layer
> to send a new output report. I suspect they had very good reasons for
> it (e.g. for headphone also dealing with different impedances).
>
> So I'm not sure how the volume control is really supposed to work, but
> I would think to do it properly it requires some interaction between
> the audio and HID drivers. Just letting the audio side do it right
> now, is more about leveraging the range of the DAC I guess versus a
> proper audio amplification stage.
Indeed, it's not possible to support hardware volume control from
ALSA/usb-audio without involving HID. This could be done, or at least it's
worth investigating further, but it's not mandatory and definitely beyond
the scope of the current work.
> Just thinking of things from the user perspective, they should have a
> unified volume control. I don't know how other devices are doing it,
> but I think we need to think a bit further and we need to reconsider
> how things work....
There's a bit more complexity in here than initially anticipated, but the
(software) volume control is not really a problem. It's worth noting I am
going to provide some UCM changes, part of the ALSA project:
https://github.com/alsa-project/alsa-ucm-conf
This is to ensure proper support for audio profile switching between
headphones/headset and internal speaker/microphone, which also addresses
a few volume control related issues. Those are mainly caused by the haptic
feedback functionality, which is controlled by a pair of dedicated channels
in a quadraphonic audio stream. One of the UCM's main jobs is to split the
4.0 PCM stream into 4 mono channels or a pair of stereo (FL+FR) channels,
depending on the active output device/profile.
The only blocker now is this HID series, which prevents us moving further.
Therefore, unless there is anything else remaining which requires urgent
attention, could you please provide an ack for Jiri to be able to pick this
up?
Thanks,
Cristian
> Thanks,
> Roderick
>
> On Tue, Jul 22, 2025 at 1:03 AM Cristian Ciocaltea
> <cristian.ciocaltea@collabora.com> wrote:
>>
>> Hi Roderick,
>>
>> On 7/22/25 9:18 AM, Roderick Colenbrander wrote:
>>> Hi Cristian and Jiri,
>>>
>>> One thing I forgot to bring up is whether it is best to have the audio
>>> plug logic have its separate input device or have it be part of an
>>> existing (e.g. main gamepad). The patch currently creates a separate
>>> input device. Originally we added multiple input devices (gamepad,
>>> touchpad and sensors) due to axes and button collisions really.
>>>
>>> For this feature there is no collision. There are not many devices in
>>> the kernel, which support these audio EV_SW. I see for example the
>>> Switch 2 controller has a mini jack port as well. Some xbox
>>> controllers too (though audio not supported in the kernel from a quick
>>> glance or at least no HID or xpad driver features for them).
>>>
>>> I don't have a strong opinion yet. Initial feeling was perhaps have it
>>> on the 'main' input device. But on the other hand, I'm not sure what
>>> software is normally listening for these kinds of EV_SW events. What
>>> would be listening for this like a pipewire?
>>
>> For now this is going to be used by the usb-audio driver which contains a
>> quirk [1] creating the jack controls for headphone and headset mic,
>> respectively. This will further setup an input handler for each of them in
>> order to intercept the related hotplug events.
>>
>> But it can be also used directly from ALSA/pipewire, e.g. for monitoring,
>> hence it think it's best to keep it as an audio dedicated input device.
>>
>> Regards,
>> Cristian
>>
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/sound/usb/mixer_quirks.c#n540
^ permalink raw reply
* [PATCH] dt-bindings: touchscreen: drop any reference to touchscreen.txt
From: Dario Binacchi @ 2025-07-23 7:14 UTC (permalink / raw)
To: linux-kernel
Cc: linux-amarula, Dario Binacchi,
Broadcom internal kernel review list, Conor Dooley,
Dmitry Torokhov, Florian Fainelli, Krzysztof Kozlowski,
Rob Herring, devicetree, linux-arm-kernel, linux-input,
linux-rpi-kernel
With commit 1d6204e2f51f ("dt-bindings: touchscreen: Add touchscreen
schema") touchscreen.txt is no longer needed. Remove the file and
replace every reference to it with the corresponding YAML schema.
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
---
.../devicetree/bindings/input/touchscreen/bu21013.txt | 2 +-
.../devicetree/bindings/input/touchscreen/eeti.txt | 2 +-
.../input/touchscreen/raspberrypi,firmware-ts.txt | 10 +++++-----
.../bindings/input/touchscreen/touchscreen.txt | 1 -
.../devicetree/bindings/input/touchscreen/zet6223.txt | 10 +++++-----
5 files changed, 12 insertions(+), 13 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt b/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt
index da4c9d8b99b1..9f1a6d03c4da 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/bu21013.txt
@@ -17,7 +17,7 @@ Optional properties:
- touchscreen-swapped-x-y : General touchscreen binding, see [1].
[1] All general touchscreen properties are described in
- Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt.
+ Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml.
Deprecated properties:
- rohm,touch-max-x : Maximum outward permitted limit in the X axis
diff --git a/Documentation/devicetree/bindings/input/touchscreen/eeti.txt b/Documentation/devicetree/bindings/input/touchscreen/eeti.txt
index 32b3712c916e..1230b0fd153f 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/eeti.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/eeti.txt
@@ -10,7 +10,7 @@ Optional properties:
latched. This is necessary for platforms that lack
support for level-triggered IRQs.
-The following optional properties described in touchscreen.txt are
+The following optional properties described in touchscreen.yaml are
also supported:
- touchscreen-inverted-x
diff --git a/Documentation/devicetree/bindings/input/touchscreen/raspberrypi,firmware-ts.txt b/Documentation/devicetree/bindings/input/touchscreen/raspberrypi,firmware-ts.txt
index 2a1af240ccc3..c554c89b4e55 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/raspberrypi,firmware-ts.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/raspberrypi,firmware-ts.txt
@@ -6,11 +6,11 @@ Required properties:
Optional properties:
- firmware: Reference to RPi's firmware device node
- - touchscreen-size-x: See touchscreen.txt
- - touchscreen-size-y: See touchscreen.txt
- - touchscreen-inverted-x: See touchscreen.txt
- - touchscreen-inverted-y: See touchscreen.txt
- - touchscreen-swapped-x-y: See touchscreen.txt
+ - touchscreen-size-x: See touchscreen.yaml
+ - touchscreen-size-y: See touchscreen.yaml
+ - touchscreen-inverted-x: See touchscreen.yaml
+ - touchscreen-inverted-y: See touchscreen.yaml
+ - touchscreen-swapped-x-y: See touchscreen.yaml
Example:
diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
deleted file mode 100644
index e1adb902d503..000000000000
--- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
+++ /dev/null
@@ -1 +0,0 @@
-See touchscreen.yaml
diff --git a/Documentation/devicetree/bindings/input/touchscreen/zet6223.txt b/Documentation/devicetree/bindings/input/touchscreen/zet6223.txt
index 27d55a506f18..384eeb4a333e 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/zet6223.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/zet6223.txt
@@ -10,11 +10,11 @@ Optional properties:
- vio-supply : Specification for VIO supply (1.8V or 3.3V,
depending on system interface needs).
- vcc-supply : Specification for 3.3V VCC supply.
-- touchscreen-size-x : See touchscreen.txt
-- touchscreen-size-y : See touchscreen.txt
-- touchscreen-inverted-x : See touchscreen.txt
-- touchscreen-inverted-y : See touchscreen.txt
-- touchscreen-swapped-x-y : See touchscreen.txt
+- touchscreen-size-x : See touchscreen.yaml
+- touchscreen-size-y : See touchscreen.yaml
+- touchscreen-inverted-x : See touchscreen.yaml
+- touchscreen-inverted-y : See touchscreen.yaml
+- touchscreen-swapped-x-y : See touchscreen.yaml
Example:
--
2.43.0
base-commit: 89be9a83ccf1f88522317ce02f854f30d6115c41
branch: drop-touchscreen.txt
^ permalink raw reply related
* Re: [PATCH 2/4] dt-bindings: input: touchscreen: fsl,imx6ul-tsc: add fsl,glitch-threshold
From: Dario Binacchi @ 2025-07-23 6:25 UTC (permalink / raw)
To: Rob Herring
Cc: linux-kernel, linux-amarula, Conor Dooley, Dmitry Torokhov,
Fabio Estevam, Haibo Chen, Krzysztof Kozlowski,
Pengutronix Kernel Team, Sascha Hauer, Shawn Guo, devicetree, imx,
linux-arm-kernel, linux-input
In-Reply-To: <20250723050319.GA1239529-robh@kernel.org>
On Wed, Jul 23, 2025 at 7:03 AM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Jul 22, 2025 at 12:36:16PM +0200, Dario Binacchi wrote:
> > Add support for glitch threshold configuration. A detected signal is valid
> > only if it lasts longer than the set threshold; otherwise, it is regarded
> > as a glitch.
> >
> > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > ---
> >
> > .../input/touchscreen/fsl,imx6ul-tsc.yaml | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml b/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml
> > index 678756ad0f92..2fee2940213f 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml
> > @@ -62,6 +62,23 @@ properties:
> > description: Number of data samples which are averaged for each read.
> > enum: [ 1, 4, 8, 16, 32 ]
> >
> > + fsl,glitch-threshold:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + default: 0
> > + enum: [ 0, 1, 2, 3 ]
> > + description: |
> > + Indicates the glitch threshold. The threshold is defined by number
> > + of clock cycles. A detect signal is only valid if it is exist longer
> > + than threshold; otherwise, it is regarded as a glitch.
> > + 0: Normal function: 8191 clock cycles
> > + Low power mode: 9 clock cycles
> > + 1: Normal function: 4095 clock cycles
> > + Low power mode: 7 clock cycles
> > + 2: Normal function: 2047 clock cycles
> > + Low power mode: 5 clock cycles
> > + 3: Normal function: 1023 clock cycles
> > + Low power mode: 3 clock cycles
>
> Don't we have common properties for this expressed in time? Debounce
> time IIRC.
I tried checking in
Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml,
but I didn't find anything about it.
It exists in some specific touchscreen bindings:
- azoteq,iqs7211.yaml
- brcm,iproc-touchscreen.txt
- fsl-mx25-tcq.txt,
- ti,ads7843.yaml.
Only fsl-mx25-tcq.txt expresses it in terms of time (ns).
Thanks and regards,
Dario
>
> > +
> > required:
> > - compatible
> > - reg
> > @@ -94,4 +111,5 @@ examples:
> > measure-delay-time = <0xfff>;
> > pre-charge-time = <0xffff>;
> > touchscreen-average-samples = <32>;
> > + fsl,glitch-threshold = <2>;
> > };
> > --
> > 2.43.0
> >
--
Dario Binacchi
Senior Embedded Linux Developer
dario.binacchi@amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: input: touchscreen: fsl,imx6ul-tsc: add fsl,glitch-threshold
From: Dario Binacchi @ 2025-07-23 6:14 UTC (permalink / raw)
To: Bough Chen
Cc: linux-kernel@vger.kernel.org, linux-amarula@amarulasolutions.com,
Conor Dooley, Dmitry Torokhov, Fabio Estevam, Krzysztof Kozlowski,
Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo,
devicetree@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org, linux-input@vger.kernel.org
In-Reply-To: <DU0PR04MB94966B1D445966C0E7C1BDF2905FA@DU0PR04MB9496.eurprd04.prod.outlook.com>
Hi Bough,
On Wed, Jul 23, 2025 at 4:45 AM Bough Chen <haibo.chen@nxp.com> wrote:
>
> > -----Original Message-----
> > From: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > Sent: 2025年7月22日 18:36
> > To: linux-kernel@vger.kernel.org
> > Cc: linux-amarula@amarulasolutions.com; Dario Binacchi
> > <dario.binacchi@amarulasolutions.com>; Conor Dooley
> > <conor+dt@kernel.org>; Dmitry Torokhov <dmitry.torokhov@gmail.com>;
> > Fabio Estevam <festevam@gmail.com>; Bough Chen <haibo.chen@nxp.com>;
> > Krzysztof Kozlowski <krzk+dt@kernel.org>; Pengutronix Kernel Team
> > <kernel@pengutronix.de>; Rob Herring <robh@kernel.org>; Sascha Hauer
> > <s.hauer@pengutronix.de>; Shawn Guo <shawnguo@kernel.org>;
> > devicetree@vger.kernel.org; imx@lists.linux.dev;
> > linux-arm-kernel@lists.infradead.org; linux-input@vger.kernel.org
> > Subject: [PATCH 2/4] dt-bindings: input: touchscreen: fsl,imx6ul-tsc: add
> > fsl,glitch-threshold
> >
> > Add support for glitch threshold configuration. A detected signal is valid only if it
> > lasts longer than the set threshold; otherwise, it is regarded as a glitch.
> >
> > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > ---
> >
> > .../input/touchscreen/fsl,imx6ul-tsc.yaml | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml
> > b/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml
> > index 678756ad0f92..2fee2940213f 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc
> > +++ .yaml
> > @@ -62,6 +62,23 @@ properties:
> > description: Number of data samples which are averaged for each read.
> > enum: [ 1, 4, 8, 16, 32 ]
> >
> > + fsl,glitch-threshold:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + default: 0
>
> Here the default should be 2 according to your third patch.
I considered the default value of the register, not the driver's.
If I'm not mistaken, the dt-bindings should be a description of the
hardware, not of the associated driver implementation.
>
> I'm okay for this patch set, let's waiting for comments about this yaml change.
>
> By the way, any real case which need to use other glitch threshod?
We recently submitted
https://lore.kernel.org/all/aGUDza5XRGDqfz5n@dragon/#t, and in this
initial contribution we removed all the custom parts that are not
supported upstream. This is
one such case. For our board, we need a de_glitch value of 1.
Thanks and regards,
Dario
>
> Regards
> Haibo Chen
>
> > + enum: [ 0, 1, 2, 3 ]
> > + description: |
> > + Indicates the glitch threshold. The threshold is defined by number
> > + of clock cycles. A detect signal is only valid if it is exist longer
> > + than threshold; otherwise, it is regarded as a glitch.
> > + 0: Normal function: 8191 clock cycles
> > + Low power mode: 9 clock cycles
> > + 1: Normal function: 4095 clock cycles
> > + Low power mode: 7 clock cycles
> > + 2: Normal function: 2047 clock cycles
> > + Low power mode: 5 clock cycles
> > + 3: Normal function: 1023 clock cycles
> > + Low power mode: 3 clock cycles
> > +
> > required:
> > - compatible
> > - reg
> > @@ -94,4 +111,5 @@ examples:
> > measure-delay-time = <0xfff>;
> > pre-charge-time = <0xffff>;
> > touchscreen-average-samples = <32>;
> > + fsl,glitch-threshold = <2>;
> > };
> > --
> > 2.43.0
>
--
Dario Binacchi
Senior Embedded Linux Developer
dario.binacchi@amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: input: touchscreen: fsl,imx6ul-tsc: add fsl,glitch-threshold
From: Rob Herring @ 2025-07-23 5:03 UTC (permalink / raw)
To: Dario Binacchi
Cc: linux-kernel, linux-amarula, Conor Dooley, Dmitry Torokhov,
Fabio Estevam, Haibo Chen, Krzysztof Kozlowski,
Pengutronix Kernel Team, Sascha Hauer, Shawn Guo, devicetree, imx,
linux-arm-kernel, linux-input
In-Reply-To: <20250722103706.3440777-3-dario.binacchi@amarulasolutions.com>
On Tue, Jul 22, 2025 at 12:36:16PM +0200, Dario Binacchi wrote:
> Add support for glitch threshold configuration. A detected signal is valid
> only if it lasts longer than the set threshold; otherwise, it is regarded
> as a glitch.
>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> ---
>
> .../input/touchscreen/fsl,imx6ul-tsc.yaml | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml b/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml
> index 678756ad0f92..2fee2940213f 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml
> @@ -62,6 +62,23 @@ properties:
> description: Number of data samples which are averaged for each read.
> enum: [ 1, 4, 8, 16, 32 ]
>
> + fsl,glitch-threshold:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + default: 0
> + enum: [ 0, 1, 2, 3 ]
> + description: |
> + Indicates the glitch threshold. The threshold is defined by number
> + of clock cycles. A detect signal is only valid if it is exist longer
> + than threshold; otherwise, it is regarded as a glitch.
> + 0: Normal function: 8191 clock cycles
> + Low power mode: 9 clock cycles
> + 1: Normal function: 4095 clock cycles
> + Low power mode: 7 clock cycles
> + 2: Normal function: 2047 clock cycles
> + Low power mode: 5 clock cycles
> + 3: Normal function: 1023 clock cycles
> + Low power mode: 3 clock cycles
Don't we have common properties for this expressed in time? Debounce
time IIRC.
> +
> required:
> - compatible
> - reg
> @@ -94,4 +111,5 @@ examples:
> measure-delay-time = <0xfff>;
> pre-charge-time = <0xffff>;
> touchscreen-average-samples = <32>;
> + fsl,glitch-threshold = <2>;
> };
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH RESEND] HID: multitouch: fix slab out-of-bounds access in mt_report_fixup()
From: Jiri Slaby @ 2025-07-23 4:40 UTC (permalink / raw)
To: Qasim Ijaz
Cc: jikos, bentiss, linux-input, linux-kernel, stable, Dmitry Savin
In-Reply-To: <aH9zl18IqvL7l9pX@gmail.com>
On 22. 07. 25, 13:19, Qasim Ijaz wrote:
> On Tue, Jul 22, 2025 at 11:16:15AM +0200, Jiri Slaby wrote:
>> On 22. 07. 25, 10:00, Qasim Ijaz wrote:
>>> A malicious HID device can trigger a slab out-of-bounds during
>>> mt_report_fixup() by passing in report descriptor smaller than
>>> 607 bytes. mt_report_fixup() attempts to patch byte offset 607
>>> of the descriptor with 0x25 by first checking if byte offset
>>> 607 is 0x15 however it lacks bounds checks to verify if the
>>> descriptor is big enough before conducting this check. Fix
>>> this vulnerability by ensuring the descriptor size is
>>> greater than or equal to 608 before accessing it.
>>>
>>> Below is the KASAN splat after the out of bounds access happens:
>>>
>>> [ 13.671954] ==================================================================
>>> [ 13.672667] BUG: KASAN: slab-out-of-bounds in mt_report_fixup+0x103/0x110
>>> [ 13.673297] Read of size 1 at addr ffff888103df39df by task kworker/0:1/10
>>> [ 13.673297]
>>> [ 13.673297] CPU: 0 UID: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.15.0-00005-gec5d573d83f4-dirty #3
>>> [ 13.673297] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/04
>>> [ 13.673297] Call Trace:
>>> [ 13.673297] <TASK>
>>> [ 13.673297] dump_stack_lvl+0x5f/0x80
>>> [ 13.673297] print_report+0xd1/0x660
>>> [ 13.673297] kasan_report+0xe5/0x120
>>> [ 13.673297] __asan_report_load1_noabort+0x18/0x20
>>> [ 13.673297] mt_report_fixup+0x103/0x110
>>> [ 13.673297] hid_open_report+0x1ef/0x810
>>> [ 13.673297] mt_probe+0x422/0x960
>>> [ 13.673297] hid_device_probe+0x2e2/0x6f0
>>> [ 13.673297] really_probe+0x1c6/0x6b0
>>> [ 13.673297] __driver_probe_device+0x24f/0x310
>>> [ 13.673297] driver_probe_device+0x4e/0x220
>>> [ 13.673297] __device_attach_driver+0x169/0x320
>>> [ 13.673297] bus_for_each_drv+0x11d/0x1b0
>>> [ 13.673297] __device_attach+0x1b8/0x3e0
>>> [ 13.673297] device_initial_probe+0x12/0x20
>>> [ 13.673297] bus_probe_device+0x13d/0x180
>>> [ 13.673297] device_add+0xe3a/0x1670
>>> [ 13.673297] hid_add_device+0x31d/0xa40
>>> [...]
>>>
>>> Fixes: c8000deb6836 ("HID: multitouch: Add support for GT7868Q")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
>>> Reviewed-by: Dmitry Savin <envelsavinds@gmail.com>
>>> ---
>>> drivers/hid/hid-multitouch.c | 25 ++++++++++++++++---------
>>> 1 file changed, 16 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>>> index 7ac8e16e6158..af4abe3ba410 100644
>>> --- a/drivers/hid/hid-multitouch.c
>>> +++ b/drivers/hid/hid-multitouch.c
>>> @@ -1461,18 +1461,25 @@ static const __u8 *mt_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>>> if (hdev->vendor == I2C_VENDOR_ID_GOODIX &&
>>> (hdev->product == I2C_DEVICE_ID_GOODIX_01E8 ||
>>> hdev->product == I2C_DEVICE_ID_GOODIX_01E9)) {
>>> - if (rdesc[607] == 0x15) {
>>> - rdesc[607] = 0x25;
>>> - dev_info(
>>> - &hdev->dev,
>>> - "GT7868Q report descriptor fixup is applied.\n");
>>> + if (*size >= 608) {
>>> + if (rdesc[607] == 0x15) {
>>> + rdesc[607] = 0x25;
>>> + dev_info(
>>> + &hdev->dev,
>>> + "GT7868Q report descriptor fixup is applied.\n");
>>> + } else {
>>> + dev_info(
>>> + &hdev->dev,
>>> + "The byte is not expected for fixing the report descriptor. \
>>> + It's possible that the touchpad firmware is not suitable for applying the fix. \
>>> + got: %x\n",
>>
>> This is wrong. You have all the spaces/tabs in the string now. Drop all the
>> backslashes, and open and close the string on every line.
>>
>>> + rdesc[607]);
>>> + }
>>
>> As this is superlong and superindented, perhaps introduce a new function for
>> these devices?
>>
>>> } else {
>>> dev_info(
>>> &hdev->dev,
>>> - "The byte is not expected for fixing the report descriptor. \
>>> -It's possible that the touchpad firmware is not suitable for applying the fix. \
>>> -got: %x\n",
>>
>> This was horrid too, yeah.
>>
>>> - rdesc[607]);
>>> + "GT7868Q fixup: report descriptor only %u bytes, skipping\n",
>>
>> A predicate missing. Eg. "has only", or "is only".
>>
>
> Thanks for the feedback Jiri, I took the advice on board, is something
> like this better?
Definitely.
> static const __u8 *mt_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> unsigned int *size)
> {
> if (hdev->vendor == I2C_VENDOR_ID_GOODIX &&
> (hdev->product == I2C_DEVICE_ID_GOODIX_01E8 ||
> hdev->product == I2C_DEVICE_ID_GOODIX_01E9)) {
> if (*size < 608) {
> dev_info(
> &hdev->dev,
> "GT7868Q fixup: report descriptor is only %u bytes, skipping\n",
> *size);
> return rdesc;
> }
>
> if (rdesc[607] == 0x15) {
> rdesc[607] = 0x25;
> dev_info(
> &hdev->dev,
> "GT7868Q fixup: report descriptor fixup is applied.\n");
> } else {
> dev_info(&hdev->dev,
> "GT7868Q fixup: offset 607 is %x (expected 0x15), "
> "descriptor may be malformed, skipping\n",
> rdesc[607]);
> }
> }
>
> return rdesc;
> }
>
> the key changes I made are:
>
> - Move size check to the top, this way the indentation level is decent
> - get rid of message backslashes
> - shorten the fixup failure message when rdesc[607] is not 0x15 and make
> it a bit clearer since this message was the longest - just a minor
> cleanup
> - added "is only %u bytes" as you suggested
>
> if this is all good I can send v2.
I would invert the conditions. So the code would look like:
static bool goodix_needs_fixup(hdev)
{
if (hdev->vendor != I2C_VENDOR_ID_GOODIX)
return false;
return hdev->product == I2C_DEVICE_ID_GOODIX_01E8 ||
hdev->product == I2C_DEVICE_ID_GOODIX_01E9;
}
static const __u8 *mt_report_fixup(struct hid_device *hdev, __u8 *rdesc,
unsigned int *size)
{
if (!goodix_needs_fixup(hdev))
return rdesc;
if (*size < 608) {
dev_info(&hdev->dev,
"GT7868Q fixup: report descriptor is only %u bytes,
skipping\n",
*size);
return rdesc;
}
if (rdesc[607] != 0x15) {
dev_info(&hdev->dev,
"GT7868Q fixup: offset 607 is %x (expected 0x15), descriptor may be
malformed, skipping\n",
rdesc[607]);
return rdesc;
}
rdesc[607] = 0x25;
dev_info(&hdev->dev,
"GT7868Q fixup: report descriptor fixup is applied.\n");
return rdesc;
}
thanks,
--
js
suse labs
^ permalink raw reply
* Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense
From: Roderick Colenbrander @ 2025-07-23 4:20 UTC (permalink / raw)
To: Cristian Ciocaltea
Cc: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg, kernel, linux-input, linux-kernel
In-Reply-To: <CAEc3jaDsX8OSVskO6-Rsvn12BbV2-8ZjhV+tPaRpu9Nai3czEg@mail.gmail.com>
I have asked someone in Tokyo for some more clarification as well. As
I said my worry is that it really isn't this simple...
Separately we are now starting to formally study Bluetooth audio. That
one is difficult, but at least less messy on the plumbing side as it
is all HID based.
Thanks,
Roderick
On Tue, Jul 22, 2025 at 9:04 PM Roderick Colenbrander
<thunderbird2k@gmail.com> wrote:
>
> Hi Cristian,
>
> Thanks for the information on the audio patches in the sound tree. We
> weren't familiar with that part.
>
> I talked a bit with my team members as well. In general audio is
> getting some bigger attention (will see where that goes). I'm getting
> a bit worried that the HID and usb driver need much closer coupling,
> the current coupling not being enough.
>
> I don't know the USB audio spec too well, but it is more on the
> digital interface and a DAC. I'm not sure on the exact circuitry on
> the DualSense, but there is a lot of logic in the console drivers for
> volume handling where adjustment of the volume talks to the HID layer
> to send a new output report. I suspect they had very good reasons for
> it (e.g. for headphone also dealing with different impedances).
>
> So I'm not sure how the volume control is really supposed to work, but
> I would think to do it properly it requires some interaction between
> the audio and HID drivers. Just letting the audio side do it right
> now, is more about leveraging the range of the DAC I guess versus a
> proper audio amplification stage.
>
> Just thinking of things from the user perspective, they should have a
> unified volume control. I don't know how other devices are doing it,
> but I think we need to think a bit further and we need to reconsider
> how things work....
>
> Thanks,
> Roderick
>
> On Tue, Jul 22, 2025 at 1:03 AM Cristian Ciocaltea
> <cristian.ciocaltea@collabora.com> wrote:
> >
> > Hi Roderick,
> >
> > On 7/22/25 9:18 AM, Roderick Colenbrander wrote:
> > > Hi Cristian and Jiri,
> > >
> > > One thing I forgot to bring up is whether it is best to have the audio
> > > plug logic have its separate input device or have it be part of an
> > > existing (e.g. main gamepad). The patch currently creates a separate
> > > input device. Originally we added multiple input devices (gamepad,
> > > touchpad and sensors) due to axes and button collisions really.
> > >
> > > For this feature there is no collision. There are not many devices in
> > > the kernel, which support these audio EV_SW. I see for example the
> > > Switch 2 controller has a mini jack port as well. Some xbox
> > > controllers too (though audio not supported in the kernel from a quick
> > > glance or at least no HID or xpad driver features for them).
> > >
> > > I don't have a strong opinion yet. Initial feeling was perhaps have it
> > > on the 'main' input device. But on the other hand, I'm not sure what
> > > software is normally listening for these kinds of EV_SW events. What
> > > would be listening for this like a pipewire?
> >
> > For now this is going to be used by the usb-audio driver which contains a
> > quirk [1] creating the jack controls for headphone and headset mic,
> > respectively. This will further setup an input handler for each of them in
> > order to intercept the related hotplug events.
> >
> > But it can be also used directly from ALSA/pipewire, e.g. for monitoring,
> > hence it think it's best to keep it as an audio dedicated input device.
> >
> > Regards,
> > Cristian
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/sound/usb/mixer_quirks.c#n540
^ permalink raw reply
* Re: [PATCH v2 00/11] HID: playstation: Add support for audio jack handling on DualSense
From: Roderick Colenbrander @ 2025-07-23 4:04 UTC (permalink / raw)
To: Cristian Ciocaltea
Cc: Roderick Colenbrander, Jiri Kosina, Benjamin Tissoires,
Henrik Rydberg, kernel, linux-input, linux-kernel
In-Reply-To: <ab1c06b1-9b79-426a-a43b-cf5a89688770@collabora.com>
Hi Cristian,
Thanks for the information on the audio patches in the sound tree. We
weren't familiar with that part.
I talked a bit with my team members as well. In general audio is
getting some bigger attention (will see where that goes). I'm getting
a bit worried that the HID and usb driver need much closer coupling,
the current coupling not being enough.
I don't know the USB audio spec too well, but it is more on the
digital interface and a DAC. I'm not sure on the exact circuitry on
the DualSense, but there is a lot of logic in the console drivers for
volume handling where adjustment of the volume talks to the HID layer
to send a new output report. I suspect they had very good reasons for
it (e.g. for headphone also dealing with different impedances).
So I'm not sure how the volume control is really supposed to work, but
I would think to do it properly it requires some interaction between
the audio and HID drivers. Just letting the audio side do it right
now, is more about leveraging the range of the DAC I guess versus a
proper audio amplification stage.
Just thinking of things from the user perspective, they should have a
unified volume control. I don't know how other devices are doing it,
but I think we need to think a bit further and we need to reconsider
how things work....
Thanks,
Roderick
On Tue, Jul 22, 2025 at 1:03 AM Cristian Ciocaltea
<cristian.ciocaltea@collabora.com> wrote:
>
> Hi Roderick,
>
> On 7/22/25 9:18 AM, Roderick Colenbrander wrote:
> > Hi Cristian and Jiri,
> >
> > One thing I forgot to bring up is whether it is best to have the audio
> > plug logic have its separate input device or have it be part of an
> > existing (e.g. main gamepad). The patch currently creates a separate
> > input device. Originally we added multiple input devices (gamepad,
> > touchpad and sensors) due to axes and button collisions really.
> >
> > For this feature there is no collision. There are not many devices in
> > the kernel, which support these audio EV_SW. I see for example the
> > Switch 2 controller has a mini jack port as well. Some xbox
> > controllers too (though audio not supported in the kernel from a quick
> > glance or at least no HID or xpad driver features for them).
> >
> > I don't have a strong opinion yet. Initial feeling was perhaps have it
> > on the 'main' input device. But on the other hand, I'm not sure what
> > software is normally listening for these kinds of EV_SW events. What
> > would be listening for this like a pipewire?
>
> For now this is going to be used by the usb-audio driver which contains a
> quirk [1] creating the jack controls for headphone and headset mic,
> respectively. This will further setup an input handler for each of them in
> order to intercept the related hotplug events.
>
> But it can be also used directly from ALSA/pipewire, e.g. for monitoring,
> hence it think it's best to keep it as an audio dedicated input device.
>
> Regards,
> Cristian
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/sound/usb/mixer_quirks.c#n540
^ permalink raw reply
* RE: [PATCH 2/4] dt-bindings: input: touchscreen: fsl,imx6ul-tsc: add fsl,glitch-threshold
From: Bough Chen @ 2025-07-23 2:45 UTC (permalink / raw)
To: Dario Binacchi, linux-kernel@vger.kernel.org
Cc: linux-amarula@amarulasolutions.com, Conor Dooley, Dmitry Torokhov,
Fabio Estevam, Krzysztof Kozlowski, Pengutronix Kernel Team,
Rob Herring, Sascha Hauer, Shawn Guo, devicetree@vger.kernel.org,
imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
linux-input@vger.kernel.org
In-Reply-To: <20250722103706.3440777-3-dario.binacchi@amarulasolutions.com>
> -----Original Message-----
> From: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> Sent: 2025年7月22日 18:36
> To: linux-kernel@vger.kernel.org
> Cc: linux-amarula@amarulasolutions.com; Dario Binacchi
> <dario.binacchi@amarulasolutions.com>; Conor Dooley
> <conor+dt@kernel.org>; Dmitry Torokhov <dmitry.torokhov@gmail.com>;
> Fabio Estevam <festevam@gmail.com>; Bough Chen <haibo.chen@nxp.com>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Rob Herring <robh@kernel.org>; Sascha Hauer
> <s.hauer@pengutronix.de>; Shawn Guo <shawnguo@kernel.org>;
> devicetree@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-input@vger.kernel.org
> Subject: [PATCH 2/4] dt-bindings: input: touchscreen: fsl,imx6ul-tsc: add
> fsl,glitch-threshold
>
> Add support for glitch threshold configuration. A detected signal is valid only if it
> lasts longer than the set threshold; otherwise, it is regarded as a glitch.
>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> ---
>
> .../input/touchscreen/fsl,imx6ul-tsc.yaml | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml
> b/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml
> index 678756ad0f92..2fee2940213f 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/fsl,imx6ul-tsc
> +++ .yaml
> @@ -62,6 +62,23 @@ properties:
> description: Number of data samples which are averaged for each read.
> enum: [ 1, 4, 8, 16, 32 ]
>
> + fsl,glitch-threshold:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + default: 0
Here the default should be 2 according to your third patch.
I'm okay for this patch set, let's waiting for comments about this yaml change.
By the way, any real case which need to use other glitch threshod?
Regards
Haibo Chen
> + enum: [ 0, 1, 2, 3 ]
> + description: |
> + Indicates the glitch threshold. The threshold is defined by number
> + of clock cycles. A detect signal is only valid if it is exist longer
> + than threshold; otherwise, it is regarded as a glitch.
> + 0: Normal function: 8191 clock cycles
> + Low power mode: 9 clock cycles
> + 1: Normal function: 4095 clock cycles
> + Low power mode: 7 clock cycles
> + 2: Normal function: 2047 clock cycles
> + Low power mode: 5 clock cycles
> + 3: Normal function: 1023 clock cycles
> + Low power mode: 3 clock cycles
> +
> required:
> - compatible
> - reg
> @@ -94,4 +111,5 @@ examples:
> measure-delay-time = <0xfff>;
> pre-charge-time = <0xffff>;
> touchscreen-average-samples = <32>;
> + fsl,glitch-threshold = <2>;
> };
> --
> 2.43.0
^ permalink raw reply
* [PATCH] HID: intel-ish-hid: Increase ISHTP resume ack timeout to 300ms
From: Zhang Lixu @ 2025-07-23 1:31 UTC (permalink / raw)
To: linux-input, srinivas.pandruvada, jikos, benjamin.tissoires
Cc: lixu.zhang, hua.he, wenji1.yang, juswin.hsueh, henry.yeh,
neo.wong
During s2idle suspend/resume testing on some systems, occasional several
tens of seconds delays were observed in HID sensor resume handling. Trace
analysis revealed repeated "link not ready" timeout errors during
set/get_report operations, which were traced to the
hid_ishtp_cl_resume_handler() timing out while waiting for the ISHTP
resume acknowledgment. The previous timeout was set to 50ms, which proved
insufficient on affected machines.
Empirical measurements on failing systems showed that the time from ISH
resume initiation to receiving the ISHTP resume ack could be as long as
180ms. As a result, the 50ms timeout caused failures.
To address this, increase the wait timeout for ISHTP resume ack from 50ms
to 300ms, providing a safer margin for slower hardware. Additionally, add
error logging when a timeout occurs to aid future debugging and issue
triage. No functional changes are made beyond the timeout adjustment and
improved error reporting.
Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ipc/pci-ish.c | 3 ---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 3 +++
drivers/hid/intel-ish-hid/ishtp/bus.c | 3 ---
drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h | 3 +++
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index c57483224db6..9d150ce234f2 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -264,9 +264,6 @@ static void ish_shutdown(struct pci_dev *pdev)
static struct device __maybe_unused *ish_resume_device;
-/* 50ms to get resume response */
-#define WAIT_FOR_RESUME_ACK_MS 50
-
/**
* ish_resume_handler() - Work function to complete resume
* @work: work struct
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 6550ad5bfbb5..d8c3c54a8c0f 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -759,6 +759,9 @@ static void hid_ishtp_cl_resume_handler(struct work_struct *work)
if (ishtp_wait_resume(ishtp_get_ishtp_device(hid_ishtp_cl))) {
client_data->suspended = false;
wake_up_interruptible(&client_data->ishtp_resume_wait);
+ } else {
+ hid_ishtp_trace(client_data, "hid client: wait for resume timed out");
+ dev_err(cl_data_to_dev(client_data), "wait for resume timed out");
}
}
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 5ac7d70a7c84..93a0432e7058 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -852,9 +852,6 @@ EXPORT_SYMBOL(ishtp_device);
*/
bool ishtp_wait_resume(struct ishtp_device *dev)
{
- /* 50ms to get resume response */
- #define WAIT_FOR_RESUME_ACK_MS 50
-
/* Waiting to get resume response */
if (dev->resume_flag)
wait_event_interruptible_timeout(dev->resume_wait,
diff --git a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
index ec9f6e87aaf2..23db97ecf21c 100644
--- a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
+++ b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
@@ -47,6 +47,9 @@
#define MAX_DMA_DELAY 20
+/* 300ms to get resume response */
+#define WAIT_FOR_RESUME_ACK_MS 300
+
/* ISHTP device states */
enum ishtp_dev_state {
ISHTP_DEV_INITIALIZING = 0,
base-commit: 05adbee3ad528100ab0285c15c91100e19e10138
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 1/2] input: Add tracepoint support
From: Mathieu Desnoyers @ 2025-07-23 1:24 UTC (permalink / raw)
To: Steven Rostedt, WangYuli
Cc: dmitry.torokhov, guanwentao, linux-input, linux-kernel,
linux-trace-kernel, mhiramat, niecheng1, wangyuli, zhanjun,
Winston Wen
In-Reply-To: <20250722202551.1614f59a@gandalf.local.home>
On 2025-07-22 20:25, Steven Rostedt wrote:
> On Thu, 10 Jul 2025 15:31:38 +0800
> WangYuli <wangyuli@uniontech.com> wrote:
>
>> diff --git a/include/trace/events/input.h b/include/trace/events/input.h
>> new file mode 100644
>> index 000000000000..3c5ffcfb7c8d
>> --- /dev/null
>> +++ b/include/trace/events/input.h
>> @@ -0,0 +1,251 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* input tracepoints
>> + *
>> + * Copyright (C) 2025 WangYuli <wangyuli@uniontech.com>
>> + */
>> +#undef TRACE_SYSTEM
>> +#define TRACE_SYSTEM input
>> +
>> +#if !defined(_TRACE_INPUT_H) || defined(TRACE_HEADER_MULTI_READ)
>> +#define _TRACE_INPUT_H
>> +
>> +#include <linux/tracepoint.h>
>> +#include <linux/input.h>
>> +
>> +/**
>> + * input_event - called when an input event is processed
>> + * @dev: input device that generated the event
>> + * @type: event type (EV_KEY, EV_REL, EV_ABS, etc.)
>> + * @code: event code within the type
>> + * @value: event value
>> + *
>> + * This tracepoint fires for every input event processed by the input core.
>> + * It can be used to monitor input device activity and debug input issues.
>> + */
I've always been worried about adding tracepoint instrumentation of the
input subsystem that includes the actual keystrokes into the event
payload. What I'm trying to avoid here is people leaking their password
by mistake just because they happened to record a trace while
typing on their keyboard.
I don't mind if this gets enabled with a new kernel command line
options "tracing_leak_my_credentials=yes" or such, but I'd try to
avoid making it easy to enable by mistake unless this information
is specifically needed.
But maybe I'm being too careful and people should really learn not
to share kernel traces with others.
Thoughts ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply
* Re: [PATCH 1/2] input: Add tracepoint support
From: Steven Rostedt @ 2025-07-23 0:25 UTC (permalink / raw)
To: WangYuli
Cc: dmitry.torokhov, guanwentao, linux-input, linux-kernel,
linux-trace-kernel, mathieu.desnoyers, mhiramat, niecheng1,
wangyuli, zhanjun, Winston Wen
In-Reply-To: <C1C54D7FA3DF3958+20250710073148.3994900-1-wangyuli@uniontech.com>
On Thu, 10 Jul 2025 15:31:38 +0800
WangYuli <wangyuli@uniontech.com> wrote:
> diff --git a/include/trace/events/input.h b/include/trace/events/input.h
> new file mode 100644
> index 000000000000..3c5ffcfb7c8d
> --- /dev/null
> +++ b/include/trace/events/input.h
> @@ -0,0 +1,251 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* input tracepoints
> + *
> + * Copyright (C) 2025 WangYuli <wangyuli@uniontech.com>
> + */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM input
> +
> +#if !defined(_TRACE_INPUT_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_INPUT_H
> +
> +#include <linux/tracepoint.h>
> +#include <linux/input.h>
> +
> +/**
> + * input_event - called when an input event is processed
> + * @dev: input device that generated the event
> + * @type: event type (EV_KEY, EV_REL, EV_ABS, etc.)
> + * @code: event code within the type
> + * @value: event value
> + *
> + * This tracepoint fires for every input event processed by the input core.
> + * It can be used to monitor input device activity and debug input issues.
> + */
> +TRACE_EVENT(
> + input_event,
> +
> + TP_PROTO(struct input_dev *dev, unsigned int type, unsigned int code,
> + int value),
> +
> + TP_ARGS(dev, type, code, value),
> +
> + TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __field(
> + unsigned int, type) __field(unsigned int, code)
> + __field(int, value) __field(u16, bustype)
> + __field(u16, vendor)
> + __field(u16, product)),
> +
The contents of the tracepoints in the subsystems are determined by the
subsystem maintainers, but please follow the tracepoint formatting. The
above is horrible. It should look like a structure layout. One wouldn't
write:
struct entry { char *name;
unsigned int type; unsigned int code;
int value; u16 bustype;
u16 vendor;
u16 product; };
That's what the above looks like. It should be instead:
TP_STRUCT__entry(
__string( name, dev->name ?: "unknown" )
__field( unsigned int, type )
__field( unsigned int, code )
__field( int, value )
__field( u16, bustype )
__field( u16, vendor )
__field( u16, product )
),
So the fields can be easily visible and easily reviewed.
> + TP_fast_assign(__assign_str(name); __entry->type = type;
> + __entry->code = code; __entry->value = value;
> + __entry->bustype = dev->id.bustype;
> + __entry->vendor = dev->id.vendor;
> + __entry->product = dev->id.product;),
> +
> + TP_printk(
> + "dev=%s type=%u code=%u value=%d bus=%04x vendor=%04x product=%04x",
> + __get_str(name), __entry->type, __entry->code, __entry->value,
> + __entry->bustype, __entry->vendor, __entry->product));
> +
> +/**
> + * input_device_register - called when an input device is registered
> + * @dev: input device being registered
> + *
> + * This tracepoint fires when a new input device is registered with the
> + * input subsystem. Useful for monitoring device hotplug events.
> + */
> +TRACE_EVENT(
> + input_device_register,
> +
> + TP_PROTO(struct input_dev *dev),
> +
> + TP_ARGS(dev),
> +
> + TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __string(
> + phys, dev->phys ?: "") __field(u16, bustype)
> + __field(u16, vendor) __field(u16, product)
> + __field(u16, version)),
Same here.
> +
> + TP_fast_assign(__assign_str(name); __assign_str(phys);
> + __entry->bustype = dev->id.bustype;
> + __entry->vendor = dev->id.vendor;
> + __entry->product = dev->id.product;
> + __entry->version = dev->id.version;),
> +
> + TP_printk(
> + "dev=%s phys=%s bus=%04x vendor=%04x product=%04x version=%04x",
> + __get_str(name), __get_str(phys), __entry->bustype,
> + __entry->vendor, __entry->product, __entry->version));
> +
> +/**
> + * input_device_unregister - called when an input device is unregistered
> + * @dev: input device being unregistered
> + *
> + * This tracepoint fires when an input device is unregistered from the
> + * input subsystem. Useful for monitoring device hotplug events.
> + */
> +TRACE_EVENT(input_device_unregister,
> +
> + TP_PROTO(struct input_dev *dev),
> +
> + TP_ARGS(dev),
> +
> + TP_STRUCT__entry(__string(name, dev->name ?: "unknown") __string(
> + phys, dev->phys ?: "") __field(u16, bustype)
> + __field(u16, vendor)
> + __field(u16, product)),
ditto.
> +
> + TP_fast_assign(__assign_str(name); __assign_str(phys);
> + __entry->bustype = dev->id.bustype;
> + __entry->vendor = dev->id.vendor;
> + __entry->product = dev->id.product;),
> +
> + TP_printk("dev=%s phys=%s bus=%04x vendor=%04x product=%04x",
> + __get_str(name), __get_str(phys), __entry->bustype,
> + __entry->vendor, __entry->product));
> +
> +/**
> + * input_handler_connect - called when an input handler connects to a device
> + * @handler: input handler
> + * @dev: input device
> + * @minor: assigned minor number (if applicable)
> + *
> + * This tracepoint fires when an input handler (like evdev, mousedev) connects
> + * to an input device, creating a new input handle.
> + */
> +TRACE_EVENT(input_handler_connect,
> +
> + TP_PROTO(struct input_handler *handler, struct input_dev *dev,
> + int minor),
> +
> + TP_ARGS(handler, dev, minor),
> +
> + TP_STRUCT__entry(__string(handler_name, handler->name)
> + __string(dev_name, dev->name ?: "unknown")
> + __field(int, minor)),
ditto.
> +
> + TP_fast_assign(__assign_str(handler_name); __assign_str(dev_name);
> + __entry->minor = minor;),
> +
> + TP_printk("handler=%s dev=%s minor=%d", __get_str(handler_name),
> + __get_str(dev_name), __entry->minor));
> +
> +/**
> + * input_grab_device - called when a device is grabbed by a handler
> + * @dev: input device being grabbed
> + * @handle: input handle doing the grab
> + *
> + * This tracepoint fires when an input device is grabbed exclusively
> + * by an input handler, typically for security or special processing.
> + */
> +TRACE_EVENT(input_grab_device,
> +
> + TP_PROTO(struct input_dev *dev, struct input_handle *handle),
> +
> + TP_ARGS(dev, handle),
> +
> + TP_STRUCT__entry(__string(dev_name, dev->name ?: "unknown")
> + __string(handler_name,
> + handle->handler->name)),
ditto.
> +
> + TP_fast_assign(__assign_str(dev_name); __assign_str(handler_name);),
> +
> + TP_printk("dev=%s grabbed_by=%s", __get_str(dev_name),
> + __get_str(handler_name)));
> +
> +/**
> + * input_release_device - called when a grabbed device is released
> + * @dev: input device being released
> + * @handle: input handle releasing the grab
> + *
> + * This tracepoint fires when an input device grab is released.
> + */
> +TRACE_EVENT(input_release_device,
> +
> + TP_PROTO(struct input_dev *dev, struct input_handle *handle),
> +
> + TP_ARGS(dev, handle),
> +
> + TP_STRUCT__entry(__string(dev_name, dev->name ?: "unknown")
> + __string(handler_name,
> + handle->handler->name)),
ditto.
> +
> + TP_fast_assign(__assign_str(dev_name); __assign_str(handler_name);),
> +
> + TP_printk("dev=%s released_by=%s", __get_str(dev_name),
> + __get_str(handler_name)));
> +
> +DECLARE_EVENT_CLASS(input_device_state,
> +
> + TP_PROTO(struct input_dev *dev),
> +
> + TP_ARGS(dev),
> +
> + TP_STRUCT__entry(__string(name, dev->name ?: "unknown")
> + __field(unsigned int, users)
> + __field(bool, inhibited)),
ditto.
-- Steve
> +
> + TP_fast_assign(__assign_str(name);
> + __entry->users = dev->users;
> + __entry->inhibited = dev->inhibited;),
> +
> + TP_printk("dev=%s users=%u inhibited=%s", __get_str(name),
> + __entry->users,
> + __entry->inhibited ? "true" : "false"));
> +
^ 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