* Re: [PATCH] Input: sur40: fix MAX_CONTACTS value based on PixelSense specification
From: sashiko-bot @ 2026-06-14 23:26 UTC (permalink / raw)
To: Oliver; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260614230847.4938-1-oliverburns.kernel@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Synchronously freeing the driver state in `sur40_disconnect()` while the V4L2 device may still be open leads to use-after-free vulnerabilities.
- [High] The error path in `sur40_probe` incorrectly calls `input_free_device()` on a successfully registered input device instead of `input_unregister_device()`, leading to potential use-after-free and state corruption.
- [High] The input device is registered and exposed to userspace before the driver's private video queue and V4L2 components are fully initialized.
--
commit 36ed451619f33b71bb9cc6a733570423cba1565e
Author: Oliver <oliverburns.kernel@gmail.com>
Input: sur40: fix MAX_CONTACTS value based on PixelSense specification
This patch updates the maximum number of contacts for the Samsung SUR40
touchscreen to 52 to match the documented hardware specification. It also
removes an older FIXME comment that guessed the maximum contacts was 64.
> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index fe63d53d56db0..77ec2c94b91fd 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
[ ... ]
> @@ -128,8 +128,8 @@ struct sur40_image_header {
> /* polling interval (ms) */
> #define POLL_INTERVAL 1
>
> -/* maximum number of contacts FIXME: this is a guess? */
> -#define MAX_CONTACTS 64
> +/* maximum number of contacts */
> +#define MAX_CONTACTS 52
>
> /* control commands */
> #define SUR40_GET_VERSION 0xb0 /* 12 bytes string */
[Severity: High]
This isn't a bug introduced by this patch, but I noticed something in
sur40_probe(). The input device is registered and exposed to userspace
before the driver's private video queue and V4L2 components are fully
initialized:
drivers/input/touchscreen/sur40.c:sur40_probe() {
...
/* register the polled input device */
error = input_register_device(input);
if (error) {
dev_err(&interface->dev,
"Unable to register polled input device.");
goto err_free_buffer;
}
...
/* initialize the queue */
error = vb2_queue_init(&sur40->queue);
...
}
Does this leave a window where an open operation could trigger sur40_open()
and start the sur40_poll() worker thread? If the worker thread invokes
sur40_process_video() and checks vb2_start_streaming_called(&sur40->queue)
concurrently with the main thread's initialization, could this cause a data
race regression on partially initialized structures?
[Severity: High]
This is a pre-existing issue, but looking at the sur40_probe() error handling,
if a failure occurs after the input device is successfully registered, the
code jumps to error labels that fall through to err_free_input:
drivers/input/touchscreen/sur40.c:sur40_probe() {
...
err_unreg_video:
video_unregister_device(&sur40->vdev);
err_unreg_v4l2:
v4l2_device_unregister(&sur40->v4l2);
err_free_buffer:
kfree(sur40->bulk_in_buffer);
err_free_input:
input_free_device(input);
err_free_dev:
kfree(sur40);
return error;
}
Can calling input_free_device() on a successfully registered device lead
to state corruption? If a userspace process opens this lingering device node,
would the polling thread dereference the freed sur40 pointer and cause a
use-after-free regression on the error path? It seems like
input_unregister_device() should be used instead.
[Severity: Critical]
This is also a pre-existing issue, but I noticed sur40_disconnect()
synchronously frees the driver state while the V4L2 device may still be open:
drivers/input/touchscreen/sur40.c:sur40_disconnect() {
struct sur40_state *sur40 = usb_get_intfdata(interface);
v4l2_ctrl_handler_free(&sur40->hdl);
video_unregister_device(&sur40->vdev);
v4l2_device_unregister(&sur40->v4l2);
input_unregister_device(sur40->input);
kfree(sur40->bulk_in_buffer);
kfree(sur40);
...
}
Could this lead to a use-after-free regression? If userspace holds a
file descriptor open to the video device during a USB disconnect,
video_unregister_device() does not forcefully close the active file
descriptor. When userspace later calls ioctl(), poll(), or close() on the
descriptor, wouldn't the V4L2 core invoke file operations like
vb2_fop_release() which dereference the already freed sur40 memory?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260614230847.4938-1-oliverburns.kernel@gmail.com?part=1
^ permalink raw reply
* [PATCH] Input: sur40: fix MAX_CONTACTS value based on PixelSense specification
From: Oliver @ 2026-06-14 23:08 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: kees, linux-input, linux-kernel, Oliver
The Samsung SUR40 with Microsoft PixelSense is offically specified to
support 52 simultaneuous touch contacts, not 64. The value of 64 was an
unverified guess as noted by the FIXME comment. Update MAX_CONTACTS to
match the documented hardware specification and remove the FIXME.
Signed-off-by: Oliver <oliverburns.kernel@gmail.com>
---
drivers/input/touchscreen/sur40.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index fe63d53d56db..77ec2c94b91f 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -128,8 +128,8 @@ struct sur40_image_header {
/* polling interval (ms) */
#define POLL_INTERVAL 1
-/* maximum number of contacts FIXME: this is a guess? */
-#define MAX_CONTACTS 64
+/* maximum number of contacts */
+#define MAX_CONTACTS 52
/* control commands */
#define SUR40_GET_VERSION 0xb0 /* 12 bytes string */
--
2.54.0
^ permalink raw reply related
* Re: [BUG] KASAN: slab-use-after-free in _raw_spin_lock_irqsave from hid-sensor-custom
From: Shuangpeng @ 2026-06-14 21:50 UTC (permalink / raw)
To: Maxwell Doose
Cc: jikos, jic23, srinivas.pandruvada, bentiss, linux-input,
linux-iio, linux-kernel
In-Reply-To: <20260614163518.2a265172@linuxescape>
> On Jun 14, 2026, at 17:35, Maxwell Doose <m32285159@gmail.com> wrote:
>
> On Sun, 14 Jun 2026 17:24:12 -0400
> Shuangpeng <shuangpeng.kernel@gmail.com> wrote:
>
>>> On Jun 14, 2026, at 17:02, Maxwell Doose <m32285159@gmail.com> wrote:
>>>
>>> Hi Shuangpeng,
>>>
>>> On Sun, 14 Jun 2026 15:19:21 -0400
>>> Shuangpeng Bai <shuangpeng.kernel@gmail.com> wrote:
>>>
>>>> I hit the following report while testing current upstream kernel:
>>>>
>>>> KASAN: slab-use-after-free in _raw_spin_lock_irqsave from
>>>> hid-sensor-custom
>>>>
>>>> on commit: e8c2f9fdadee7cbc75134dc463c1e0d856d6e5c7 (May 25 2026)
>>>>
>>>
>>> Is this correct? It seems to point to changes in HPFS.
>>>
>>
>> That commit was the linux.git HEAD where I reproduced the crash. I did not mean
>> to imply that the HPFS merge introduced the issue.
>>
>
> If you have (a lot of) time, it may be worth trying git bisect to get
> the exact commit. No worries if you don't of course, but it would be
> incredibly helpful to the HID folks.
>
Thanks for the suggestion.
Unfortunately, I don’t have enough time to run a bisect right now,
but I’ll keep it in mind and will follow up if I get a chance to look
into it later.
Best,
Shuangpeng
> --
> best regards,
> max
>
>
>
>>>>
>>>> The reproducer and .config files are here.
>>>> https://gist.github.com/shuangpengbai/d82ac0d19fda016e81d7fa1ab028d967
>>>>
>>>> I'm happy to test debug patches or provide additional information.
>>>>
>>>> Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
>>>>
>>>
>>> This bug report also seems to have nothing to do with IIO after
>>> investigating the call trace, seems more like for the HID/input folks
>>> than iio. HID folks, seems like it was caused here:
>>>
>>> [ 73.163547][ T8356] hid_sensor_custom_poll (include/linux/poll.h:45 drivers/hid/hid-sensor-custom.c:706)
>>>
>>> before _raw_spin_lock_irqsave() gets called and KASAN triggers the slab-use-after-free.
>>>
>>
>> Thanks for checking.
>>
>> I agree that this does not look like an IIO-specific issue from the trace. The crash
>> is reported from hid_sensor_custom_poll() in drivers/hid/hid-sensor-custom.c.
^ permalink raw reply
* Re: [BUG] KASAN: slab-use-after-free in _raw_spin_lock_irqsave from hid-sensor-custom
From: Maxwell Doose @ 2026-06-14 21:35 UTC (permalink / raw)
To: Shuangpeng
Cc: jikos, jic23, srinivas.pandruvada, bentiss, linux-input,
linux-iio, linux-kernel
In-Reply-To: <C79160E7-5244-407D-81E7-2A1B14231B22@gmail.com>
On Sun, 14 Jun 2026 17:24:12 -0400
Shuangpeng <shuangpeng.kernel@gmail.com> wrote:
> > On Jun 14, 2026, at 17:02, Maxwell Doose <m32285159@gmail.com> wrote:
> >
> > Hi Shuangpeng,
> >
> > On Sun, 14 Jun 2026 15:19:21 -0400
> > Shuangpeng Bai <shuangpeng.kernel@gmail.com> wrote:
> >
> >> I hit the following report while testing current upstream kernel:
> >>
> >> KASAN: slab-use-after-free in _raw_spin_lock_irqsave from
> >> hid-sensor-custom
> >>
> >> on commit: e8c2f9fdadee7cbc75134dc463c1e0d856d6e5c7 (May 25 2026)
> >>
> >
> > Is this correct? It seems to point to changes in HPFS.
> >
>
> That commit was the linux.git HEAD where I reproduced the crash. I did not mean
> to imply that the HPFS merge introduced the issue.
>
If you have (a lot of) time, it may be worth trying git bisect to get
the exact commit. No worries if you don't of course, but it would be
incredibly helpful to the HID folks.
--
best regards,
max
> >>
> >> The reproducer and .config files are here.
> >> https://gist.github.com/shuangpengbai/d82ac0d19fda016e81d7fa1ab028d967
> >>
> >> I'm happy to test debug patches or provide additional information.
> >>
> >> Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
> >>
> >
> > This bug report also seems to have nothing to do with IIO after
> > investigating the call trace, seems more like for the HID/input folks
> > than iio. HID folks, seems like it was caused here:
> >
> > [ 73.163547][ T8356] hid_sensor_custom_poll (include/linux/poll.h:45 drivers/hid/hid-sensor-custom.c:706)
> >
> > before _raw_spin_lock_irqsave() gets called and KASAN triggers the slab-use-after-free.
> >
>
> Thanks for checking.
>
> I agree that this does not look like an IIO-specific issue from the trace. The crash
> is reported from hid_sensor_custom_poll() in drivers/hid/hid-sensor-custom.c.
>
^ permalink raw reply
* Re: [PATCH] Input: mms114 - reject an oversized device packet size
From: Dmitry Torokhov @ 2026-06-14 21:35 UTC (permalink / raw)
To: hexlabsecurity; +Cc: linux-kernel, linux-input, Joonyoung Shim, Kyungmin Park
In-Reply-To: <20260612-b4-disp-dc4b8dc4-v1-1-d7cb0a828d92@proton.me>
On Fri, Jun 12, 2026 at 11:21:14PM -0500, Bryam Vargas via B4 Relay wrote:
> From: Bryam Vargas <hexlabsecurity@proton.me>
>
> mms114_interrupt() reads a packet of touch data from the device into a
> fixed-size on-stack buffer
>
> struct mms114_touch touch[MMS114_MAX_TOUCH];
>
> which holds MMS114_MAX_TOUCH (10) events of MMS114_EVENT_SIZE (8) bytes,
> i.e. 80 bytes. The length of the I2C read into it is taken verbatim from
> the device:
>
> packet_size = mms114_read_reg(data, MMS114_PACKET_SIZE);
> if (packet_size <= 0)
> goto out;
> ...
> error = __mms114_read_reg(data, MMS114_INFORMATION, packet_size,
> (u8 *)touch);
>
> packet_size is a single device register byte (0x0F) and the only check
> is the lower bound packet_size <= 0; it is never bounded against the
> size of touch[]. A malfunctioning, malicious or counterfeit controller
> (or an attacker tampering with the I2C bus) can report a packet_size of
> up to 255, so __mms114_read_reg() writes up to 175 bytes past the end of
> touch[] on the IRQ-thread stack: a stack out-of-bounds write that can
> overwrite the stack canary, saved registers and the return address.
>
> A well-formed device never reports more than the buffer holds, so reject
> an oversized packet and drop the report, consistent with the handler's
> other error paths, rather than reading past the buffer.
>
> Fixes: 07b8481d4aff ("Input: add MELFAS mms114 touchscreen driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
> drivers/input/touchscreen/mms114.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index af462086a65c..4c75f16c503d 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -226,6 +226,13 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
> if (packet_size <= 0)
> goto out;
>
> + /* the device controls packet_size; reject anything too big for touch[] */
> + if (packet_size > (int)sizeof(touch)) {
I gonna drop this cast (as thankfully we are not using -Wsign-compare)
and apply, thank you.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [BUG] KASAN: slab-use-after-free in _raw_spin_lock_irqsave from hid-sensor-custom
From: Shuangpeng @ 2026-06-14 21:24 UTC (permalink / raw)
To: Maxwell Doose
Cc: jikos, jic23, srinivas.pandruvada, bentiss, linux-input,
linux-iio, linux-kernel
In-Reply-To: <20260614160213.085e1efc@linuxescape>
> On Jun 14, 2026, at 17:02, Maxwell Doose <m32285159@gmail.com> wrote:
>
> Hi Shuangpeng,
>
> On Sun, 14 Jun 2026 15:19:21 -0400
> Shuangpeng Bai <shuangpeng.kernel@gmail.com> wrote:
>
>> I hit the following report while testing current upstream kernel:
>>
>> KASAN: slab-use-after-free in _raw_spin_lock_irqsave from
>> hid-sensor-custom
>>
>> on commit: e8c2f9fdadee7cbc75134dc463c1e0d856d6e5c7 (May 25 2026)
>>
>
> Is this correct? It seems to point to changes in HPFS.
>
That commit was the linux.git HEAD where I reproduced the crash. I did not mean
to imply that the HPFS merge introduced the issue.
>>
>> The reproducer and .config files are here.
>> https://gist.github.com/shuangpengbai/d82ac0d19fda016e81d7fa1ab028d967
>>
>> I'm happy to test debug patches or provide additional information.
>>
>> Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
>>
>
> This bug report also seems to have nothing to do with IIO after
> investigating the call trace, seems more like for the HID/input folks
> than iio. HID folks, seems like it was caused here:
>
> [ 73.163547][ T8356] hid_sensor_custom_poll (include/linux/poll.h:45 drivers/hid/hid-sensor-custom.c:706)
>
> before _raw_spin_lock_irqsave() gets called and KASAN triggers the slab-use-after-free.
>
Thanks for checking.
I agree that this does not look like an IIO-specific issue from the trace. The crash
is reported from hid_sensor_custom_poll() in drivers/hid/hid-sensor-custom.c.
> --
> best regards,
> max
^ permalink raw reply
* Re: [PATCH] Input: goodix - clamp the device-reported contact count
From: Dmitry Torokhov @ 2026-06-14 21:02 UTC (permalink / raw)
To: Hans de Goede; +Cc: hexlabsecurity, linux-input, linux-kernel
In-Reply-To: <6f529998-2b4f-441e-88be-fbc4eb33461c@kernel.org>
On Sun, Jun 14, 2026 at 01:44:07PM +0200, Hans de Goede wrote:
> Hi,
>
> On 13-Jun-26 04:10, Bryam Vargas via B4 Relay wrote:
> > From: Bryam Vargas <hexlabsecurity@proton.me>
> >
> > goodix_ts_read_input_report() copies the number of touch points reported
> > by the device into an on-stack buffer
> >
> > u8 point_data[2 + GOODIX_MAX_CONTACT_SIZE * GOODIX_MAX_CONTACTS];
> >
> > which is sized for at most GOODIX_MAX_CONTACTS (10) contacts. The only
> > runtime check bounds the per-interrupt count against ts->max_touch_num,
> > but that value is taken verbatim from a 4-bit field of the device
> > configuration block and is never clamped:
> >
> > ts->max_touch_num = ts->config[MAX_CONTACTS_LOC] & 0x0f;
> >
> > The nibble can be 0..15, so a malfunctioning, malicious or counterfeit
> > controller (or an attacker tampering with the I2C bus) can advertise up
> > to 15 contacts. goodix_ts_read_input_report() then accepts a touch_num
> > of up to 15 and the second goodix_i2c_read() writes
> > ts->contact_size * (touch_num - 1) bytes past the one-contact header into
> > point_data - up to 30 bytes (45 with the 9-byte report format) beyond the
> > 92-byte buffer: a stack out-of-bounds write.
> >
> > Clamp max_touch_num to GOODIX_MAX_CONTACTS, the number of contacts
> > point_data[] is sized for, when reading it from the configuration.
> >
> > Fixes: a7ac7c95d468 ("Input: goodix - use max touch number from device config")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
>
> Thanks, patch looks good to me:
>
> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
>
> Regards,
>
> Hans
>
>
> > ---
> > drivers/input/touchscreen/goodix.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> > index f8798d11ec03..17fcfe45988c 100644
> > --- a/drivers/input/touchscreen/goodix.c
> > +++ b/drivers/input/touchscreen/goodix.c
> > @@ -1057,7 +1057,8 @@ static void goodix_read_config(struct goodix_ts_data *ts)
> > }
> >
> > ts->int_trigger_type = ts->config[TRIGGER_LOC] & 0x03;
> > - ts->max_touch_num = ts->config[MAX_CONTACTS_LOC] & 0x0f;
> > + ts->max_touch_num = min(ts->config[MAX_CONTACTS_LOC] & 0x0f,
> > + GOODIX_MAX_CONTACTS);
Should we drop the report if is has bogus data in it?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [BUG] KASAN: slab-use-after-free in _raw_spin_lock_irqsave from hid-sensor-custom
From: Maxwell Doose @ 2026-06-14 21:02 UTC (permalink / raw)
To: Shuangpeng Bai
Cc: jikos, jic23, srinivas.pandruvada, bentiss, linux-input,
linux-iio, linux-kernel
In-Reply-To: <178144969601.60470.12928355382146160896@gmail.com>
Hi Shuangpeng,
On Sun, 14 Jun 2026 15:19:21 -0400
Shuangpeng Bai <shuangpeng.kernel@gmail.com> wrote:
> I hit the following report while testing current upstream kernel:
>
> KASAN: slab-use-after-free in _raw_spin_lock_irqsave from
> hid-sensor-custom
>
> on commit: e8c2f9fdadee7cbc75134dc463c1e0d856d6e5c7 (May 25 2026)
>
Is this correct? It seems to point to changes in HPFS.
>
> The reproducer and .config files are here.
> https://gist.github.com/shuangpengbai/d82ac0d19fda016e81d7fa1ab028d967
>
> I'm happy to test debug patches or provide additional information.
>
> Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
>
This bug report also seems to have nothing to do with IIO after
investigating the call trace, seems more like for the HID/input folks
than iio. HID folks, seems like it was caused here:
[ 73.163547][ T8356] hid_sensor_custom_poll (include/linux/poll.h:45 drivers/hid/hid-sensor-custom.c:706)
before _raw_spin_lock_irqsave() gets called and KASAN triggers the slab-use-after-free.
--
best regards,
max
^ permalink raw reply
* Re: [PATCH] Input: touchwin - reset the packet index on every complete packet
From: Dmitry Torokhov @ 2026-06-14 20:59 UTC (permalink / raw)
To: hexlabsecurity; +Cc: linux-input, Rick Koch, linux-kernel
In-Reply-To: <20260613-b4-disp-69921bfd-v1-1-82c036899959@proton.me>
On Sat, Jun 13, 2026 at 08:07:20PM -0500, Bryam Vargas via B4 Relay wrote:
> From: Bryam Vargas <hexlabsecurity@proton.me>
>
> tw_interrupt() accumulates each non-zero serial byte into a fixed
> three-byte buffer with a running index that is only reset once a full
> packet has been received *and* the device's two Y bytes agree:
>
> tw->data[tw->idx++] = data;
> if (tw->idx == TW_LENGTH && tw->data[1] == tw->data[2]) {
> ...
> tw->idx = 0;
> }
>
> The reset is gated on tw->data[1] == tw->data[2], a value the device
> controls. A malicious, malfunctioning or counterfeit Touchwindow
> peripheral can stream non-zero bytes whose 2nd and 3rd bytes differ: the
> index reaches TW_LENGTH without the equality holding, is never reset, and
> keeps growing, so tw->data[tw->idx++] walks off the end of the three-byte
> array and the rest of the heap-allocated struct tw, one attacker-chosen
> byte at a time -- an unbounded, device-driven heap out-of-bounds write.
>
> Reset the index on every completed packet and report an event only when
> the two Y bytes match, like the other serio touchscreen drivers do.
>
> Fixes: 11ea3173d5f2 ("Input: add driver for Touchwin serial touchscreens")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: apple_z2 - bound the device-reported finger count
From: Dmitry Torokhov @ 2026-06-14 20:56 UTC (permalink / raw)
To: hexlabsecurity
Cc: Sasha Finkelstein, linux-kernel, Janne Grunau, linux-arm-kernel,
linux-input, Sven Peter, asahi, Neal Gompa
In-Reply-To: <20260613-b4-disp-4ebcbd68-v2-1-0161acfbd688@proton.me>
Hi Bryam,
On Sat, Jun 13, 2026 at 08:22:51PM -0500, Bryam Vargas via B4 Relay wrote:
> From: Bryam Vargas <hexlabsecurity@proton.me>
>
> apple_z2_parse_touches() takes the finger count from the touch
> controller's report and loops over that many fixed-size finger records
> without ever checking the count against the length of the report:
>
> nfingers = msg[APPLE_Z2_NUM_FINGERS_OFFSET];
> fingers = (struct apple_z2_finger *)(msg + APPLE_Z2_FINGERS_OFFSET);
> for (i = 0; i < nfingers; i++)
> /* read fingers[i] ... */
>
> msg points into the fixed 4000-byte z2->rx_buf and nfingers is a single
> device-supplied byte, so it can be as large as 255. A malicious,
> malfunctioning or counterfeit controller (or an interposer on the SPI
> bus) can report a large finger count in a short packet, making the loop
> read up to 255 * sizeof(struct apple_z2_finger) bytes starting 24 bytes
> into msg -- far past the 4000-byte buffer. This is a controller-driven
> heap out-of-bounds read, and the finger fields that are read (position,
> pressure, touch and tool dimensions) are forwarded to userspace as input
> events, leaking adjacent kernel memory.
>
> Bound the device-reported count to the number of finger records the
> report actually carries.
As Sashiko mentioned, if we do not trust hardware to send valid data we
should also validate that packet size supplied by the device is
reasonable.
Also I wonder why would we want to report some of fingers in case when
device sends bogus number of contacts? I'd drop such packet (maybe
logging ratelimited or "once" message).
You can ignore Sahiko's comment about __free(kfree) not handling error
pointers.
Thanks.
--
Dmitry
^ permalink raw reply
* [BUG] KASAN: slab-use-after-free in _raw_spin_lock_irqsave from hid-sensor-custom
From: Shuangpeng Bai @ 2026-06-14 19:19 UTC (permalink / raw)
To: jikos, jic23, srinivas.pandruvada, bentiss, linux-input,
linux-iio, linux-kernel
Hi Kernel Maintainers,
I hit the following report while testing current upstream kernel:
KASAN: slab-use-after-free in _raw_spin_lock_irqsave from hid-sensor-custom
on commit: e8c2f9fdadee7cbc75134dc463c1e0d856d6e5c7 (May 25 2026)
The reproducer and .config files are here.
https://gist.github.com/shuangpengbai/d82ac0d19fda016e81d7fa1ab028d967
I'm happy to test debug patches or provide additional information.
Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
[ 73.157590][ T8356] BUG: KASAN: slab-use-after-free in _raw_spin_lock_irqsave (include/linux/instrumented.h:112 include/linux/atomic/atomic-instrumented.h:1300 include/asm-generic/qspinlock.h:111 include/linux/spinlock.h:187 include/linux/spinlock_api_smp.h:133 kernel/locking/spinlock.c:166)
[ 73.161235][ T8356] Write of size 4 at addr ffff88810eb72528 by task hid_sensor_cust/8356
[ 73.163453][ T8356] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 73.163457][ T8356] Call Trace:
[ 73.163461][ T8356] <TASK>
[ 73.163464][ T8356] dump_stack_lvl (lib/dump_stack.c:94 lib/dump_stack.c:120)
[ 73.163471][ T8356] print_report (mm/kasan/report.c:378 mm/kasan/report.c:482)
[ 73.163486][ T8356] kasan_report (mm/kasan/report.c:595)
[ 73.163495][ T8356] kasan_check_range (mm/kasan/generic.c:? mm/kasan/generic.c:200)
[ 73.163500][ T8356] _raw_spin_lock_irqsave (include/linux/instrumented.h:112 include/linux/atomic/atomic-instrumented.h:1300 include/asm-generic/qspinlock.h:111 include/linux/spinlock.h:187 include/linux/spinlock_api_smp.h:133 kernel/locking/spinlock.c:166)
[ 73.163539][ T8356] add_wait_queue (kernel/sched/wait.c:23)
[ 73.163547][ T8356] hid_sensor_custom_poll (include/linux/poll.h:45 drivers/hid/hid-sensor-custom.c:706)
[ 73.163556][ T8356] do_sys_poll (include/linux/poll.h:82 fs/select.c:877 fs/select.c:920 fs/select.c:1015)
[ 73.163692][ T8356] __x64_sys_poll (fs/select.c:1072 fs/select.c:1060 fs/select.c:1060)
[ 73.163708][ T8356] do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
[ 73.163714][ T8356] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
[ 73.163755][ T8356] </TASK>
[ 73.214615][ T8356] Freed by task 781 on cpu 1 at 72.569353s:
[ 73.215524][ T8356] kasan_save_track (mm/kasan/common.c:57 mm/kasan/common.c:78)
[ 73.216247][ T8356] kasan_save_free_info (mm/kasan/generic.c:584)
[ 73.217018][ T8356] __kasan_slab_free (mm/kasan/common.c:253 mm/kasan/common.c:285)
[ 73.217739][ T8356] kfree (include/linux/kasan.h:235 mm/slub.c:2689 mm/slub.c:6251 mm/slub.c:6566)
[ 73.218335][ T8356] devres_release_all (drivers/base/devres.c:50 drivers/base/devres.c:547 drivers/base/devres.c:576)
[ 73.219108][ T8356] device_release_driver_internal (drivers/base/dd.c:598 drivers/base/dd.c:1357 drivers/base/dd.c:1375)
[ 73.220034][ T8356] bus_remove_device (drivers/base/bus.c:657)
[ 73.220796][ T8356] device_del (drivers/base/core.c:3895)
[ 73.221458][ T8356] platform_device_unregister (drivers/base/platform.c:797 drivers/base/platform.c:839)
[ 73.222310][ T8356] mfd_remove_devices_fn (drivers/mfd/mfd-core.c:385)
[ 73.223121][ T8356] device_for_each_child_reverse (drivers/base/core.c:4065)
[ 73.224033][ T8356] mfd_remove_devices (drivers/mfd/mfd-core.c:401)
[ 73.224779][ T8356] hid_device_remove (drivers/hid/hid-core.c:?)
[ 73.225537][ T8356] device_release_driver_internal (drivers/base/dd.c:619 drivers/base/dd.c:1352 drivers/base/dd.c:1375)
[ 73.226449][ T8356] bus_remove_device (drivers/base/bus.c:657)
[ 73.227200][ T8356] device_del (drivers/base/core.c:3895)
[ 73.227857][ T8356] hid_destroy_device (drivers/hid/hid-core.c:3064 drivers/hid/hid-core.c:3086)
[ 73.228617][ T8356] usbhid_disconnect (drivers/hid/usbhid/hid-core.c:1476)
[ 73.238613][ T8356] The buggy address belongs to the object at ffff88810eb72400
[ 73.238613][ T8356] which belongs to the cache kmalloc-512 of size 512
[ 73.240744][ T8356] The buggy address is located 296 bytes inside of
[ 73.240744][ T8356] freed 512-byte region [ffff88810eb72400, ffff88810eb72600)
Best,
Shuangpeng
^ permalink raw reply
* Re: [PATCH 2/9] iio: orientation: hid-sensor-incl-3d: Fix race between callback registration and device exposure
From: Jonathan Cameron @ 2026-06-14 18:24 UTC (permalink / raw)
To: Pandruvada, Srinivas
Cc: dlechner@baylibre.com, archana.patni@linux.intel.com,
hongyan.song@intel.com, nuno.sa@analog.com, jikos@kernel.org,
andy@kernel.org, sanjayembeddedse@gmail.com,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-iio@vger.kernel.org
In-Reply-To: <b3c449ee646fc3bff89dbbdfaf04f731fea87b2b.camel@intel.com>
On Mon, 8 Jun 2026 15:34:05 +0000
"Pandruvada, Srinivas" <srinivas.pandruvada@intel.com> wrote:
> On Sat, 2026-06-06 at 17:07 +0530, Sanjay Chitroda wrote:
> > From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> >
> > The driver registers the IIO device before completing sensor hub
> > callback registration and unregisters callbacks while the IIO device
> > is still exposed during teardown.
> >
> > This creates race windows in both probe and remove paths, which can
> > lead to NULL pointer dereferences or use-after-free.
>
> Reordering is fine, but can you show how this use after free is
> possible?
Agreed - I'm not seeing a definite issue so more info needed.
For now I'm going to mark this changes-requested in patchwork.
It might be a touch slow if someone manages to get buffered capture
up before the callbacks are available, but I think that just means
dropping a few samples?
Jonathan
>
> Thanks,
> Srinivas
^ permalink raw reply
* Re: [PATCH 2/2] iio: hid-sensor-rotation: Fix stale or zero output when reading raw values
From: Jonathan Cameron @ 2026-06-14 16:50 UTC (permalink / raw)
To: Zhang, Lixu
Cc: Jiri Kosina, Srinivas Pandruvada, Benjamin Tissoires,
David Lechner, Nuno Sá, Andy Shevchenko,
linux-input@vger.kernel.org, linux-iio@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <SJ0PR11MB5613EE4659912C262E950C9D93182@SJ0PR11MB5613.namprd11.prod.outlook.com>
On Fri, 12 Jun 2026 05:33:32 +0000
"Zhang, Lixu" <lixu.zhang@intel.com> wrote:
> >-----Original Message-----
> >From: Jonathan Cameron <jic23@kernel.org>
> >Sent: Friday, June 12, 2026 12:35 AM
> >To: Zhang, Lixu <lixu.zhang@intel.com>
> >Cc: Jiri Kosina <jikos@kernel.org>; Srinivas Pandruvada
> ><srinivas.pandruvada@linux.intel.com>; Benjamin Tissoires
> ><bentiss@kernel.org>; David Lechner <dlechner@baylibre.com>; Nuno Sá
> ><nuno.sa@analog.com>; Andy Shevchenko <andy@kernel.org>; linux-
> >input@vger.kernel.org; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org
> >Subject: Re: [PATCH 2/2] iio: hid-sensor-rotation: Fix stale or zero output when
> >reading raw values
> >
> >On Wed, 10 Jun 2026 16:29:10 +0800
> >Zhang Lixu <lixu.zhang@intel.com> wrote:
> >
> >> When reading the raw quaternion attribute (in_rot_quaternion_raw), the
> >> driver currently returns either all zeros (if the sensor was never
> >> enabled) or stale data (if the sensor was previously enabled) because
> >> it reads from the internal buffer without explicitly requesting a new
> >> sample from the sensor.
> >>
> >> To fix this, power up the sensor, call
> >> sensor_hub_input_attr_read_values()
> >> to issue a synchronous GET_REPORT and receive the full quaternion data
> >> directly into a local buffer, then decode the four components.
> >>
> >> Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
> >Hi Zhang,
> >
> >Given this is clearly a fix, can you reply to this thread with an appropriate Fixes
> >tag. Otherwise looks fine to me.
>
> Sure, here is the Fixes tag:
>
> Fixes: fc18dddc0625 ("iio: hid-sensors: Added device rotation support")
>
> >
> >Given timing and I guess that this bug is fairly old, this will probably only go
> >upstream after rc1.
>
> That's perfectly fine. Thanks for the review!
Applied to the fixes-togreg branch of iio.git.
Note I'll be rebasing that on rc1 before sending out a pull request.
Thanks,
Jonathan
>
> Best regards,
> Lixu
>
> >
> >Thanks,
> >
> >Jonathan
> >
>
^ permalink raw reply
* [BUG] KASAN: slab-out-of-bounds in rmi_create_function
From: Shuangpeng Bai @ 2026-06-14 15:16 UTC (permalink / raw)
To: dmitry.torokhov, linux-input, linux-kernel
Hi Kernel Maintainers,
I hit the following report while testing current upstream kernel:
KASAN: slab-out-of-bounds in rmi_create_function
on commit: e8c2f9fdadee7cbc75134dc463c1e0d856d6e5c7 (May 25 2026)
To help trigger the bug more reliably, we applied a minimal diagnostic patch
that only adds delays and print statements.
The reproducer and .config files are here.
https://gist.github.com/shuangpengbai/6f392317d13655f16a8983fe1587dbcc
I'm happy to test debug patches or provide additional information.
Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
[ 128.210187][ T10] BUG: KASAN: slab-out-of-bounds in rmi_create_function (include/linux/instrumented.h:97 include/asm-generic/bitops/instrumented-atomic.h:28 drivers/input/rmi4/rmi_driver.c:861)
[ 128.210768][ T10] Write of size 8 at addr ffff888178e09b50 by task kworker/0:1/10
[ 128.211308][ T10]
[ 128.211489][ T10] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 128.211492][ T10] Workqueue: events acpi_table_events_fn
[ 128.211499][ T10] Call Trace:
[ 128.211501][ T10] <TASK>
[ 128.211503][ T10] dump_stack_lvl (lib/dump_stack.c:94 lib/dump_stack.c:120)
[ 128.211508][ T10] print_report (mm/kasan/report.c:378 mm/kasan/report.c:482)
[ 128.211520][ T10] kasan_report (mm/kasan/report.c:595)
[ 128.211526][ T10] kasan_check_range (mm/kasan/generic.c:? mm/kasan/generic.c:200)
[ 128.211530][ T10] rmi_create_function (include/linux/instrumented.h:97 include/asm-generic/bitops/instrumented-atomic.h:28 drivers/input/rmi4/rmi_driver.c:861)
[ 128.211533][ T10] rmi_scan_pdt (drivers/input/rmi4/rmi_driver.c:525 drivers/input/rmi4/rmi_driver.c:552)
[ 128.211549][ T10] rmi_init_functions (drivers/input/rmi4/rmi_driver.c:1074)
[ 128.211565][ T10] rmi_driver_probe (drivers/input/rmi4/rmi_driver.c:1207)
[ 128.211569][ T10] really_probe (drivers/base/dd.c:? drivers/base/dd.c:709)
[ 128.211571][ T10] __driver_probe_device (drivers/base/dd.c:871)
[ 128.211579][ T10] driver_probe_device (drivers/base/dd.c:901)
[ 128.211581][ T10] __device_attach_driver (drivers/base/dd.c:1029)
[ 128.211587][ T10] bus_for_each_drv (drivers/base/bus.c:500)
[ 128.211600][ T10] __device_attach (drivers/base/dd.c:1101)
[ 128.211613][ T10] device_initial_probe (drivers/base/dd.c:1156)
[ 128.211616][ T10] bus_probe_device (drivers/base/bus.c:613)
[ 128.211619][ T10] device_add (drivers/base/core.c:3706)
[ 128.211623][ T10] rmi_register_transport_device (drivers/input/rmi4/rmi_bus.c:98)
[ 128.211626][ T10] rmi_spi_probe (drivers/input/rmi4/rmi_spi.c:435)
[ 128.211629][ T10] really_probe (drivers/base/dd.c:? drivers/base/dd.c:709)
[ 128.211631][ T10] __driver_probe_device (drivers/base/dd.c:871)
[ 128.211634][ T10] driver_probe_device (drivers/base/dd.c:901)
[ 128.211636][ T10] __device_attach_driver (drivers/base/dd.c:1029)
[ 128.211641][ T10] bus_for_each_drv (drivers/base/bus.c:500)
[ 128.211653][ T10] __device_attach (drivers/base/dd.c:1101)
[ 128.211671][ T10] device_initial_probe (drivers/base/dd.c:1156)
[ 128.211674][ T10] bus_probe_device (drivers/base/bus.c:613)
[ 128.211676][ T10] device_add (drivers/base/core.c:3706)
[ 128.211679][ T10] __spi_add_device (drivers/spi/spi.c:756)
[ 128.211689][ T10] acpi_register_spi_device (drivers/spi/spi.c:786 drivers/spi/spi.c:3055)
[ 128.211697][ T10] acpi_spi_notify (drivers/spi/spi.c:5093)
[ 128.211699][ T10] notifier_call_chain (kernel/notifier.c:85)
[ 128.211702][ T10] blocking_notifier_call_chain (kernel/notifier.c:380)
[ 128.211705][ T10] acpi_generic_device_attach (drivers/acpi/scan.c:2297)
[ 128.211710][ T10] acpi_bus_attach (drivers/acpi/scan.c:2323 drivers/acpi/scan.c:2372)
[ 128.211740][ T10] device_for_each_child (drivers/base/core.c:4035)
[ 128.211751][ T10] acpi_dev_for_each_child (drivers/acpi/bus.c:1208)
[ 128.211761][ T10] acpi_bus_attach (drivers/acpi/scan.c:2393)
[ 128.211776][ T10] device_for_each_child (drivers/base/core.c:4035)
[ 128.211787][ T10] acpi_dev_for_each_child (drivers/acpi/bus.c:1208)
[ 128.211797][ T10] acpi_bus_attach (drivers/acpi/scan.c:2393)
[ 128.211816][ T10] device_for_each_child (drivers/base/core.c:4035)
[ 128.211827][ T10] acpi_dev_for_each_child (drivers/acpi/bus.c:1208)
[ 128.211839][ T10] acpi_bus_attach (drivers/acpi/scan.c:2393)
[ 128.211859][ T10] acpi_bus_scan (drivers/acpi/scan.c:2743)
[ 128.211871][ T10] acpi_table_events_fn (drivers/acpi/scan.c:2933)
[ 128.211874][ T10] process_scheduled_works (kernel/workqueue.c:3314 kernel/workqueue.c:3397)
[ 128.211879][ T10] worker_thread (kernel/workqueue.c:3478)
[ 128.211884][ T10] kthread (kernel/kthread.c:436)
[ 128.211891][ T10] ret_from_fork (kernel/process.c:158)
[ 128.211902][ T10] ret_from_fork_asm (arch/x86/entry/entry_64.S:245)
Best,
Shuangpeng
^ permalink raw reply
* Re: [PATCH v3 5/8] HID: asus: avoid sleeping calls in atomic context
From: Denis Benato @ 2026-06-14 13:33 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: linux-kernel, linux-input, Benjamin Tissoires, Jiri Kosina,
Luke D . Jones, Mateusz Schyboll, Denis Benato, Connor Belli,
sahiko-bot
In-Reply-To: <CAGwozwHXKosBi23oS8uvVrjr+fKZ+O+yUpHb0oT=acFCgRDwng@mail.gmail.com>
On 6/13/26 18:15, Antheas Kapenekakis wrote:
> On Sat, 13 Jun 2026 at 17:30, Denis Benato <denis.benato@linux.dev> wrote:
>> Avoid possibly calling asus_wmi_send_event(): a method that can sleep in
>> asus_raw_event() that is called in atomic context.
>>
>> This commit changes when methods are called, not method parameters:
>> the driver behaves as before.
> Observe the new AI guidelines and add assisted by tags if necessary.
> Or write the patches manually for mainline. This sentence is an AI
> euphemism
No AI was harmed during the production of these patches.
TBF I tried, but it changed too much things I didn't want to
change and then I ran out of credits, so.... I decided good old
manual bricolage was better for this.
> Who let 1489a34e97ef through? I implemented the plumbing for WMI
> callbacks with asus_hid_event last year because WMI cannot be called
> through HID. Brightness events had the exact same problem.
> kbd_led_work/kbd_led_update_all could be renamed and used for this. I
> would rather avoid introducing a new work queue. The one I added is
> enough if not too much
Alright, I'll try to use that queue for everything.
> Antheas
>
>> Fixes: 1489a34e97ef ("HID: asus: Implement Fn+F5 fan control key handler")
>> Reported-by: sahiko-bot@kernel.org
>> Signed-off-by: Denis Benato <denis.benato@linux.dev>
>> ---
>> drivers/hid/hid-asus.c | 67 +++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 56 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>> index f38b18ad65c6..a6467172c455 100644
>> --- a/drivers/hid/hid-asus.c
>> +++ b/drivers/hid/hid-asus.c
>> @@ -150,6 +150,13 @@ struct asus_drvdata {
>> unsigned long battery_next_query;
>> struct work_struct fn_lock_sync_work;
>> bool fn_lock;
>> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
>> + struct work_struct wmi_work;
>> + bool wmi_work_disabled;
>> + u8 wmi_data[FEATURE_KBD_REPORT_SIZE];
>> + int wmi_size;
>> + spinlock_t wmi_lock;
>> +#endif
>> struct asus_xgm_led *xgm_led;
>> };
>>
>> @@ -338,6 +345,7 @@ static int asus_e1239t_event(struct asus_drvdata *drvdat, u8 *data, int size)
>> return 0;
>> }
>>
>> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
>> /*
>> * Send events to asus-wmi driver for handling special keys
>> */
>> @@ -361,6 +369,32 @@ static int asus_wmi_send_event(struct asus_drvdata *drvdata, u8 code)
>> return 0;
>> }
>>
>> +static void asus_wmi_work(struct work_struct *work)
>> +{
>> + struct asus_drvdata *drvdata = container_of(work, struct asus_drvdata, wmi_work);
>> + u8 report_data[FEATURE_KBD_REPORT_SIZE];
>> + int report_size;
>> + unsigned long flags;
>> + int ret;
>> +
>> + spin_lock_irqsave(&drvdata->wmi_lock, flags);
>> + memcpy(report_data, drvdata->wmi_data, drvdata->wmi_size);
>> + report_size = drvdata->wmi_size;
>> + spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
>> +
>> + ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
>> + if (ret) {
>> + if (ret != -ENODEV)
>> + hid_warn(drvdata->hdev, "Failed to notify asus-wmi: %d\n", ret);
>> +
>> + /* Fallback: pass the raw event to the HID core */
>> + hid_report_raw_event(drvdata->hdev, HID_INPUT_REPORT,
>> + report_data, report_size,
>> + report_size, 1);
>> + }
>> +}
>> +#endif
>> +
>> static int asus_event(struct hid_device *hdev, struct hid_field *field,
>> struct hid_usage *usage, __s32 value)
>> {
>> @@ -397,6 +431,9 @@ static int asus_raw_event(struct hid_device *hdev,
>> struct hid_report *report, u8 *data, int size)
>> {
>> struct asus_drvdata *drvdata = hid_get_drvdata(hdev);
>> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
>> + unsigned long flags;
>> +#endif
>>
>> if (drvdata->battery && data[0] == BATTERY_REPORT_ID)
>> return asus_report_battery(drvdata, data, size);
>> @@ -422,19 +459,18 @@ static int asus_raw_event(struct hid_device *hdev,
>> * pass to userspace so it can implement its own fan control.
>> */
>> if (data[1] == ASUS_FAN_CTRL_KEY_CODE) {
>> - int ret = asus_wmi_send_event(drvdata, ASUS_FAN_CTRL_KEY_CODE);
>> -
>> - if (ret == 0) {
>> - /* Successfully handled by asus-wmi, block event */
>> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
>> + spin_lock_irqsave(&drvdata->wmi_lock, flags);
>> + memcpy(drvdata->wmi_data, data, min_t(int, size, sizeof(drvdata->wmi_data)));
>> + drvdata->wmi_size = size;
>> + spin_unlock_irqrestore(&drvdata->wmi_lock, flags);
>> +
>> + if (!drvdata->wmi_work_disabled) {
>> + schedule_work(&drvdata->wmi_work);
>> + /* Successfully handled asynchronously, block event */
>> return -1;
>> }
>> -
>> - /*
>> - * Warn if asus-wmi failed (but not if it's unavailable).
>> - * Let the event reach userspace in all failure cases.
>> - */
>> - if (ret != -ENODEV)
>> - hid_warn(hdev, "Failed to notify asus-wmi: %d\n", ret);
>> +#endif
>> }
>>
>> /*
>> @@ -1350,6 +1386,10 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
>>
>> drvdata->hdev = hdev;
>> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
>> + INIT_WORK(&drvdata->wmi_work, asus_wmi_work);
>> + spin_lock_init(&drvdata->wmi_lock);
>> +#endif
>>
>> if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI)) {
>> ret = asus_battery_probe(hdev);
>> @@ -1460,6 +1500,11 @@ static void asus_remove(struct hid_device *hdev)
>> if (drvdata->quirks & QUIRK_HID_FN_LOCK)
>> cancel_work_sync(&drvdata->fn_lock_sync_work);
>>
>> +#if IS_REACHABLE(CONFIG_ASUS_WMI)
>> + drvdata->wmi_work_disabled = true;
>> + cancel_work_sync(&drvdata->wmi_work);
>> +#endif
>> +
>> if (drvdata->xgm_led)
>> led_classdev_unregister(&drvdata->xgm_led->cdev);
>>
>> --
>> 2.47.3
>>
>>
^ permalink raw reply
* Re: [PATCH v3 4/8] HID: asus: cleanup keyboard listener on failure: avoid use-after-free
From: Denis Benato @ 2026-06-14 12:55 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: linux-kernel, linux-input, Benjamin Tissoires, Jiri Kosina,
Luke D . Jones, Mateusz Schyboll, Denis Benato, Connor Belli,
sahiko-bot
In-Reply-To: <CAGwozwGsDUh_pGjf-NcX6JiYfg8GhbADUcFTFs_E5BsuktdKaA@mail.gmail.com>
On 6/13/26 17:57, Antheas Kapenekakis wrote:
> On Sat, 13 Jun 2026 at 17:30, Denis Benato <denis.benato@linux.dev> wrote:
>> asus_kbd_register_leds(), I noticed it registers a listener to a global list
>> and uses devm_kzalloc(). If a subsequent initialization step in asus_probe()
>> fails the driver returns without unregistering the listener, and the devres
>> subsystem will automatically free the memory, leaving a dangling pointer
>> in the global list.
>>
>> Fixes: b34b5945a769 ("HID: asus: listen to the asus-wmi brightness device instead of creating one")
>> Reported-by: sahiko-bot@kernel.org
>> Signed-off-by: Denis Benato <denis.benato@linux.dev>
>> ---
>> drivers/hid/hid-asus.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>> index 0a6c97155549..f38b18ad65c6 100644
>> --- a/drivers/hid/hid-asus.c
>> +++ b/drivers/hid/hid-asus.c
>> @@ -1426,11 +1426,17 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> if (drvdata->tp) {
>> ret = asus_start_multitouch(hdev);
>> if (ret)
>> - goto err_stop_hw;
>> + goto err_unregister_backlight;
>> }
> This block currently does two things, rename and init the touchpad.
> Instead, you may pull the touchpad init to be below the hid_hw_start
> block and keep the same bail.
I'm not particularly keen on changing the sequence of packets in these
kinds of patches: I was trying to solve the reported issue in a way that
only solves the issue: nothing less and nothing more.
>> }
>>
>> return 0;
>> +err_unregister_backlight:
>> + if (drvdata->kbd_backlight) {
>> + asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
>> + devm_kfree(&hdev->dev, drvdata->kbd_backlight);
>> + drvdata->kbd_backlight = NULL;
>> + }
>> err_stop_hw:
> OR add asus_hid_unregister_listener(&drvdata->kbd_backlight->listener) here.
>
> Here, you'd also drop err_unregister_backlight, devm_kfree, and
> nulling. The driver is closing. It is not necessary.
>
> err_stop_hw is only used once, you do not need a secondary tag.
I will do this instead, thanks.
>> hid_hw_stop(hdev);
>> return ret;
>> --
>> 2.47.3
>>
>>
^ permalink raw reply
* Re: [PATCH] Input: goodix - clamp the device-reported contact count
From: Hans de Goede @ 2026-06-14 11:44 UTC (permalink / raw)
To: hexlabsecurity, Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <20260612-b4-disp-6844625d-v1-1-df0aed080c9d@proton.me>
Hi,
On 13-Jun-26 04:10, Bryam Vargas via B4 Relay wrote:
> From: Bryam Vargas <hexlabsecurity@proton.me>
>
> goodix_ts_read_input_report() copies the number of touch points reported
> by the device into an on-stack buffer
>
> u8 point_data[2 + GOODIX_MAX_CONTACT_SIZE * GOODIX_MAX_CONTACTS];
>
> which is sized for at most GOODIX_MAX_CONTACTS (10) contacts. The only
> runtime check bounds the per-interrupt count against ts->max_touch_num,
> but that value is taken verbatim from a 4-bit field of the device
> configuration block and is never clamped:
>
> ts->max_touch_num = ts->config[MAX_CONTACTS_LOC] & 0x0f;
>
> The nibble can be 0..15, so a malfunctioning, malicious or counterfeit
> controller (or an attacker tampering with the I2C bus) can advertise up
> to 15 contacts. goodix_ts_read_input_report() then accepts a touch_num
> of up to 15 and the second goodix_i2c_read() writes
> ts->contact_size * (touch_num - 1) bytes past the one-contact header into
> point_data - up to 30 bytes (45 with the 9-byte report format) beyond the
> 92-byte buffer: a stack out-of-bounds write.
>
> Clamp max_touch_num to GOODIX_MAX_CONTACTS, the number of contacts
> point_data[] is sized for, when reading it from the configuration.
>
> Fixes: a7ac7c95d468 ("Input: goodix - use max touch number from device config")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Regards,
Hans
> ---
> drivers/input/touchscreen/goodix.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index f8798d11ec03..17fcfe45988c 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -1057,7 +1057,8 @@ static void goodix_read_config(struct goodix_ts_data *ts)
> }
>
> ts->int_trigger_type = ts->config[TRIGGER_LOC] & 0x03;
> - ts->max_touch_num = ts->config[MAX_CONTACTS_LOC] & 0x0f;
> + ts->max_touch_num = min(ts->config[MAX_CONTACTS_LOC] & 0x0f,
> + GOODIX_MAX_CONTACTS);
>
> x_max = get_unaligned_le16(&ts->config[RESOLUTION_LOC]);
> y_max = get_unaligned_le16(&ts->config[RESOLUTION_LOC + 2]);
>
> ---
> base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
> change-id: 20260612-b4-disp-6844625d-463f81173dc6
>
> Best regards,
^ permalink raw reply
* Re: [PATCH] HID: ft260: fix stack-use-after-return write in I2C read race
From: Michael Zaidman @ 2026-06-14 9:36 UTC (permalink / raw)
To: Raman Varabets
Cc: jikos, bentiss, linux-i2c, linux-input, linux-kernel, stable
In-Reply-To: <20260610142952.3335586-1-kernel-linux-20260610-80b7ab08@raman.v1.sg>
Hi Raman,
Thanks for this patch. The race is real and separate from the input
report length checks already in for-7.1: those bound the HID report
source, but do not protect read_buf lifetime after a timed-out read
unwinds. A spinlock (not dev->lock) is the right fix for the IRQ/input
path.
For stable backport notes: did you reproduce the use-after-return, or
was this found by inspection?
Optional hardening for v2: initialize read_lock before hid_hw_open(), or
immediately after allocating dev, so a malicious USB device cannot hit
spin_lock on an uninitialized lock during the brief probe window after
the input path is live.
I'm validating on FT260 hardware and will follow up on this thread with
Tested-by when that is complete.
Reviewed-by: Michael Zaidman <michael.zaidman@gmail.com>
On Wed, Jun 10, 2026 at 5:30 PM Raman Varabets
<kernel-linux-20260610-80b7ab08@raman.v1.sg> wrote:
>
> ft260_i2c_read() points dev->read_buf at a caller-supplied buffer
> (often an on-stack variable), arms a completion and waits up to five
> seconds for the device to return the data. The HID input callback
> ft260_raw_event() runs in the input/IRQ path, independent of the
> dev->lock mutex held by the read path, and copies the device-supplied
> payload into dev->read_buf after a plain NULL check.
>
> These two paths share read_buf, read_idx and read_len with no
> serialization. If the device delays its response until the read
> times out, ft260_i2c_read() resets the controller, clears read_buf
> and returns, unwinding the stack frame the buffer lived in. A
> response that arrives at that moment lets ft260_raw_event() pass the
> NULL check and then memcpy() the device-controlled payload into the
> now-freed stack location, a bounded but attacker-influenced
> stack-use-after-return write triggerable by malicious or
> malfunctioning hardware.
>
> Add a dedicated spinlock that serializes every access to read_buf,
> read_idx and read_len. ft260_raw_event() now holds it across the
> NULL check, the memcpy and the index update, while the read path
> takes it when arming and when clearing the buffer, so the teardown
> can no longer slip between the check and the copy.
>
> Fixes: 6a82582d9fa4 ("HID: ft260: add usb hid to i2c host bridge driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Raman Varabets <kernel-linux-20260610-80b7ab08@raman.v1.sg>
> ---
> drivers/hid/hid-ft260.c | 25 ++++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 70e2eedb4..f47945954 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
> @@ -240,6 +240,8 @@ struct ft260_device {
> struct mutex lock;
> u8 write_buf[FT260_REPORT_MAX_LENGTH];
> unsigned long need_wakeup_at;
> + /* Protects read_buf, read_idx and read_len against ft260_raw_event() */
> + spinlock_t read_lock;
> u8 *read_buf;
> u16 read_idx;
> u16 read_len;
> @@ -501,6 +503,7 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
> int timeout, ret = 0;
> struct ft260_i2c_read_request_report rep;
> struct hid_device *hdev = dev->hdev;
> + unsigned long irqflags;
> u8 bus_busy = 0;
>
> if ((flag & FT260_FLAG_START_REPEATED) == FT260_FLAG_START_REPEATED)
> @@ -526,9 +529,11 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
>
> reinit_completion(&dev->wait);
>
> + spin_lock_irqsave(&dev->read_lock, irqflags);
> dev->read_idx = 0;
> dev->read_buf = data;
> dev->read_len = rd_len;
> + spin_unlock_irqrestore(&dev->read_lock, irqflags);
>
> ret = ft260_hid_output_report(hdev, (u8 *)&rep, sizeof(rep));
> if (ret < 0) {
> @@ -543,7 +548,9 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
> goto ft260_i2c_read_exit;
> }
>
> + spin_lock_irqsave(&dev->read_lock, irqflags);
> dev->read_buf = NULL;
> + spin_unlock_irqrestore(&dev->read_lock, irqflags);
>
> if (flag & FT260_FLAG_STOP)
> bus_busy = FT260_I2C_STATUS_BUS_BUSY;
> @@ -562,7 +569,9 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
> } while (len > 0);
>
> ft260_i2c_read_exit:
> + spin_lock_irqsave(&dev->read_lock, irqflags);
> dev->read_buf = NULL;
> + spin_unlock_irqrestore(&dev->read_lock, irqflags);
> return ret;
> }
>
> @@ -1018,6 +1027,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
> "FT260 usb-i2c bridge");
>
> mutex_init(&dev->lock);
> + spin_lock_init(&dev->read_lock);
> init_completion(&dev->wait);
>
> ret = ft260_xfer_status(dev, FT260_I2C_STATUS_BUS_BUSY);
> @@ -1067,6 +1077,7 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
> {
> struct ft260_device *dev = hid_get_drvdata(hdev);
> struct ft260_i2c_input_report *xfer = (void *)data;
> + unsigned long irqflags;
>
> if (size < offsetof(struct ft260_i2c_input_report, data)) {
> hid_err(hdev, "short report %d\n", size);
> @@ -1075,6 +1086,8 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
>
> if (xfer->report >= FT260_I2C_REPORT_MIN &&
> xfer->report <= FT260_I2C_REPORT_MAX) {
> + bool complete_read;
> +
> ft260_dbg("i2c resp: rep %#02x len %d size %d\n",
> xfer->report, xfer->length, size);
>
> @@ -1085,8 +1098,15 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
> return -1;
> }
>
> + /*
> + * Hold read_lock so a timed-out ft260_i2c_read() cannot
> + * clear read_buf between the NULL check and the memcpy.
> + */
> + spin_lock_irqsave(&dev->read_lock, irqflags);
> +
> if ((dev->read_buf == NULL) ||
> (xfer->length > dev->read_len - dev->read_idx)) {
> + spin_unlock_irqrestore(&dev->read_lock, irqflags);
> hid_err(hdev, "unexpected report %#02x, length %d\n",
> xfer->report, xfer->length);
> return -1;
> @@ -1095,8 +1115,11 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
> memcpy(&dev->read_buf[dev->read_idx], &xfer->data,
> xfer->length);
> dev->read_idx += xfer->length;
> + complete_read = dev->read_idx == dev->read_len;
> +
> + spin_unlock_irqrestore(&dev->read_lock, irqflags);
>
> - if (dev->read_idx == dev->read_len)
> + if (complete_read)
> complete(&dev->wait);
>
> } else {
> --
> 2.54.0
>
^ permalink raw reply
* Re: [PATCH] HID: ft260: fix SMBus block read protocol handling
From: Michael Zaidman @ 2026-06-14 9:31 UTC (permalink / raw)
To: kernel-linux-20260610-80b7ab08
Cc: jikos, bentiss, linux-i2c, linux-input, linux-kernel
In-Reply-To: <CAPnwWgNLd2RZVx__T1Z9siKwN_5HrA2hcKwrZ2C6YivQuF6Ubw@mail.gmail.com>
Apologies, wrong address in my previous Reviewed-by.
Reviewed-by: Michael Zaidman <michael.zaidman@gmail.com>
On Sun, Jun 14, 2026 at 12:27 PM Michael Zaidman
<michael.zaidman@gmail.com> wrote:
>
> Hi Raman,
>
> Thanks for the patch. Your analysis matches what I see in the driver:
> on a block read the count comes from the slave, data->block[0] is not
> initialized by the caller, so sizing the read from data->block[0] + 1
> is wrong. Reading and validating the count first is the correct fix
> for the FT260.
>
> I'm validating on FT260 hardware and will follow up on this thread with
> Tested-by/Acked-by when that is complete.
>
> Reviewed-by: Michael Zaidman <michaelz@xsightlabs.com>
>
> On Wed, Jun 10, 2026 at 4:42 PM Raman Varabets
> <kernel-linux-20260610-80b7ab08@raman.v1.sg> wrote:
> >
> > For I2C_SMBUS_BLOCK_DATA reads, ft260_smbus_xfer() passed
> > data->block[0] + 1 as the read length. But on a block read the byte
> > count is supplied by the slave as the first byte of the response;
> > data->block[0] is not initialized by the caller, so the transfer
> > length was taken from stale buffer contents, and the count byte the
> > slave did return was stored without any validation.
> >
> > Implement the SMBus 2.0 block read protocol properly: read the count
> > byte first with a repeated START and no STOP, validate it against
> > I2C_SMBUS_BLOCK_MAX (resetting the bus and returning -EPROTO on a
> > bogus count), then read exactly that many data bytes and finish the
> > transaction with STOP. This keeps the whole sequence within a single
> > I2C transaction:
> >
> > S Addr+Wr A Reg A Sr Addr+Rd A Count A Data... P
> >
> > To support issuing the two reads as one transaction, teach
> > ft260_i2c_read() to honor the caller's flags instead of always
> > forcing a START and unconditionally appending STOP to the last
> > chunk: START is only emitted if requested, and STOP is appended to
> > the final chunk only when the caller asked for it.
> >
> > Signed-off-by: Raman Varabets <kernel-linux-20260610-80b7ab08@raman.v1.sg>
> > ---
> > drivers/hid/hid-ft260.c | 45 ++++++++++++++++++++++++++++++++++++-----
> > 1 file changed, 40 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> > index 70e2eedb4..946ed0c6f 100644
> > --- a/drivers/hid/hid-ft260.c
> > +++ b/drivers/hid/hid-ft260.c
> > @@ -502,15 +502,23 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
> > struct ft260_i2c_read_request_report rep;
> > struct hid_device *hdev = dev->hdev;
> > u8 bus_busy = 0;
> > + /*
> > + * STOP terminates the last chunk; clear means hold the bus so a
> > + * follow-up call continues the same I2C transaction.
> > + */
> > + bool want_stop = !!(flag & FT260_FLAG_STOP);
> >
> > if ((flag & FT260_FLAG_START_REPEATED) == FT260_FLAG_START_REPEATED)
> > flag = FT260_FLAG_START_REPEATED;
> > - else
> > + else if (flag & FT260_FLAG_START)
> > flag = FT260_FLAG_START;
> > + else
> > + flag = 0; /* no fresh START - continue current transaction */
> > do {
> > if (len <= rd_data_max) {
> > rd_len = len;
> > - flag |= FT260_FLAG_STOP;
> > + if (want_stop)
> > + flag |= FT260_FLAG_STOP;
> > } else {
> > rd_len = rd_data_max;
> > }
> > @@ -708,14 +716,41 @@ static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags,
> > break;
> > case I2C_SMBUS_BLOCK_DATA:
> > if (read_write == I2C_SMBUS_READ) {
> > + u8 count = 0;
> > +
> > + /*
> > + * SMBus 2.0 section 6.5.7 block read in one I2C
> > + * transaction:
> > + *
> > + * S Addr+Wr A Reg A Sr Addr+Rd A Count A Data... P
> > + *
> > + * The count is read separately and validated
> > + * before sizing the data read so a misbehaving
> > + * slave cannot drive a write past data->block[].
> > + */
> > ret = ft260_smbus_write(dev, addr, cmd, NULL, 0,
> > FT260_FLAG_START);
> > if (ret)
> > goto smbus_exit;
> >
> > - ret = ft260_i2c_read(dev, addr, data->block,
> > - data->block[0] + 1,
> > - FT260_FLAG_START_STOP_REPEATED);
> > + ret = ft260_i2c_read(dev, addr, &count, 1,
> > + FT260_FLAG_START_REPEATED);
> > + if (ret)
> > + goto smbus_exit;
> > +
> > + if (count == 0 || count > I2C_SMBUS_BLOCK_MAX) {
> > + hid_warn(hdev,
> > + "smbus block read: invalid count %u from slave 0x%02x\n",
> > + count, addr);
> > + ft260_i2c_reset(hdev);
> > + ret = -EPROTO;
> > + goto smbus_exit;
> > + }
> > +
> > + data->block[0] = count;
> > +
> > + ret = ft260_i2c_read(dev, addr, data->block + 1,
> > + count, FT260_FLAG_STOP);
> > } else {
> > ret = ft260_smbus_write(dev, addr, cmd, data->block,
> > data->block[0] + 1,
> >
> > base-commit: ab5fce87a778cb780a05984a2ca448f2b41aafbf
> > --
> > 2.54.0
> >
^ permalink raw reply
* Re: [PATCH] HID: ft260: fix SMBus block read protocol handling
From: Michael Zaidman @ 2026-06-14 9:27 UTC (permalink / raw)
To: kernel-linux-20260610-80b7ab08
Cc: jikos, bentiss, linux-i2c, linux-input, linux-kernel
In-Reply-To: <20260610134119.3313484-1-kernel-linux-20260610-80b7ab08@raman.v1.sg>
Hi Raman,
Thanks for the patch. Your analysis matches what I see in the driver:
on a block read the count comes from the slave, data->block[0] is not
initialized by the caller, so sizing the read from data->block[0] + 1
is wrong. Reading and validating the count first is the correct fix
for the FT260.
I'm validating on FT260 hardware and will follow up on this thread with
Tested-by/Acked-by when that is complete.
Reviewed-by: Michael Zaidman <michaelz@xsightlabs.com>
On Wed, Jun 10, 2026 at 4:42 PM Raman Varabets
<kernel-linux-20260610-80b7ab08@raman.v1.sg> wrote:
>
> For I2C_SMBUS_BLOCK_DATA reads, ft260_smbus_xfer() passed
> data->block[0] + 1 as the read length. But on a block read the byte
> count is supplied by the slave as the first byte of the response;
> data->block[0] is not initialized by the caller, so the transfer
> length was taken from stale buffer contents, and the count byte the
> slave did return was stored without any validation.
>
> Implement the SMBus 2.0 block read protocol properly: read the count
> byte first with a repeated START and no STOP, validate it against
> I2C_SMBUS_BLOCK_MAX (resetting the bus and returning -EPROTO on a
> bogus count), then read exactly that many data bytes and finish the
> transaction with STOP. This keeps the whole sequence within a single
> I2C transaction:
>
> S Addr+Wr A Reg A Sr Addr+Rd A Count A Data... P
>
> To support issuing the two reads as one transaction, teach
> ft260_i2c_read() to honor the caller's flags instead of always
> forcing a START and unconditionally appending STOP to the last
> chunk: START is only emitted if requested, and STOP is appended to
> the final chunk only when the caller asked for it.
>
> Signed-off-by: Raman Varabets <kernel-linux-20260610-80b7ab08@raman.v1.sg>
> ---
> drivers/hid/hid-ft260.c | 45 ++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 70e2eedb4..946ed0c6f 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
> @@ -502,15 +502,23 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
> struct ft260_i2c_read_request_report rep;
> struct hid_device *hdev = dev->hdev;
> u8 bus_busy = 0;
> + /*
> + * STOP terminates the last chunk; clear means hold the bus so a
> + * follow-up call continues the same I2C transaction.
> + */
> + bool want_stop = !!(flag & FT260_FLAG_STOP);
>
> if ((flag & FT260_FLAG_START_REPEATED) == FT260_FLAG_START_REPEATED)
> flag = FT260_FLAG_START_REPEATED;
> - else
> + else if (flag & FT260_FLAG_START)
> flag = FT260_FLAG_START;
> + else
> + flag = 0; /* no fresh START - continue current transaction */
> do {
> if (len <= rd_data_max) {
> rd_len = len;
> - flag |= FT260_FLAG_STOP;
> + if (want_stop)
> + flag |= FT260_FLAG_STOP;
> } else {
> rd_len = rd_data_max;
> }
> @@ -708,14 +716,41 @@ static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags,
> break;
> case I2C_SMBUS_BLOCK_DATA:
> if (read_write == I2C_SMBUS_READ) {
> + u8 count = 0;
> +
> + /*
> + * SMBus 2.0 section 6.5.7 block read in one I2C
> + * transaction:
> + *
> + * S Addr+Wr A Reg A Sr Addr+Rd A Count A Data... P
> + *
> + * The count is read separately and validated
> + * before sizing the data read so a misbehaving
> + * slave cannot drive a write past data->block[].
> + */
> ret = ft260_smbus_write(dev, addr, cmd, NULL, 0,
> FT260_FLAG_START);
> if (ret)
> goto smbus_exit;
>
> - ret = ft260_i2c_read(dev, addr, data->block,
> - data->block[0] + 1,
> - FT260_FLAG_START_STOP_REPEATED);
> + ret = ft260_i2c_read(dev, addr, &count, 1,
> + FT260_FLAG_START_REPEATED);
> + if (ret)
> + goto smbus_exit;
> +
> + if (count == 0 || count > I2C_SMBUS_BLOCK_MAX) {
> + hid_warn(hdev,
> + "smbus block read: invalid count %u from slave 0x%02x\n",
> + count, addr);
> + ft260_i2c_reset(hdev);
> + ret = -EPROTO;
> + goto smbus_exit;
> + }
> +
> + data->block[0] = count;
> +
> + ret = ft260_i2c_read(dev, addr, data->block + 1,
> + count, FT260_FLAG_STOP);
> } else {
> ret = ft260_smbus_write(dev, addr, cmd, data->block,
> data->block[0] + 1,
>
> base-commit: ab5fce87a778cb780a05984a2ca448f2b41aafbf
> --
> 2.54.0
>
^ permalink raw reply
* Re: [PATCH 1/2] Input: synaptics-rmi4 - bound the F3A keymap to the GPIO count
From: sashiko-bot @ 2026-06-14 5:51 UTC (permalink / raw)
To: Bryam Vargas via B4 Relay; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260614-b4-disp-818d6bda-v1-1-cf39a3615085@proton.me>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] Out-of-bounds shift and logic error in `rmi_f3a_report_button` when `button >= 8`.
- [High] Unbinding the `rmi4_f3a` driver leaves a dangling `input->keycode` pointer, leading to a Use-After-Free.
- [High] Failing to insert into the attention FIFO leaks the dynamically allocated duplicate buffer.
- [Medium] Multiple RMI4 functions concurrently overwrite the shared `input->keycode` map, making all but the last probed map inaccessible to userspace.
--
commit 1556a27ea7ad4e1c54736060ac2c334003a11681
Author: Bryam Vargas <hexlabsecurity@proton.me>
Input: synaptics-rmi4 - bound the F3A keymap to the GPIO count
This commit addresses an out-of-bounds access in the F3A keymap. It sizes the
gpio_key_map buffer to the full gpio_count rather than capping it at 6,
preventing reads or writes past the allocated array during attention
interrupts or keymap ioctls.
> diff --git a/drivers/input/rmi4/rmi_f3a.c b/drivers/input/rmi4/rmi_f3a.c
> index 0e8baed84dbb5..a0777644eef07 100644
> --- a/drivers/input/rmi4/rmi_f3a.c
> +++ b/drivers/input/rmi4/rmi_f3a.c
> @@ -132,7 +132,7 @@ static int rmi_f3a_map_gpios(struct rmi_function *fn, struct f3a_data *f3a,
> int button_count = min_t(u8, f3a->gpio_count, TRACKSTICK_RANGE_END);
>
> f3a->gpio_key_map = devm_kcalloc(&fn->dev,
> - button_count,
> + f3a->gpio_count,
> sizeof(f3a->gpio_key_map[0]),
> GFP_KERNEL);
[Severity: High]
This isn't a bug introduced by this patch, but I have a question about the
lifecycle of the keymap allocated here.
Since input->keycode is set to f3a->gpio_key_map later in this function, if
the rmi4_f3a function driver is unbound via sysfs, devres will free this
memory. Because rmi_f3a_handler lacks a .remove callback and does not set
.suppress_bind_attrs = true, does this leave input->keycode pointing to freed
memory? Could subsequent EVIOCGKEYCODE or EVIOCSKEYCODE ioctls on the shared
input_dev dereference this freed pointer?
[Severity: Medium]
This is a pre-existing issue, but I also noticed how this keymap interacts
with the shared input device.
Both F3A and F30 assign their own private arrays to the shared input->keycode:
drivers/input/rmi4/rmi_f3a.c:rmi_f3a_map_gpios() {
...
input->keycode = f3a->gpio_key_map;
...
}
drivers/input/rmi4/rmi_f30.c:rmi_f30_map_gpios() {
...
input->keycode = f30->gpioled_key_map;
...
}
Since multiple RMI4 functions concurrently overwrite the shared
input->keycode, does this make all but the last probed map inaccessible to
userspace? Would earlier functions then report events using their
unmodifiable, private arrays?
> if (!f3a->gpio_key_map) {
[ ... ]
[Severity: High]
This is a pre-existing issue, but looking at rmi_f3a_report_button() in
drivers/input/rmi4/rmi_f3a.c:
drivers/input/rmi4/rmi_f3a.c:rmi_f3a_report_button() {
...
bool key_down = !(f3a->data_regs[0] & BIT(button));
...
}
Since f3a->data_regs is a u8 array, could this incorrectly report the key as
constantly pressed if button >= 8? If button >= 32, could this result in an
out-of-bounds shift? Since userspace can map a high button index via
EVIOCSKEYCODE and trigger an interrupt, could this be triggered intentionally?
[Severity: High]
This is a pre-existing issue, but looking at rmi_set_attn_data() in
drivers/input/rmi4/rmi_driver.c:
drivers/input/rmi4/rmi_driver.c:rmi_set_attn_data() {
...
fifo_data = kmemdup(data, size, GFP_ATOMIC);
...
attn_data.data = fifo_data;
...
kfifo_put(&drvdata->attn_fifo, attn_data);
...
}
If kfifo_put() fails because the FIFO is full, does this silently leak the
dynamically allocated fifo_data buffer? Could a flood of interrupts lead to
memory exhaustion?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260614-b4-disp-818d6bda-v1-0-cf39a3615085@proton.me?part=1
^ permalink raw reply
* Re: [PATCH 2/2] Input: synaptics-rmi4 - bound the F30 keymap to the GPIO/LED count
From: sashiko-bot @ 2026-06-14 5:50 UTC (permalink / raw)
To: Bryam Vargas via B4 Relay; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260614-b4-disp-818d6bda-v1-2-cf39a3615085@proton.me>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Unbinding the `rmi4_f30` driver leaves a dangling pointer in the shared input device's `keycode` field, leading to a Use-After-Free.
--
commit 251790cc33b0da19e8d0320efecb822396ad7b5f
Author: Bryam Vargas <hexlabsecurity@proton.me>
Input: synaptics-rmi4 - bound the F30 keymap to the GPIO/LED count
This commit fixes an out-of-bounds read and write issue in the F30
keymap handler. It allocates the keymap based on the full reported
GPIO/LED count rather than capping the allocation at 6 entries.
> diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
> --- a/drivers/input/rmi4/rmi_f30.c
> +++ b/drivers/input/rmi4/rmi_f30.c
> @@ -233,7 +233,7 @@ static int rmi_f30_map_gpios(struct rmi_function *fn,
> int button_count = min_t(u8, f30->gpioled_count, TRACKSTICK_RANGE_END);
>
> f30->gpioled_key_map = devm_kcalloc(&fn->dev,
> - button_count,
> + f30->gpioled_count,
> sizeof(f30->gpioled_key_map[0]),
> GFP_KERNEL);
[Severity: High]
This is a pre-existing issue, but since we are modifying this allocation,
can this lead to a use-after-free when the driver is unbound?
The memory for f30->gpioled_key_map is managed by the function subdevice
using fn->dev. Later in rmi_f30_map_gpios(), this pointer is assigned to the
shared input device's keycode field:
input->keycode = f30->gpioled_key_map;
Since the shared input device is attached to the parent RMI device, it can
outlive the function subdevice. If userspace unbinds the rmi4_f30 driver
via sysfs (because the driver lacks a remove callback and does not set
suppress_bind_attrs), devres will free f30->gpioled_key_map while
the input device remains registered.
Would a subsequent ioctl like EVIOCGKEYCODE or EVIOCSKEYCODE access this
freed input->keycode memory?
Does this also cause a race condition during device unplug, since the
function devices are unbound before the input device is unregistered?
> if (!f30->gpioled_key_map) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260614-b4-disp-818d6bda-v1-0-cf39a3615085@proton.me?part=2
^ permalink raw reply
* [PATCH 1/2] Input: synaptics-rmi4 - bound the F3A keymap to the GPIO count
From: Bryam Vargas via B4 Relay @ 2026-06-14 5:36 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-kernel, Hans de Goede, Vincent Huang, linux-input,
Andrew Duggan
In-Reply-To: <20260614-b4-disp-818d6bda-v1-0-cf39a3615085@proton.me>
From: Bryam Vargas <hexlabsecurity@proton.me>
rmi_f3a_initialize() takes the GPIO count from the device query register
(f3a->gpio_count = buf & RMI_F3A_GPIO_COUNT, range 0..127).
rmi_f3a_map_gpios() then allocates gpio_key_map with
min(gpio_count, TRACKSTICK_RANGE_END) == at most 6 entries, but
rmi_f3a_attention() iterates the full gpio_count and dereferences
gpio_key_map[i], and input->keycodemax is set to the full gpio_count
while input->keycode points at the 6-entry allocation.
A device that reports gpio_count > 6 therefore causes an out-of-bounds
read of gpio_key_map[] on every attention interrupt, and out-of-bounds
accesses through the input core's default keymap ioctls: EVIOCGKEYCODE
reads past the buffer (leaking adjacent slab memory to user space) and
EVIOCSKEYCODE writes a caller-controlled value past it, for any process
able to open the evdev node, since input_default_getkeycode() and
input_default_setkeycode() only bound the index against keycodemax.
Size the keymap for the full gpio_count. The mapping loop is unchanged:
it still assigns only the first min(gpio_count, TRACKSTICK_RANGE_END)
entries; the remaining slots stay KEY_RESERVED (devm_kcalloc zero-fills)
and are skipped when reporting.
Fixes: 9e4c596bfd00 ("Input: synaptics-rmi4 - add support for F3A")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
drivers/input/rmi4/rmi_f3a.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/rmi4/rmi_f3a.c b/drivers/input/rmi4/rmi_f3a.c
index 0e8baed84dbb..a0777644eef0 100644
--- a/drivers/input/rmi4/rmi_f3a.c
+++ b/drivers/input/rmi4/rmi_f3a.c
@@ -132,7 +132,7 @@ static int rmi_f3a_map_gpios(struct rmi_function *fn, struct f3a_data *f3a,
int button_count = min_t(u8, f3a->gpio_count, TRACKSTICK_RANGE_END);
f3a->gpio_key_map = devm_kcalloc(&fn->dev,
- button_count,
+ f3a->gpio_count,
sizeof(f3a->gpio_key_map[0]),
GFP_KERNEL);
if (!f3a->gpio_key_map) {
--
2.43.0
^ permalink raw reply related
* [PATCH 2/2] Input: synaptics-rmi4 - bound the F30 keymap to the GPIO/LED count
From: Bryam Vargas via B4 Relay @ 2026-06-14 5:36 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-kernel, Hans de Goede, Vincent Huang, linux-input,
Andrew Duggan
In-Reply-To: <20260614-b4-disp-818d6bda-v1-0-cf39a3615085@proton.me>
From: Bryam Vargas <hexlabsecurity@proton.me>
rmi_f30_map_gpios() allocates gpioled_key_map with
min(gpioled_count, TRACKSTICK_RANGE_END) == at most 6 entries, but
rmi_f30_attention() iterates the full f30->gpioled_count (device query
register, range 0..31) and dereferences gpioled_key_map[i], and
input->keycodemax is set to the full gpioled_count while input->keycode
points at the 6-entry allocation.
A device that reports gpioled_count > 6 with GPIO support enabled
therefore causes an out-of-bounds read on the attention interrupt and
out-of-bounds read/write through the EVIOCGKEYCODE/EVIOCSKEYCODE ioctls,
which bound the index only against keycodemax. This is the same defect
as the F3A handler, which was copied from F30.
Size the keymap for the full gpioled_count; the mapping loop still
assigns only the first min(gpioled_count, TRACKSTICK_RANGE_END) entries.
Fixes: 3e64fcbdbd10 ("Input: synaptics-rmi4 - limit the range of what GPIOs are buttons")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
drivers/input/rmi4/rmi_f30.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
index 35045f161dc2..b2155c8e20e7 100644
--- a/drivers/input/rmi4/rmi_f30.c
+++ b/drivers/input/rmi4/rmi_f30.c
@@ -233,7 +233,7 @@ static int rmi_f30_map_gpios(struct rmi_function *fn,
int button_count = min_t(u8, f30->gpioled_count, TRACKSTICK_RANGE_END);
f30->gpioled_key_map = devm_kcalloc(&fn->dev,
- button_count,
+ f30->gpioled_count,
sizeof(f30->gpioled_key_map[0]),
GFP_KERNEL);
if (!f30->gpioled_key_map) {
--
2.43.0
^ permalink raw reply related
* [PATCH 0/2] (no cover subject)
From: Bryam Vargas via B4 Relay @ 2026-06-14 5:36 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-kernel, Hans de Goede, Vincent Huang, linux-input,
Andrew Duggan
From be2c4843f6145374f28edd25cef43c9373542874 Mon Sep 17 00:00:00 2001
Message-ID: <cover.1781415009.git.hexlabsecurity@proton.me>
From: Bryam Vargas <hexlabsecurity@proton.me>
Date: Sun, 14 Jun 2026 00:30:09 -0500
Subject: [PATCH 0/2] Input: synaptics-rmi4 - fix out-of-bounds keymap access on large GPIO counts
The RMI4 F3A and F30 function handlers size their GPIO/LED key map to
min(device_gpio_count, TRACKSTICK_RANGE_END) == at most 6 entries, but
then consume that map with the full, unclamped device-reported count in
two places: the attention-interrupt report loop, and input->keycodemax
(while input->keycode points at the small allocation).
A device that reports a GPIO/LED count greater than 6 therefore yields a
key map of at most 12 bytes that is indexed up to count-1:
- rmi_f3a_attention() / rmi_f30_attention() read gpio[led]_key_map[i]
out of bounds on every attention interrupt (device-triggered), and
- the input core's default keymap ioctls bound the index only against
keycodemax, so EVIOCGKEYCODE reads adjacent slab memory back to user
space and EVIOCSKEYCODE writes a caller-controlled u16 past the
allocation -- a controlled out-of-bounds heap write reachable by any
process that can open the evdev node.
Both handlers have the same shape; F3A was copied from F30. Patch 1 fixes
F3A, patch 2 fixes F30. The one-line fix in each sizes the key map to the
full device count; the mapping loop is unchanged (it still populates only
the first min(count, 6) entries, the rest stay KEY_RESERVED and are
skipped on report), and keycodemax now matches the allocation, closing
the interrupt and both ioctl paths.
Reproduced with KASAN on a faithful in-kernel model of each handler
(kmalloc-16 bucket, 12-byte request, reported count = 127):
# without the patch -- attention read path
BUG: KASAN: slab-out-of-bounds in rmi_f3a_attention
Read of size 2 ... 0 bytes to the right of allocated 12-byte region
... cache kmalloc-16
# without the patch -- EVIOCSKEYCODE write path
BUG: KASAN: slab-out-of-bounds ...
Write of size 2 ... 0 bytes to the right of allocated 12-byte region
# with the patch (key map sized to the full count): clean
# benign device (reported count <= 6): clean
The full read and write splats and a 32/64-bit ASan model of the same
geometry are available on request.
Bryam Vargas (2):
Input: synaptics-rmi4 - bound the F3A keymap to the GPIO count
Input: synaptics-rmi4 - bound the F30 keymap to the GPIO/LED count
drivers/input/rmi4/rmi_f30.c | 2 +-
drivers/input/rmi4/rmi_f3a.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
---
Bryam Vargas (2):
Input: synaptics-rmi4 - bound the F3A keymap to the GPIO count
Input: synaptics-rmi4 - bound the F30 keymap to the GPIO/LED count
drivers/input/rmi4/rmi_f30.c | 2 +-
drivers/input/rmi4/rmi_f3a.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
---
base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
change-id: 20260614-b4-disp-818d6bda-8df90972ace6
Best regards,
--
Bryam Vargas <hexlabsecurity@proton.me>
^ 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