* 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
* Re: [PATCH] Input: synaptics-rmi4 - bound the F54 report size to the allocated buffer
From: sashiko-bot @ 2026-06-14 4:14 UTC (permalink / raw)
To: Bryam Vargas via B4 Relay; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260613-b4-disp-6721686c-v1-1-4d5bb84ee520@proton.me>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Stale f54->report_size causes V4L2 buffer heap overflow.
- [High] Missing cancel_delayed_work_sync in rmi_f54_remove causes Use-After-Free.
- [High] Preexisting copy-paste typo in F55 properties parsing triggers the new oversize check, permanently breaking conforming devices.
- [High] Use-After-Free of devm-managed video_device on V4L2 file descriptor close.
- [Medium] V4L2 buffer queue silently returns uninitialized memory on hardware errors.
--
commit 08992eb2bae1821355b21e17559ef51ec82606cd
Author: Bryam Vargas <hexlabsecurity@proton.me>
Input: synaptics-rmi4 - bound the F54 report size to the allocated buffer
This commit adds a bounds check to ensure the F54 diagnostic report size
does not exceed the allocated buffer. It rejects reports that are larger
than the allocation to prevent writing past the end of the report data
buffer.
> diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
> index 61909e1a39e24..7a7508a4de85c 100644
> --- a/drivers/input/rmi4/rmi_f54.c
> +++ b/drivers/input/rmi4/rmi_f54.c
[ ... ]
> @@ -548,6 +549,13 @@ static void rmi_f54_work(struct work_struct *work)
> goto error; /* retry won't help */
> }
>
> + if (report_size > f54->report_data_size) {
[Severity: High]
This is a pre-existing issue, but does this new check permanently break
conforming hardware due to a copy-paste typo in the F55 parser?
Looking at rmi_f55_detect() in rmi_f55.c, it incorrectly assigns the
RX count to the TX count:
drv_data->num_rx_electrodes = f55->cfg_num_rx_electrodes;
drv_data->num_tx_electrodes = f55->cfg_num_rx_electrodes;
For hardware lacking F55_CAP_SENSOR_ASSIGN and having more RX electrodes
than TX electrodes, this inflates the perceived TX count. This inflated count
propagates to rmi_f54_get_report_size(), yielding a larger value that will
unconditionally fail this new bounds check.
> + dev_err(&fn->dev, "Report size %d exceeds buffer size %zu\n",
> + report_size, f54->report_data_size);
> + error = -EINVAL;
> + goto error;
[Severity: Critical]
This isn't a bug introduced by this patch, but does taking this goto error
path bypass the zeroing of f54->report_size and cause a V4L2 buffer heap
overflow?
In rmi_f54_work(), error paths jump to the error: label, bypassing the
abort: label where f54->report_size is normally zeroed out. This leaves
f54->report_size containing its previous successful payload size.
If a user then alters the V4L2 format to a smaller size, and an error occurs
(like this bounds check failing), rmi_f54_buffer_queue() will copy the
stale, larger payload size into the shrunken V4L2 buffer:
memcpy(ptr, f54->report_data, f54->report_size);
[Severity: Medium]
This is also a pre-existing issue, but does this error path cause the V4L2
buffer queue to silently return uninitialized memory?
When rmi_f54_work() triggers this bounds check and jumps to error, the
buffer is not updated. However, rmi_f54_buffer_queue() ignores the failure
and marks the buffer as fully populated:
vb2_set_plane_payload(vb, 0, rmi_f54_get_report_size(f54));
state = VB2_BUF_STATE_DONE;
This completes the buffer without indicating an error to userspace.
> + }
> +
> /*
> * Need to check if command has completed.
[ ... ]
> @@ -678,8 +686,8 @@ static int rmi_f54_probe(struct rmi_function *fn)
>
> rx = f54->num_rx_electrodes;
> tx = f54->num_tx_electrodes;
> - f54->report_data = devm_kzalloc(&fn->dev,
> - array3_size(tx, rx, sizeof(u16)),
> + f54->report_data_size = array3_size(tx, rx, sizeof(u16));
> + f54->report_data = devm_kzalloc(&fn->dev, f54->report_data_size,
> GFP_KERNEL);
[Severity: High]
This is a pre-existing issue, but does allocating the f54_data struct
(which embeds struct video_device vdev) with devm_kzalloc lead to a
use-after-free when the V4L2 file descriptor is closed?
In rmi_f54_probe(), f54 is allocated using devm_kzalloc(). When the driver
unbinds, devm automatically frees f54_data.
If a userspace application still holds /dev/videoX open, the V4L2 core
retains a reference to the freed vdev. Subsequent ioctl calls or the final
closing of the file descriptor will access the freed vdev structure.
[Severity: High]
This isn't a bug introduced by this patch, but does the lack of a timer
cancellation in rmi_f54_remove() cause a use-after-free?
While rmi_f54_probe() correctly uses cancel_delayed_work_sync(&f54->work)
in its error path, the teardown path in rmi_f54_remove() just destroys
the workqueue:
rmi_f54_remove() {
...
video_unregister_device(&f54->vdev);
v4l2_device_unregister(&f54->v4l2);
destroy_workqueue(f54->workqueue);
...
}
This does not cancel pending delayed_work timers. If the driver is unbound
while a V4L2 stream is active, the timer will fire after devm has freed
the f54 structure and attempt to dereference f54->workqueue.
> if (f54->report_data == NULL)
> return -ENOMEM;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613-b4-disp-6721686c-v1-1-4d5bb84ee520@proton.me?part=1
^ permalink raw reply
* [PATCH] Input: synaptics-rmi4 - bound the F54 report size to the allocated buffer
From: Bryam Vargas via B4 Relay @ 2026-06-14 4:01 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, linux-kernel, Guenter Roeck, Benjamin Tissoires
From: Bryam Vargas <hexlabsecurity@proton.me>
rmi_f54_work() reads a diagnostics report from the device into
f54->report_data, sizing the transfer with rmi_f54_get_report_size():
report_size = rmi_f54_get_report_size(f54);
...
for (i = 0; i < report_size; i += F54_REPORT_DATA_SIZE) {
int size = min(F54_REPORT_DATA_SIZE, report_size - i);
...
rmi_read_block(.., f54->report_data + i, size);
}
report_data is allocated once at probe from F54's own electrode counts
(array3_size(f54->num_tx_electrodes, f54->num_rx_electrodes, sizeof(u16))),
but rmi_f54_get_report_size() computes the size from
drv_data->num_*_electrodes when those are set, i.e. from the F55
function's electrode counts. Both counts come straight from device
queries (F54 and F55 each report up to 255 electrodes) and nothing
constrains the F55 counts to the F54 ones.
A malicious or malfunctioning RMI4 device that reports larger F55
electrode counts than its F54 counts makes report_size exceed the
allocation, so the read loop writes past report_data (and the V4L2
dequeue memcpy() then reads past it). On conforming hardware the F55
configured electrodes are a subset of the F54 physical electrodes, so
report_size never exceeds the buffer and well-behaved devices are
unaffected.
Record the allocation size and reject a report that does not fit,
mirroring the existing zero-size check.
Fixes: c762cc68b6a1 ("Input: synaptics-rmi4 - propagate correct number of rx and tx electrodes to F54")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Reachable from a malicious or counterfeit RMI4 peripheral (HID/I2C/SPI)
that exposes both an F54 diagnostics function and an F55 sensor-tuning
function reporting more electrodes than F54; the report is filled when a
local client drives an F54 capture through the V4L2 touch device node.
Verified with an in-kernel KASAN litmus that allocates report_data from
the F54 dims and fills it from the F55-preferred dims (the verbatim
rmi_f54_get_report_size() preference and the rmi_f54_work() 32-byte read
loop), Linux 7.1.0-rc5, CONFIG_KASAN=y, kasan.fault=panic, x86_64:
Arm A, F54 8x8 (capacity 128) vs F55 8x24 (report_size 384):
BUG: KASAN: slab-out-of-bounds in ... Write of size 32
located 0 bytes to the right of the allocated 128-byte region
... cache kmalloc-128 of size 128
Kernel panic - not syncing: kasan.fault=panic set
Arm B, with this patch (oversize report rejected): clean
Arm C, conforming device (F55 dims == F54 dims): clean
AddressSanitizer (x86_64 and i386): heap-buffer-overflow WRITE of size 32,
0 bytes after the 128-byte region.
The same rmi_f54_get_report_size() value drives the V4L2 dequeue
memcpy(.., report_size) on the read side, so an oversize report also
over-reads report_data; rejecting it in rmi_f54_work() (report_size then
stays 0) closes both. Reproducer and full logs available on request.
This is independent of the in-flight "Input: rmi4 - release F54 queue on
video registration failure" change, which touches only the probe error
path (vb2_queue_release in rmi_f54_probe); it applies cleanly alongside it.
---
drivers/input/rmi4/rmi_f54.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
index 61909e1a39e2..7a7508a4de85 100644
--- a/drivers/input/rmi4/rmi_f54.c
+++ b/drivers/input/rmi4/rmi_f54.c
@@ -104,6 +104,7 @@ struct f54_data {
enum rmi_f54_report_type report_type;
u8 *report_data;
+ size_t report_data_size;
int report_size;
bool is_busy;
@@ -548,6 +549,13 @@ static void rmi_f54_work(struct work_struct *work)
goto error; /* retry won't help */
}
+ if (report_size > f54->report_data_size) {
+ dev_err(&fn->dev, "Report size %d exceeds buffer size %zu\n",
+ report_size, f54->report_data_size);
+ error = -EINVAL;
+ goto error;
+ }
+
/*
* Need to check if command has completed.
* If not try again later.
@@ -678,8 +686,8 @@ static int rmi_f54_probe(struct rmi_function *fn)
rx = f54->num_rx_electrodes;
tx = f54->num_tx_electrodes;
- f54->report_data = devm_kzalloc(&fn->dev,
- array3_size(tx, rx, sizeof(u16)),
+ f54->report_data_size = array3_size(tx, rx, sizeof(u16));
+ f54->report_data = devm_kzalloc(&fn->dev, f54->report_data_size,
GFP_KERNEL);
if (f54->report_data == NULL)
return -ENOMEM;
---
base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
change-id: 20260613-b4-disp-6721686c-33a3bdacdf38
Best regards,
--
Bryam Vargas <hexlabsecurity@proton.me>
^ permalink raw reply related
* Re: [PATCH] Input: iforce - bound the device-reported force-feedback effect index
From: sashiko-bot @ 2026-06-14 3:10 UTC (permalink / raw)
To: Bryam Vargas via B4 Relay; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260613-b4-disp-4828d263-v1-1-02320e1a89dd@proton.me>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] A zero-length USB packet causes an integer underflow when calculating the packet length, leading to massive out-of-bounds reads and a kernel panic.
--
commit 03557dab27a995017f2c109d738ebd32fc4d550b
Author: Bryam Vargas <hexlabsecurity@proton.me>
Input: iforce - bound the device-reported force-feedback effect index
This commit bounds the force-feedback effect index reported by the device
to prevent an out-of-bounds read-modify-write past the core_effects[] array.
It fixes a memory corruption vulnerability caused by an unvalidated payload.
> diff --git a/drivers/input/joystick/iforce/iforce-packets.c b/drivers/input/joystick/iforce/iforce-packets.c
> index fd1cd731d781a..ff266568d586c 100644
> --- a/drivers/input/joystick/iforce/iforce-packets.c
> +++ b/drivers/input/joystick/iforce/iforce-packets.c
> @@ -186,14 +186,18 @@ void iforce_process_packet(struct iforce *iforce,
>
> /* Check if an effect was just started or stopped */
> i = data[1] & 0x7f;
> - if (data[1] & 0x80) {
> - if (!test_and_set_bit(FF_CORE_IS_PLAYED, iforce->core_effects[i].flags)) {
> - /* Report play event */
> - input_report_ff_status(dev, i, FF_STATUS_PLAYING);
> + if (i < IFORCE_EFFECTS_MAX) {
> + unsigned long *flags = iforce->core_effects[i].flags;
> +
> + if (data[1] & 0x80) {
> + if (!test_and_set_bit(FF_CORE_IS_PLAYED, flags)) {
> + /* Report play event */
> + input_report_ff_status(dev, i, FF_STATUS_PLAYING);
> + }
> + } else if (test_and_clear_bit(FF_CORE_IS_PLAYED, flags)) {
> + /* Report stop event */
> + input_report_ff_status(dev, i, FF_STATUS_STOPPED);
> }
> - } else if (test_and_clear_bit(FF_CORE_IS_PLAYED, iforce->core_effects[i].flags)) {
> - /* Report stop event */
> - input_report_ff_status(dev, i, FF_STATUS_STOPPED);
> }
>
> for (j = 3; j < len; j += 2)
[Severity: Critical]
This is a pre-existing issue, but was not introduced by the patch under review.
Could a zero-length USB packet cause an integer underflow that triggers a
massive out-of-bounds read in this loop?
Looking at iforce_usb_irq() in drivers/input/joystick/iforce/iforce-usb.c:
iforce_process_packet(iforce, iforce_usb->data_in[0],
iforce_usb->data_in + 1, urb->actual_length - 1);
If urb->actual_length is 0, the subtraction underflows to a massive positive
unsigned value, which is passed as len.
If the first byte evaluates to a status report (packet ID 0x02), this loop
would execute against the underflowed len. Could this iterate billions of
times, repeatedly executing mark_core_as_ready() and causing uncontrolled
memory reads until an unmapped page is hit?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613-b4-disp-4828d263-v1-1-02320e1a89dd@proton.me?part=1
^ permalink raw reply
* [PATCH] Input: iforce - bound the device-reported force-feedback effect index
From: Bryam Vargas via B4 Relay @ 2026-06-14 2:58 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
From: Bryam Vargas <hexlabsecurity@proton.me>
iforce_process_packet() handles a status report (packet id 0x02) by
taking a force-feedback effect index straight from the device wire and
using it to address the per-effect state array:
i = data[1] & 0x7f;
if (data[1] & 0x80) {
if (!test_and_set_bit(FF_CORE_IS_PLAYED,
iforce->core_effects[i].flags))
...
} else if (test_and_clear_bit(FF_CORE_IS_PLAYED,
iforce->core_effects[i].flags)) {
...
}
The index is masked only with 0x7f, so it ranges 0..127, but
core_effects[] holds only IFORCE_EFFECTS_MAX (32) entries. For an index
of 32..127 the test_and_set_bit()/test_and_clear_bit() is an
out-of-bounds single-bit read-modify-write past the array. core_effects[]
is the second-to-last member of struct iforce, so the write lands in the
trailing members and beyond the embedding kzalloc()'d iforce_serio /
iforce_usb object.
data[1] is unvalidated device payload on both transports (the USB
interrupt endpoint and serio), and the status path is not gated on force
feedback being present, so a malicious or counterfeit device can set or
clear a bit at an attacker-chosen offset past the object.
Reject an out-of-range index instead of indexing with it. Bound against
the array dimension IFORCE_EFFECTS_MAX rather than dev->ff->max_effects so
the check guarantees memory safety regardless of how many effects the
device registered. A legitimate "effect started/stopped" status always
carries an index below IFORCE_EFFECTS_MAX, so well-formed devices are
unaffected; the neighbouring mark_core_as_ready() loop is already bounded
and is left untouched.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Reachable from a malicious, malfunctioning or counterfeit iforce
force-feedback device on either transport: a USB device matching the
iforce id table (iforce-usb.c forwards the interrupt-IN buffer as
iforce_process_packet(.., data_in[0], data_in + 1, ..)) or an RS232
iforce peripheral bound via inputattach (iforce-serio.c forwards data_in).
data[1] is never validated on the path, and the 0x02 status block runs
regardless of packet length or whether force feedback was created.
struct iforce embeds core_effects[32] as its second-to-last member and is
itself the first member of the kzalloc()'d iforce_serio / iforce_usb
object (~4.8 KB, kmalloc-8k). A status report with data[1] = 0x80 | idx
sets bit FF_CORE_IS_PLAYED in core_effects[idx].flags; for idx up to 127
that is about 13 KB past the end of the array.
Verified with a faithful in-kernel KASAN litmus (the verbatim struct
iforce / iforce_serio layout and the case-0x02 effect-index block),
Linux 7.1.0-rc5, CONFIG_KASAN=y, kasan.fault=panic, x86_64:
Arm A, status report effect index 57 (chosen to clear the kmalloc-8k
bucket; KASAN's kmalloc redzone already trips from index 32):
BUG: KASAN: slab-out-of-bounds in ... Write of size 8
located 3440 bytes to the right of the allocated 4840-byte region
... cache kmalloc-8k of size 8192
Kernel panic - not syncing: kasan.fault=panic set
Arm B, with this patch (index bounded): clean
Arm C, benign in-range index: clean
AddressSanitizer (x86_64 and i386): heap-buffer-overflow WRITE, both ABIs.
The "Write of size 8" is the test_and_set_bit() read-modify-write on the
flags long. Reproducer and full logs available on request.
---
drivers/input/joystick/iforce/iforce-packets.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/input/joystick/iforce/iforce-packets.c b/drivers/input/joystick/iforce/iforce-packets.c
index fd1cd731d781..ff266568d586 100644
--- a/drivers/input/joystick/iforce/iforce-packets.c
+++ b/drivers/input/joystick/iforce/iforce-packets.c
@@ -186,14 +186,18 @@ void iforce_process_packet(struct iforce *iforce,
/* Check if an effect was just started or stopped */
i = data[1] & 0x7f;
- if (data[1] & 0x80) {
- if (!test_and_set_bit(FF_CORE_IS_PLAYED, iforce->core_effects[i].flags)) {
- /* Report play event */
- input_report_ff_status(dev, i, FF_STATUS_PLAYING);
+ if (i < IFORCE_EFFECTS_MAX) {
+ unsigned long *flags = iforce->core_effects[i].flags;
+
+ if (data[1] & 0x80) {
+ if (!test_and_set_bit(FF_CORE_IS_PLAYED, flags)) {
+ /* Report play event */
+ input_report_ff_status(dev, i, FF_STATUS_PLAYING);
+ }
+ } else if (test_and_clear_bit(FF_CORE_IS_PLAYED, flags)) {
+ /* Report stop event */
+ input_report_ff_status(dev, i, FF_STATUS_STOPPED);
}
- } else if (test_and_clear_bit(FF_CORE_IS_PLAYED, iforce->core_effects[i].flags)) {
- /* Report stop event */
- input_report_ff_status(dev, i, FF_STATUS_STOPPED);
}
for (j = 3; j < len; j += 2)
---
base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
change-id: 20260613-b4-disp-4828d263-5f8b1a7c2346
Best regards,
--
Bryam Vargas <hexlabsecurity@proton.me>
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox