* [PATCH 05/10] Input: synaptics-rmi4 - block s_input when F54 queue is busy
From: Dmitry Torokhov @ 2026-06-26 5:17 UTC (permalink / raw)
To: Bryam Vargas, Hans Verkuil; +Cc: linux-input, linux-kernel, stable
In-Reply-To: <20260626051802.4033172-1-dmitry.torokhov@gmail.com>
Changing the input (diagnostic report type) mid-stream changes the
report size. Since V4L2 buffers are allocated based on the size at
stream start, changing the input while streaming could lead to a
heap buffer overflow if the new size is larger than the allocated
buffers.
Prevent this by blocking VIDIOC_S_INPUT with -EBUSY if the V4L2 queue
is busy (streaming).
Fixes: 3a762dbd5347 ("[media] Input: synaptics-rmi4 - add support for F54 diagnostics")
Cc: stable@vger.kernel.org
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/rmi4/rmi_f54.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
index aebe74d2032c..e86dfc9ce7d9 100644
--- a/drivers/input/rmi4/rmi_f54.c
+++ b/drivers/input/rmi4/rmi_f54.c
@@ -445,7 +445,12 @@ static int rmi_f54_set_input(struct f54_data *f54, unsigned int i)
static int rmi_f54_vidioc_s_input(struct file *file, void *priv, unsigned int i)
{
- return rmi_f54_set_input(video_drvdata(file), i);
+ struct f54_data *f54 = video_drvdata(file);
+
+ if (vb2_is_busy(&f54->queue))
+ return -EBUSY;
+
+ return rmi_f54_set_input(f54, i);
}
static int rmi_f54_vidioc_g_input(struct file *file, void *priv,
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH 04/10] Input: synaptics-rmi4 - cancel delayed work on F54 remove
From: Dmitry Torokhov @ 2026-06-26 5:17 UTC (permalink / raw)
To: Bryam Vargas, Hans Verkuil; +Cc: linux-input, linux-kernel, sashiko-bot, stable
In-Reply-To: <20260626051802.4033172-1-dmitry.torokhov@gmail.com>
Ensure that any pending delayed work is cancelled before destroying
the workqueue in rmi_f54_remove() to prevent a potential Use-After-Free.
While destroy_workqueue() drains the queue, it does not cancel pending
timers for delayed work. If the timer has not yet expired when
destroy_workqueue() is called, the work is not in the queue yet. Once
the timer expires later, the timer handler will attempt to queue the
work onto the already destroyed workqueue, or access the freed f54
structure (since it is devm-allocated), leading to a crash.
Fixes: 3a762dbd5347 ("[media] Input: synaptics-rmi4 - add support for F54 diagnostics")
Reported-by: sashiko-bot@kernel.org
Cc: stable@vger.kernel.org
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/rmi4/rmi_f54.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
index 75839a54656b..aebe74d2032c 100644
--- a/drivers/input/rmi4/rmi_f54.c
+++ b/drivers/input/rmi4/rmi_f54.c
@@ -749,6 +749,7 @@ static void rmi_f54_remove(struct rmi_function *fn)
video_unregister_device(&f54->vdev);
v4l2_device_unregister(&f54->v4l2);
+ cancel_delayed_work_sync(&f54->work);
destroy_workqueue(f54->workqueue);
}
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH 03/10] Input: synaptics-rmi4 - bound the F54 report size to the allocated buffer
From: Dmitry Torokhov @ 2026-06-26 5:17 UTC (permalink / raw)
To: Bryam Vargas, Hans Verkuil; +Cc: linux-input, linux-kernel, stable
In-Reply-To: <20260626051802.4033172-1-dmitry.torokhov@gmail.com>
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>
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
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 8eac320c43e3..75839a54656b 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 max_report_size;
int report_size;
bool is_busy;
@@ -548,6 +549,13 @@ static void rmi_f54_work(struct work_struct *work)
goto out; /* retry won't help */
}
+ if (report_size > f54->max_report_size) {
+ dev_err(&fn->dev, "Report size %d exceeds buffer size %zu\n",
+ report_size, f54->max_report_size);
+ error = -EINVAL;
+ goto out;
+ }
+
/*
* 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->max_report_size = array3_size(tx, rx, sizeof(u16));
+ f54->report_data = devm_kzalloc(&fn->dev, f54->max_report_size,
GFP_KERNEL);
if (f54->report_data == NULL)
return -ENOMEM;
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH 02/10] Input: synaptics-rmi4 - zero report size on F54 work error
From: Dmitry Torokhov @ 2026-06-26 5:17 UTC (permalink / raw)
To: Bryam Vargas, Hans Verkuil; +Cc: linux-input, linux-kernel, stable, sashiko-bot
In-Reply-To: <20260626051802.4033172-1-dmitry.torokhov@gmail.com>
In rmi_f54_work(), if an error occurs during report request or command
verification, the code jumped directly to the 'error' label, bypassing
the 'abort' label where f54->report_size was normally zeroed out.
This left f54->report_size containing its previous successful payload
size. If a user then altered the V4L2 format to a smaller size, and a
subsequent run failed, rmi_f54_buffer_queue() would copy the stale,
larger payload size into the shrunken V4L2 buffer, causing a heap
buffer overflow.
Fix this by merging the 'abort' and 'error' labels into a single 'out'
exit path, and ensuring that f54->report_size is always set to 0 on
failure by checking for error and zeroing the local report_size first.
Fixes: 3a762dbd5347 ("[media] Input: synaptics-rmi4 - add support for F54 diagnostics")
Cc: stable@vger.kernel.org
Reported-by: sashiko-bot@kernel.org
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/rmi4/rmi_f54.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
index 61909e1a39e2..8eac320c43e3 100644
--- a/drivers/input/rmi4/rmi_f54.c
+++ b/drivers/input/rmi4/rmi_f54.c
@@ -545,7 +545,7 @@ static void rmi_f54_work(struct work_struct *work)
dev_err(&fn->dev, "Bad report size, report type=%d\n",
f54->report_type);
error = -EINVAL;
- goto error; /* retry won't help */
+ goto out; /* retry won't help */
}
/*
@@ -556,7 +556,7 @@ static void rmi_f54_work(struct work_struct *work)
&command);
if (error) {
dev_err(&fn->dev, "Failed to read back command\n");
- goto error;
+ goto out;
}
if (command & F54_GET_REPORT) {
if (time_after(jiffies, f54->timeout)) {
@@ -564,7 +564,7 @@ static void rmi_f54_work(struct work_struct *work)
error = -ETIMEDOUT;
}
report_size = 0;
- goto error;
+ goto out;
}
rmi_dbg(RMI_DEBUG_FN, &fn->dev, "Get report command completed, reading data\n");
@@ -579,7 +579,7 @@ static void rmi_f54_work(struct work_struct *work)
fifo, sizeof(fifo));
if (error) {
dev_err(&fn->dev, "Failed to set fifo start offset\n");
- goto abort;
+ goto out;
}
error = rmi_read_block(fn->rmi_dev, fn->fd.data_base_addr +
@@ -588,16 +588,16 @@ static void rmi_f54_work(struct work_struct *work)
if (error) {
dev_err(&fn->dev, "%s: read [%d bytes] returned %d\n",
__func__, size, error);
- goto abort;
+ goto out;
}
}
-abort:
- f54->report_size = error ? 0 : report_size;
-error:
+out:
if (error)
report_size = 0;
+ f54->report_size = report_size;
+
if (report_size == 0 && !error) {
queue_delayed_work(f54->workqueue, &f54->work,
msecs_to_jiffies(1));
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH 01/10] Input: synaptics-rmi4 - fix F55 transmitter electrode count typo
From: Dmitry Torokhov @ 2026-06-26 5:17 UTC (permalink / raw)
To: Bryam Vargas, Hans Verkuil; +Cc: linux-input, linux-kernel, sashiko-bot, stable
During F55 sensor detection, the transmitter (TX) electrode count was
incorrectly assigned the value of the receiver (RX) electrode count
due to copy-paste typos.
This incorrect value was then propagated to the driver data and used
by F54 to determine the diagnostics report size. On devices with more
RX than TX electrodes, this inflated the perceived TX count, leading
to incorrect report size calculations and potential out-of-bounds
buffer accesses.
Fix the typos by correctly assigning the TX electrode counts.
Fixes: 6adba43fd222 ("Input: synaptics-rmi4 - add support for F55 sensor tuning")
Fixes: c762cc68b6a1 ("Input: synaptics-rmi4 - propagate correct number of rx and tx electrodes to F54")
Reported-by: sashiko-bot@kernel.org
Cc: stable@vger.kernel.org
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/rmi4/rmi_f55.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f55.c b/drivers/input/rmi4/rmi_f55.c
index 488adaca4dd0..a0877d32a914 100644
--- a/drivers/input/rmi4/rmi_f55.c
+++ b/drivers/input/rmi4/rmi_f55.c
@@ -54,10 +54,10 @@ static int rmi_f55_detect(struct rmi_function *fn)
f55->num_tx_electrodes = f55->qry[F55_NUM_TX_OFFSET];
f55->cfg_num_rx_electrodes = f55->num_rx_electrodes;
- f55->cfg_num_tx_electrodes = f55->num_rx_electrodes;
+ f55->cfg_num_tx_electrodes = f55->num_tx_electrodes;
drv_data->num_rx_electrodes = f55->cfg_num_rx_electrodes;
- drv_data->num_tx_electrodes = f55->cfg_num_rx_electrodes;
+ drv_data->num_tx_electrodes = f55->cfg_num_tx_electrodes;
if (f55->qry[F55_PHYS_CHAR_OFFSET] & F55_CAP_SENSOR_ASSIGN) {
int i, total;
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* Re: [PATCH v6 3/3] HID: nintendo: Add unified report format support
From: Vicki Pfau @ 2026-06-26 2:23 UTC (permalink / raw)
To: sashiko-reviews; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260625051335.6F41E1F000E9@smtp.kernel.org>
On 6/24/26 10:13 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Global minimum report size check incorrectly rejects valid legacy, non-unified reports.
> --
>
> commit ebbc8f2a738bc80390ec8c8785093f44578537e8
> Author: Vicki Pfau <vi@endrift.com>
>
> HID: nintendo: Add unified report format support
>
> This commit adds support for the unified report format that all controllers
> also support. It includes overlapping fields for like buttons and axes between
> them.
>
>> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
>> index ae360f4d15d1a..9cb4a503fe2d3 100644
>> --- a/drivers/hid/hid-nintendo.c
>> +++ b/drivers/hid/hid-nintendo.c
>
> [ ... ]
>
>> @@ -3732,7 +3833,7 @@ static int switch2_event(struct hid_device *hdev, struct hid_report *report, uin
>> if (report->type != HID_INPUT_REPORT)
>> return 0;
>>
>> - if (size < 15)
>> + if (size < 17)
>> return -EINVAL;
>>
>
> [Severity: High]
> Does this global size check incorrectly reject valid non-unified reports?
>
> Since non-unified reports like NS2_REPORT_GC only access up to raw_data[14]
> and can legitimately have a size of exactly 15 bytes, it seems this check will
> cause those legitimate 15-byte or 16-byte reports to be dropped with -EINVAL
> in switch2_event().
>
> Should this 17-byte requirement be scoped inside the case NS2_REPORT_UNIFIED:
> block instead?
>
Generally speaking, these reports should always be 63 bytes. This check is mostly here as a basic sanity check to make sure we don't accidentally reach out of bounds in the case of something very weird happening. It's fine if it seems a little overzealous since it's so far beyond normal operation already. I would guess than the Switch 2 rejects anything that short out-of-hand already.
^ permalink raw reply
* Re: [PATCH 0/2] (no cover subject)
From: Dmitry Torokhov @ 2026-06-26 0:47 UTC (permalink / raw)
To: hexlabsecurity
Cc: linux-kernel, Hans de Goede, Vincent Huang, linux-input,
Andrew Duggan
In-Reply-To: <20260614-b4-disp-818d6bda-v1-0-cf39a3615085@proton.me>
On Sun, Jun 14, 2026 at 12:36:10AM -0500, Bryam Vargas via B4 Relay wrote:
> 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.
Applied the lot, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v6 2/2] Input: isa1200 - new driver for Imagis ISA1200
From: Dmitry Torokhov @ 2026-06-25 23:45 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Linus Walleij,
linux-input, devicetree, linux-kernel
In-Reply-To: <20260617070528.35006-3-clamor95@gmail.com>
Hi Svyatoslav,
On Wed, Jun 17, 2026 at 10:05:27AM +0300, Svyatoslav Ryhel wrote:
> From: Linus Walleij <linusw@kernel.org>
>
> The ISA1200 is a haptic feedback unit from Imagis Technology using two
> motors for haptic feedback in mobile phones. Used in many mobile devices
> c. 2012 including Samsung Galxy S Advance GT-I9070 (Janice), Samsung Beam
> GT-I8350 (Gavini), LG Optimus 4X P880 and LG Optimus Vu P895.
>
> The exact datasheet for the ISA1200 is not available; all data was modeled
> based on available downstream kernel sources for various devices and
> fragments of information scattered across the internet.
>
> Tested-by: Linus Walleij <linusw@kernel.org> # GT-I9070 Janice
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
Sashiko convinced me that using mutex_trylock() in the playback work
handler will result in dropping requests and that we are safe not taking
the lock there at all. Can you please try the following modification?
diff --git a/drivers/input/misc/isa1200.c b/drivers/input/misc/isa1200.c
index c61adc4b605c..fb7f68fa0a2b 100644
--- a/drivers/input/misc/isa1200.c
+++ b/drivers/input/misc/isa1200.c
@@ -253,15 +253,12 @@ static void isa1200_stop(struct isa1200 *isa)
static void isa1200_play_work(struct work_struct *work)
{
struct isa1200 *isa = container_of(work, struct isa1200, play_work);
- struct input_dev *input = isa->input;
-
- scoped_guard(mutex_try, &input->mutex) {
- if (!isa->suspended) {
- if (isa->level)
- isa1200_start(isa);
- else
- isa1200_stop(isa);
- }
+
+ if (!READ_ONCE(isa->suspended)) {
+ if (isa->level)
+ isa1200_start(isa);
+ else
+ isa1200_stop(isa);
}
}
If this works no need to resend, I'll fold on my side.
Thanks.
--
Dmitry
^ permalink raw reply related
* Re: [PATCH v2 2/2] arm64: dts: qcom: sdm845-oneplus: Update compatible to include model
From: David Heidelberg @ 2026-06-25 18:39 UTC (permalink / raw)
To: Dmitry Torokhov, Krzysztof Kozlowski
Cc: Krzysztof Kozlowski, Konrad Dybcio, Rob Herring, Conor Dooley,
Jason A. Donenfeld, Matthias Schiffer, Vincent Huang,
Bjorn Andersson, Konrad Dybcio, linux-input, devicetree,
linux-kernel, linux-arm-msm, phone-devel
In-Reply-To: <aj1OhZQjO5nNYlAo@google.com>
On 25/06/2026 18:57, Dmitry Torokhov wrote:
> Hi Krzysztof,
>
> On Thu, Jun 25, 2026 at 10:23:54AM +0200, Krzysztof Kozlowski wrote:
>> On 25/06/2026 06:53, Dmitry Torokhov wrote:
>>> On Wed, Jun 24, 2026 at 04:37:25PM +0200, David Heidelberg wrote:
>>>> On 24/06/2026 06:28, Dmitry Torokhov wrote:
>>>>> Hi David,
>>>>>
>>>>> On Sun, Jun 21, 2026 at 07:11:45PM +0200, David Heidelberg wrote:
>>>>>> On 28/05/2026 00:13, David Heidelberg wrote:
>>>>>>> On 27/05/2026 23:56, Dmitry Torokhov wrote:
>>>>>>>> Hi David,
>>>>>>>>
>>>>>>>> On Sat, May 23, 2026 at 11:45:35AM +0200, David Heidelberg via B4 Relay wrote:
>>>>>>>>> From: David Heidelberg <david@ixit.cz>
>>>>>>>>>
>>>>>>>>> We know the driver is reporting s3706b, introduce the compatible so we
>>>>>>>>> can more easily introduce quirks for weird touchscreen replacements in
>>>>>>>>> followup series.
>>>>>>>>>
>>>>>>>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>>>>>> Signed-off-by: David Heidelberg <david@ixit.cz>
>>>>>>>>> ---
>>>>>>>>> arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi | 2 +-
>>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>>>>>> b/arch/ arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>>>>>> index 6b7378cf4d493..148164d456a5a 100644
>>>>>>>>> --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>>>>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>>>>>> @@ -475,17 +475,17 @@ bq27441_fg: bq27441-battery@55 {
>>>>>>>>> };
>>>>>>>>> };
>>>>>>>>> &i2c12 {
>>>>>>>>> status = "okay";
>>>>>>>>> clock-frequency = <400000>;
>>>>>>>>> synaptics-rmi4-i2c@20 {
>>>>>>>>> - compatible = "syna,rmi4-i2c";
>>>>>>>>> + compatible = "syna,rmi4-s3706b", "syna,rmi4-i2c";
>>>>>>>>
>>>>>>>> So I believe we established that this device (s3706b) does not in fact
>>>>>>>> implement rmi4 protocol properly. Why do we have "syna,rmi4-i2c" as a
>>>>>>>> fallback? Shouldn't it be just "syna,rmi4-s3706b"?
>>>>>>>
>>>>>>> The vendor supplies s3706b which does implement the RMI4 properly.
>>>>>>>
>>>>>>> The 3rd party replacement impersonating original parts may not implement
>>>>>>> it properly, but I don't address this issue in this initial submission.
>>>>>>>
>>>>>>> With this compatible we know which original part is used by the vendor
>>>>>>> and installed in the phones, so later we can deduct specific sequences
>>>>>>> for the replacement aftermarket parts to keep phone touchscreen working
>>>>>>> same as they do on Android without affecting other devices.
>>>>>>
>>>>>> Hello Dmitry.
>>>>>>
>>>>>> May I ask what is currently preventing this series from moving forward?
>>>>>>
>>>>>> The first version was posted in 2023 [1]. I picked it up again in 2025 [2]
>>>>>> and am now on the 9th iteration (this patchset). At this point, the series
>>>>>> has been under discussion for well over a year, with relatively little
>>>>>> feedback and increasingly long gaps between review rounds.
>>>>>>
>>>>>> The current approach is based on the guidance I have received so far,
>>>>>> including suggestions from the device-tree maintainers. When concerns were
>>>>>> raised, I tried to address them and rework the series accordingly.
>>>>>>
>>>>>> What I am struggling with is understanding what specific issue still needs
>>>>>> to be resolved before these patches can be accepted. If there are remaining
>>>>>> requirements, objections to the approach, or technical concerns that I have
>>>>>> not addressed, I would appreciate having them stated explicitly so I can
>>>>>> work on them.
>>>>>>
>>>>>> I also split out the straightforward, self-contained changes in the hope
>>>>>> that at least those could progress independently while I continued working
>>>>>> on any follow-up requirements. However, even those patches do not appear to
>>>>>> be moving forward.
>>>>>>
>>>>>> Could you please clarify what outcome you would like to see from this
>>>>>> series, and what concrete changes would be required to get it accepted?
>>>>>
>>>>> I am still confused about how you want to differentiate between the full
>>>>> RMI4 support vs the OnePlus flavor. The "syna,rmi4-s3706b", as you
>>>>> mentioned, implements RMI4 protocol properly, so we do not need to
>>>>> actually have it documented neither in binding nor in DTS.
>>>>
>>>> --- part 1 ---
>>>>
>>>> This series addresses identification within device-tree. It's normal
>>>> recommended practice.
>>>>
>>>> If we know, the device ships specific, but **compliant** variant, we just
>>>> put it as compatible = "more-specific", "less-specific"; in this case
>>>> "syna,rmi4-s3706b", "syna,rmi4-i2c"
>>>>
>>>> This approach is used everywhere. This has nothing to do with after-market parts.
>>>
>>> We do this in many cases, sometimes when a part has different timings or
>>> maybe additional functionality compared to the base model.
>>
>> Generic expectation is to have always dedicated front compatible for
>> every device. rmi4-i2c is not really specific enough, more like a
>> family, thus a specific device compatible is essential by the DT rules.
>
> Essential in what way? What will break if such compatible is not there?
> We have lived without it for many years and will continue live happily
> without it for years to come.
Hi Dmitry, Krzystof,
Device tree should describe the hardware, rmi4-i2c isn't the exact model of
hardware used, the real hardware is Synaptics S3706B. Device-tree should, where
possible, describe the actual hardware used.
>
> We keep having this conversation each time there is self-describing
> protocol that does not require knowledge of a specific part number:
> i2c-hid, rmi4, spi-hid coming over soon.
While the protocol doesn't require this knowledge, where is the issue provide
the model, at least in the places where we know it?
Does it making things worse to describe hardware in more detail?
>
> We might need a device-specific compatible if we need to implement
> particular power on sequence/adjust timings, and that's when it starts
> making sense to introduce one.
>
>>
>> It does not matter if that specific compatible is ever used.
>>
>>>
>>> How does this new compatible for controller that fully implements RMI4
>>> protocol help here?
>>
>> It does not matter. This is a different device, thus it needs
>> front-specific compatible.
>
> Different from what?
>
> $ git grep syna,rmi4 -- arch/ | wc -l
> 43
>
> Do you have plans to list each and every chip currently covered by
> syna,rmi4* ?
Not really. I would definitely do the chips we know the model and encourage
others to identify chips their devices use, so developers know which hardware is
present. Each vendor should know which touchscreen model they ship with and
provide this information.
Reading the model from the dmesg at runtime is the least optimal way in my eyes.
>
>>
>> Also, the commit msg actually did mention how this helps: allowing
>> further quirks (I did not verify that in practice, but explanation is
>> plausible).
>
> Well, the devil is in the details. And that is what I am trying to
> understand.
This detail is irrelevant for this patchset. This patchset makes what
device-tree should do - describe the hardware independently on any future
patches which may use it in the future.
>
>>
>>>
>>>>
>>>> --- part 2 (irrelevant for this series) ---
>>>>
>>>>>
>>>>> The issue you have with after-market parts that are not compliant and we
>>>>> need to figure out how to deal with them. Inside the driver I
>>>>
>>>> As was suggested by device-tree folks, this is the first step, there isn't
>>>> better one available. If there is, please suggest one, and I'll apply it.
>>>
>>> Was it clearly communicated to DT folks that the compatible you are
>>> adding is fully compatible with the base "syna,rmi4-i2c" but other ones
>>> will not be compatible?
>>
>> That was not communicated but also did not have to. You can install in
>> your board whatever you wish, e.g. replacing foo device with bar being
>> something completely different and incompatible. Does not matter really
>> if this is after-market or a person just swapped things.
>>
>> DT does not solve that problem simply, because we describe static
>> hardware configuration.
>
> But the core issue that David is trying to solve is the fact that these
> headsets do not work well with aftermarket parts with the upstream
> kernels. It is not a theoretical problem for him, it is something that
> he's been trying to solve for a while.
As you can see, I'm trying solve different problem here, and that's describing
the hardware model.
>
> However from my POV I need to make sure the changes to the driver do not
> affect or limit well-behaved devices implementing RMI4 protocol
> properly.
I think we can agree that defining model of touchscreen used in the device-tree
won't affect any hardware you care about.
I'll be more than happy discuss the after-market parts in relevant patchset
submission. These two patches are not introducing anything directly related to
after-market parts, more like opposite defining the hardware which is shipped
with the phones from manufacturer.
David
[...]
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: sdm845-oneplus: Update compatible to include model
From: Dmitry Torokhov @ 2026-06-25 16:57 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: David Heidelberg, Krzysztof Kozlowski, Konrad Dybcio, Rob Herring,
Conor Dooley, Jason A. Donenfeld, Matthias Schiffer,
Vincent Huang, Bjorn Andersson, Konrad Dybcio, linux-input,
devicetree, linux-kernel, linux-arm-msm, phone-devel
In-Reply-To: <f81e4d83-90d9-47c8-aee9-319df3f8b0fb@kernel.org>
Hi Krzysztof,
On Thu, Jun 25, 2026 at 10:23:54AM +0200, Krzysztof Kozlowski wrote:
> On 25/06/2026 06:53, Dmitry Torokhov wrote:
> > On Wed, Jun 24, 2026 at 04:37:25PM +0200, David Heidelberg wrote:
> >> On 24/06/2026 06:28, Dmitry Torokhov wrote:
> >>> Hi David,
> >>>
> >>> On Sun, Jun 21, 2026 at 07:11:45PM +0200, David Heidelberg wrote:
> >>>> On 28/05/2026 00:13, David Heidelberg wrote:
> >>>>> On 27/05/2026 23:56, Dmitry Torokhov wrote:
> >>>>>> Hi David,
> >>>>>>
> >>>>>> On Sat, May 23, 2026 at 11:45:35AM +0200, David Heidelberg via B4 Relay wrote:
> >>>>>>> From: David Heidelberg <david@ixit.cz>
> >>>>>>>
> >>>>>>> We know the driver is reporting s3706b, introduce the compatible so we
> >>>>>>> can more easily introduce quirks for weird touchscreen replacements in
> >>>>>>> followup series.
> >>>>>>>
> >>>>>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> >>>>>>> Signed-off-by: David Heidelberg <david@ixit.cz>
> >>>>>>> ---
> >>>>>>> arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi | 2 +-
> >>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> >>>>>>> b/arch/ arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> >>>>>>> index 6b7378cf4d493..148164d456a5a 100644
> >>>>>>> --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> >>>>>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> >>>>>>> @@ -475,17 +475,17 @@ bq27441_fg: bq27441-battery@55 {
> >>>>>>> };
> >>>>>>> };
> >>>>>>> &i2c12 {
> >>>>>>> status = "okay";
> >>>>>>> clock-frequency = <400000>;
> >>>>>>> synaptics-rmi4-i2c@20 {
> >>>>>>> - compatible = "syna,rmi4-i2c";
> >>>>>>> + compatible = "syna,rmi4-s3706b", "syna,rmi4-i2c";
> >>>>>>
> >>>>>> So I believe we established that this device (s3706b) does not in fact
> >>>>>> implement rmi4 protocol properly. Why do we have "syna,rmi4-i2c" as a
> >>>>>> fallback? Shouldn't it be just "syna,rmi4-s3706b"?
> >>>>>
> >>>>> The vendor supplies s3706b which does implement the RMI4 properly.
> >>>>>
> >>>>> The 3rd party replacement impersonating original parts may not implement
> >>>>> it properly, but I don't address this issue in this initial submission.
> >>>>>
> >>>>> With this compatible we know which original part is used by the vendor
> >>>>> and installed in the phones, so later we can deduct specific sequences
> >>>>> for the replacement aftermarket parts to keep phone touchscreen working
> >>>>> same as they do on Android without affecting other devices.
> >>>>
> >>>> Hello Dmitry.
> >>>>
> >>>> May I ask what is currently preventing this series from moving forward?
> >>>>
> >>>> The first version was posted in 2023 [1]. I picked it up again in 2025 [2]
> >>>> and am now on the 9th iteration (this patchset). At this point, the series
> >>>> has been under discussion for well over a year, with relatively little
> >>>> feedback and increasingly long gaps between review rounds.
> >>>>
> >>>> The current approach is based on the guidance I have received so far,
> >>>> including suggestions from the device-tree maintainers. When concerns were
> >>>> raised, I tried to address them and rework the series accordingly.
> >>>>
> >>>> What I am struggling with is understanding what specific issue still needs
> >>>> to be resolved before these patches can be accepted. If there are remaining
> >>>> requirements, objections to the approach, or technical concerns that I have
> >>>> not addressed, I would appreciate having them stated explicitly so I can
> >>>> work on them.
> >>>>
> >>>> I also split out the straightforward, self-contained changes in the hope
> >>>> that at least those could progress independently while I continued working
> >>>> on any follow-up requirements. However, even those patches do not appear to
> >>>> be moving forward.
> >>>>
> >>>> Could you please clarify what outcome you would like to see from this
> >>>> series, and what concrete changes would be required to get it accepted?
> >>>
> >>> I am still confused about how you want to differentiate between the full
> >>> RMI4 support vs the OnePlus flavor. The "syna,rmi4-s3706b", as you
> >>> mentioned, implements RMI4 protocol properly, so we do not need to
> >>> actually have it documented neither in binding nor in DTS.
> >>
> >> --- part 1 ---
> >>
> >> This series addresses identification within device-tree. It's normal
> >> recommended practice.
> >>
> >> If we know, the device ships specific, but **compliant** variant, we just
> >> put it as compatible = "more-specific", "less-specific"; in this case
> >> "syna,rmi4-s3706b", "syna,rmi4-i2c"
> >>
> >> This approach is used everywhere. This has nothing to do with after-market parts.
> >
> > We do this in many cases, sometimes when a part has different timings or
> > maybe additional functionality compared to the base model.
>
> Generic expectation is to have always dedicated front compatible for
> every device. rmi4-i2c is not really specific enough, more like a
> family, thus a specific device compatible is essential by the DT rules.
Essential in what way? What will break if such compatible is not there?
We have lived without it for many years and will continue live happily
without it for years to come.
We keep having this conversation each time there is self-describing
protocol that does not require knowledge of a specific part number:
i2c-hid, rmi4, spi-hid coming over soon.
We might need a device-specific compatible if we need to implement
particular power on sequence/adjust timings, and that's when it starts
making sense to introduce one.
>
> It does not matter if that specific compatible is ever used.
>
> >
> > How does this new compatible for controller that fully implements RMI4
> > protocol help here?
>
> It does not matter. This is a different device, thus it needs
> front-specific compatible.
Different from what?
$ git grep syna,rmi4 -- arch/ | wc -l
43
Do you have plans to list each and every chip currently covered by
syna,rmi4* ?
>
> Also, the commit msg actually did mention how this helps: allowing
> further quirks (I did not verify that in practice, but explanation is
> plausible).
Well, the devil is in the details. And that is what I am trying to
understand.
>
> >
> >>
> >> --- part 2 (irrelevant for this series) ---
> >>
> >>>
> >>> The issue you have with after-market parts that are not compliant and we
> >>> need to figure out how to deal with them. Inside the driver I
> >>
> >> As was suggested by device-tree folks, this is the first step, there isn't
> >> better one available. If there is, please suggest one, and I'll apply it.
> >
> > Was it clearly communicated to DT folks that the compatible you are
> > adding is fully compatible with the base "syna,rmi4-i2c" but other ones
> > will not be compatible?
>
> That was not communicated but also did not have to. You can install in
> your board whatever you wish, e.g. replacing foo device with bar being
> something completely different and incompatible. Does not matter really
> if this is after-market or a person just swapped things.
>
> DT does not solve that problem simply, because we describe static
> hardware configuration.
But the core issue that David is trying to solve is the fact that these
headsets do not work well with aftermarket parts with the upstream
kernels. It is not a theoretical problem for him, it is something that
he's been trying to solve for a while.
However from my POV I need to make sure the changes to the driver do not
affect or limit well-behaved devices implementing RMI4 protocol
properly.
>
> >
> >>
> >>> essentially need a"incomplete protocol" flag that we can use to
> >>> implement additional checks or skip known to be not implemented
> >>> functions/queries. In DT we could introduce something like
> >>> "oneplus,rmi4-i2c" that is decidedly not compatible with "syna,rmi4-i2c"
> >>> and neither one should be a fallback for the other.
> >>>
> >>> This of course needs buy-in from DT maintainers.
> >>
> >> As you can see, this still holds Acked-by and Reviewed-by from the relevant
> >> people - Krzysztof and Konrad.
> >
> > I see that but the commit does not explain how exactly you are planning
> > to deal with knockoffs.
>
> I think it does not have to. David does not need to solve all possible
> problems. He is solving his problems and not preventing the other
> problems to be solved in the future. Unless you expect that the second
> part - future problems - will be blocked by this code.
Well, as I mentioned above my understanding is that David really wants
to solve the issue with after market parts not working properly.
>
> But then I think no one ever solved in DT a problem of replacement of
> valid parts with after-market incompatible parts. Therefore I do not
> even know what solution I could suggest to David.
>
> >
> >>
> >>>
> >>> Does this make sense?
> >>
> >> For the scope we're discussing it doesn't seems so.
> >>
> >> This discussion should be associated with the last revision of the full
> >> series I sent 3 months ago. We're in very unflattering state, where:
> >>
> >> 2018 - these aftermarket touchscreen worked on Android well enough for
> >> people to have working touch (let's say with slightly worse experience then
> >> the original).
> >>
> >> 2026 in the mainline, we cannot even more forward and report to user-space
> >> there is aftermarket non-compliant piece of hardware installed.
> >>
> >> Actionable steps I suggest after this series lands:
> >>
> >> 1. don't do any changes, but since we know what 3rd party touchscreen do
> >> incorrectly deviating from the standard, REPORT it to the userspace, so USER
> >> know, their device (phone/tablet) doesn't have original part.
> >>
> >> 2. then figure out, IF we can reasonably well workaround it and HOW to do it
> >>
> >> These two steps present some progress which could be discussed and could
> >> lead us somewhere, what do you think?
> >
> > So since we know that these devices can come with controllers that do
> > not implement RMI4 fully, can we:
> >
> > 1. Establish a new compatible that is separate from syna,rmi4-i2c? As I
> > mentioned, it could be oneplus,rmi4-i2c or event a concrete controller
> > vendor,id combo. The point that it should be completely separate from
> > the current compatible and not use the current compatible as a fallback.
>
> Board/machine level compatible already gives you that.
> after-market-incompatible-parts is not a problem of this s3706b, if I
> understood correctly. It feels to me more of a problem of the board:
> people install into this board some incompatible replacement parts.
So is the suggestion to check if the board matches "oneplus,enchilada",
"oneplus,fajitas" with of_machine_compatible_match()?
>
> I would understand that this is not a board/machine level problem, if
> multiple different boards with s3706b were affected. Is this the case?
David?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: usbhid: Use seq_buf_putc() in three functions
From: Markus Elfring @ 2026-06-25 16:55 UTC (permalink / raw)
To: sashiko-reviews, linux-input, linux-usb, Benjamin Tissoires,
Jiri Kosina, Mahad Ibrahim
Cc: Dmitry Torokhov, Steven Rostedt, Woradorn Laodhanadhaworn, LKML,
kernel-janitors
In-Reply-To: <20260625093209.34B6B1F000E9@smtp.kernel.org>
>> +++ b/drivers/hid/usbhid/hid-core.c
>> @@ -1412,7 +1412,7 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
>>
>> if (dev->product) {
>> if (dev->manufacturer)
>> - seq_buf_puts(&hid_name, " ");
>> + seq_buf_putc(&hid_name, ' ');
>
> [Severity: Medium]
> Does this change introduce a missing null termination bug in usbhid_probe()
> if the manufacturer string is exactly 127 bytes long?
>
> The previous code using seq_buf_puts() requires two bytes of available
> capacity (one for the space, one for the trailing null byte) and safely fails
> if there isn't enough room.
I see a need to reconsider such information in more detail.
> Looking at seq_buf_putc(), it only checks if there is room for one byte.
> If the sequence buffer is exactly 128 bytes long
This is the case as such a technical detail can be determined from the member “name”
of the data structure “hid_device”.
https://elixir.bootlin.com/linux/v7.1.1/source/include/linux/hid.h#L721
> and already contains 127
> characters from dev->manufacturer plus the null terminator, seq_buf_putc()
> will overwrite the null terminator at index 127 with a space character.
Other contributors can add further experiences for corresponding data length characteristics.
> This leaves hid->name completely unterminated.
I would like to point the comment “Don't count the trailing null byte against the capacity”
once more from the implementation of the function “seq_buf_puts”.
https://elixir.bootlin.com/linux/v7.1.1/source/lib/seq_buf.c#L193-L194
> When exposed to userspace via
> sysfs or input ioctls, this could lead to an out-of-bounds read into the
> adjacent hid->phys memory field.
If such a data reuse would be attempted, API requirements for sequence buffers
should probably be taken better into account at a concrete place.
Regards,
Markus
^ permalink raw reply
* Re: [PATCH v2] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()
From: sashiko-bot @ 2026-06-25 12:39 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260625121611.2003780-1-sanjayembedded@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The driver incorrectly uses a static global structure `temperature_callbacks` to hold per-device state, causing cross-device data corruption and potential Use-After-Free.
- [High] A race condition during device unbind can lead to a Use-After-Free or NULL pointer dereference because `sensor_hub_remove_callback` does not wait for concurrently executing events.
--
commit 0a06e7ba97cafc3e2c79740421bf8186cb0fc540
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()
The patch switches from devm_iio_device_register() to iio_device_register()
to fix a teardown ordering issue. It adds an explicit iio_device_unregister()
in the remove function to ensure the device is not accessed after its backend
resources begin to be dismantled.
> diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
> index 9f628a8e5cfbe..34bff7e9f3a33 100644
> --- a/drivers/iio/temperature/hid-sensor-temperature.c
> +++ b/drivers/iio/temperature/hid-sensor-temperature.c
> @@ -244,7 +244,7 @@ static int hid_temperature_probe(struct platform_device *pdev)
> if (ret)
> goto error_remove_trigger;
>
> - ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> + ret = iio_device_register(indio_dev);
[Severity: High]
This is a pre-existing issue, but does this driver incorrectly use a static
global structure for per-device state?
Looking at hid_temperature_probe(), we overwrite the shared global pdev
pointer in the statically allocated temperature_callbacks structure:
drivers/iio/temperature/hid-sensor-temperature.c:hid_temperature_probe() {
...
temperature_callbacks.pdev = pdev;
ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE,
&temperature_callbacks);
...
}
If multiple temperature sensors are probed, the most recently probed device
will overwrite this shared pointer. If the last probed device is later
unbound, its driver data is cleared.
Subsequent events for other active devices will dereference the cleared pdev
in temperature_proc_event(), leading to a null pointer dereference or
use-after-free.
> if (ret)
> goto error_remove_callback;
>
> @@ -264,6 +264,7 @@ static void hid_temperature_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct temperature_state *temp_st = iio_priv(indio_dev);
>
> + iio_device_unregister(indio_dev);
> sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE);
[Severity: High]
This isn't a bug introduced by this patch, but can a race condition during
device unbind lead to a use-after-free or null pointer dereference?
The sensor_hub_remove_callback() removes the callback from the sensor hub's
list, but does not synchronize with concurrently executing callback handlers
like sensor_hub_raw_event().
If an event from the sensor hub arrives exactly as the driver is unbinding,
the event handler might fetch the callback just before it is removed. When
hid_temperature_remove() completes, devres frees indio_dev, and the driver
core sets drvdata to NULL.
The concurrent event handler then executes temperature_capture_sample(),
calling platform_get_drvdata(pdev) which either returns NULL or a dangling
pointer to the freed indio_dev, causing a crash.
> hid_sensor_remove_trigger(indio_dev, &temp_st->common_attributes);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625121611.2003780-1-sanjayembedded@gmail.com?part=1
^ permalink raw reply
* [PATCH v2] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()
From: Sanjay Chitroda @ 2026-06-25 12:16 UTC (permalink / raw)
To: jikos, jic23, srinivas.pandruvada
Cc: dlechner, nuno.sa, andy, hongyan.song, linux-input, linux-iio,
linux-kernel
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Avoid using devm_iio_device_register(), as this driver requires explicit
error handling and teardown ordering.
With devm_iio_device_register(), IIO device remains registered until the
devres cleanup phase. However, driver's remove() callback removes the
sensor hub callback and trigger support. This can create a race window
where IIO device is still visible and read_raw() requests are issued.
These requests might call sensor_hub_input_attr_get_raw_value(), which
waits up to 5 seconds for a response from the sensor hub callback that
has already been removed.
Add an explicit iio_device_unregister() call in the teardown path to
ensure deterministic cleanup, so that userspace can no longer access the
device once backend resources begin to be dismantled.
Fixes: 59d0f2da3569 ("iio: hid: Add temperature sensor support")
Cc: stable@vger.kernel.org
Reviewed-by: Maxwell Doose <m32285159@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
Changes in v2:
- Added review tags of Maxwell and Andy along with stable Cc
- Based on input from Srinivas and investigation use-after-free
explanation was not sufficiently justified so updated the commit
message with appropriate information and kept Fixes tag
- Link to v1: https://lore.kernel.org/all/20260622052135.1804135-1-sanjayembedded@gmail.com
---
drivers/iio/temperature/hid-sensor-temperature.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
index a8d3a15f9c53..ab6ec8f659b8 100644
--- a/drivers/iio/temperature/hid-sensor-temperature.c
+++ b/drivers/iio/temperature/hid-sensor-temperature.c
@@ -244,7 +244,7 @@ static int hid_temperature_probe(struct platform_device *pdev)
if (ret)
goto error_remove_trigger;
- ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
+ ret = iio_device_register(indio_dev);
if (ret)
goto error_remove_callback;
@@ -264,6 +264,7 @@ static void hid_temperature_remove(struct platform_device *pdev)
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct temperature_state *temp_st = iio_priv(indio_dev);
+ iio_device_unregister(indio_dev);
sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE);
hid_sensor_remove_trigger(indio_dev, &temp_st->common_attributes);
}
--
--
^ permalink raw reply related
* Re: [PATCH v6 3/6 RESEND] dt-bindings: mfd: motorola-cpcap: convert to DT schema
From: sashiko-bot @ 2026-06-25 10:34 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: dmitry.torokhov, linux-input, lee, linux-leds, robh, conor+dt,
devicetree
In-Reply-To: <20260625081812.33474-4-clamor95@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The YAML schema introduces a `ports` wrapper node for the `audio-codec`, which conflicts with the original TXT binding and existing device trees that define `port@X` directly as children.
- [Medium] The schema explicitly expects the power button subnode to be named exactly `key-power`, but the in-tree device tree uses `button`.
- [Medium] The `spi-cs-high` property, which is physically required by the hardware and was explicitly listed as required in the legacy text binding, has been omitted from the `required` list in the YAML schema.
--
commit 30a40e2d4eb937fd9bbca306ed8246e061a54ab4
Author: Svyatoslav Ryhel <clamor95@gmail.com>
dt-bindings: mfd: motorola-cpcap: convert to DT schema
Convert devicetree bindings for the Motorola CPCAP MFD from TXT to YAML.
> diff --git a/Documentation/devicetree/bindings/mfd/motorola,cpcap.yaml b/Documentation/devicetree/bindings/mfd/motorola,cpcap.yaml
> new file mode 100644
> index 0000000000000..76705ea56805a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/motorola,cpcap.yaml
[ ... ]
> + audio-codec:
> + type: object
> + additionalProperties: false
> +
> + properties:
[ ... ]
> + ports:
> + $ref: /schemas/graph.yaml#/properties/ports
[Severity: Medium]
Will this break validation for existing device trees?
The legacy text binding example shows port@0 and port@1 directly under the
audio-codec node, but this schema requires them to be wrapped in a ports
node. Because additionalProperties is set to false on audio-codec, direct
port@X nodes in existing device trees will be rejected.
[ ... ]
> + charger:
> + $ref: /schemas/power/supply/cpcap-charger.yaml#
> +
> + key-power:
> + $ref: /schemas/input/motorola,cpcap-pwrbutton.yaml#
[Severity: Medium]
Is it intentional to restrict the subnode name strictly to key-power?
Existing in-tree device trees use button for this node. By strictly matching
key-power, existing configurations will be rejected as an undocumented
property during schema validation.
[ ... ]
> +patternProperties:
> + "^led(-[a-z]+)?$":
> + $ref: /schemas/leds/motorola,cpcap-leds.yaml#
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - interrupt-controller
> + - "#interrupt-cells"
> + - spi-max-frequency
[Severity: Medium]
Does this schema incorrectly omit spi-cs-high from the required list?
The legacy text binding explicitly listed spi-cs-high as a required property.
Without it, incomplete device trees lacking this hardware-critical property
will incorrectly pass validation.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625081812.33474-1-clamor95@gmail.com?part=3
^ permalink raw reply
* Re: [PATCH] HID: usbhid: Use seq_buf_putc() in three functions
From: Markus Elfring @ 2026-06-25 9:55 UTC (permalink / raw)
To: sashiko-reviews, linux-input, linux-usb, Benjamin Tissoires,
Jiri Kosina, Mahad Ibrahim
Cc: Dmitry Torokhov, Woradorn Laodhanadhaworn, LKML, kernel-janitors
In-Reply-To: <20260625093209.34B6B1F000E9@smtp.kernel.org>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Replacing `seq_buf_puts()` with `seq_buf_putc()` introduces a NUL-termination bug when the manufacturer string is exactly 127 bytes long, leading to an out-of-bounds read.
These functions were used without error detection so far.
> --
>
> commit c41669030bdd8635988fc175a1caf4183a28344a
> Author: Markus Elfring <elfring@users.sourceforge.net>
>
> HID: usbhid: Use seq_buf_putc() in three functions
>
> This patch updates three probe functions to use seq_buf_putc() instead
> of seq_buf_puts() when inserting a single space character into sequence
> buffers.
>
>> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
>> --- a/drivers/hid/usbhid/hid-core.c
>> +++ b/drivers/hid/usbhid/hid-core.c
>> @@ -1412,7 +1412,7 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
>>
>> if (dev->product) {
>> if (dev->manufacturer)
>> - seq_buf_puts(&hid_name, " ");
>> + seq_buf_putc(&hid_name, ' ');
>
> [Severity: Medium]
> Does this change introduce a missing null termination bug in usbhid_probe()
> if the manufacturer string is exactly 127 bytes long?
>
> The previous code using seq_buf_puts() requires two bytes of available
> capacity (one for the space, one for the trailing null byte) and safely fails
> if there isn't enough room.
Can descriptions be extended for possible failure conditions?
https://elixir.bootlin.com/linux/v7.1.1/source/lib/seq_buf.c#L203-L221
> Looking at seq_buf_putc(), it only checks if there is room for one byte.
> If the sequence buffer is exactly 128 bytes long and already contains 127
> characters from dev->manufacturer plus the null terminator, seq_buf_putc()
> will overwrite the null terminator at index 127 with a space character.
>
> This leaves hid->name completely unterminated. When exposed to userspace via
> sysfs or input ioctls, this could lead to an out-of-bounds read into the
> adjacent hid->phys memory field.
>
>> seq_buf_puts(&hid_name, dev->product);
How likely is it that such product information will be appended as expected
at mentioned source code places?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH] HID: usbhid: Use seq_buf_putc() in three functions
From: sashiko-bot @ 2026-06-25 9:32 UTC (permalink / raw)
To: Markus Elfring; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <89e9c672-efee-4ed2-9ef5-637ec9707f0b@web.de>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Replacing `seq_buf_puts()` with `seq_buf_putc()` introduces a NUL-termination bug when the manufacturer string is exactly 127 bytes long, leading to an out-of-bounds read.
--
commit c41669030bdd8635988fc175a1caf4183a28344a
Author: Markus Elfring <elfring@users.sourceforge.net>
HID: usbhid: Use seq_buf_putc() in three functions
This patch updates three probe functions to use seq_buf_putc() instead
of seq_buf_puts() when inserting a single space character into sequence
buffers.
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -1412,7 +1412,7 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
>
> if (dev->product) {
> if (dev->manufacturer)
> - seq_buf_puts(&hid_name, " ");
> + seq_buf_putc(&hid_name, ' ');
[Severity: Medium]
Does this change introduce a missing null termination bug in usbhid_probe()
if the manufacturer string is exactly 127 bytes long?
The previous code using seq_buf_puts() requires two bytes of available
capacity (one for the space, one for the trailing null byte) and safely fails
if there isn't enough room.
Looking at seq_buf_putc(), it only checks if there is room for one byte.
If the sequence buffer is exactly 128 bytes long and already contains 127
characters from dev->manufacturer plus the null terminator, seq_buf_putc()
will overwrite the null terminator at index 127 with a space character.
This leaves hid->name completely unterminated. When exposed to userspace via
sysfs or input ioctls, this could lead to an out-of-bounds read into the
adjacent hid->phys memory field.
> seq_buf_puts(&hid_name, dev->product);
> }
>
> diff --git a/drivers/hid/usbhid/usbkbd.c b/drivers/hid/usbhid/usbkbd.c
> --- a/drivers/hid/usbhid/usbkbd.c
> +++ b/drivers/hid/usbhid/usbkbd.c
[ ... ]
> @@ -302,7 +302,7 @@ static int usb_kbd_probe(struct usb_interface *iface,
>
> if (dev->product) {
> if (dev->manufacturer)
> - seq_buf_puts(&kbd_name, " ");
> + seq_buf_putc(&kbd_name, ' ');
[Severity: Medium]
Does this have the same unterminated string issue for kbd->name in
usb_kbd_probe()?
If an untrusted USB device presents a manufacturer string of exactly 127
bytes, this overwrite could allow an out-of-bounds read into the adjacent
kbd->phys array.
> seq_buf_puts(&kbd_name, dev->product);
> }
>
> diff --git a/drivers/hid/usbhid/usbmouse.c b/drivers/hid/usbhid/usbmouse.c
> --- a/drivers/hid/usbhid/usbmouse.c
> +++ b/drivers/hid/usbhid/usbmouse.c
[ ... ]
> @@ -150,7 +150,7 @@ static int usb_mouse_probe(struct usb_interface *intf, const struct usb_device_i
>
> if (dev->product) {
> if (dev->manufacturer)
> - seq_buf_puts(&mouse_name, " ");
> + seq_buf_putc(&mouse_name, ' ');
[Severity: Medium]
Is mouse->name vulnerable to the same null terminator overwrite here in
usb_mouse_probe()?
Similarly, this could leak the contents of mouse->phys to userspace.
> seq_buf_puts(&mouse_name, dev->product);
> }
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/89e9c672-efee-4ed2-9ef5-637ec9707f0b@web.de?part=1
^ permalink raw reply
* [PATCH] HID: usbhid: Use seq_buf_putc() in three functions
From: Markus Elfring @ 2026-06-25 9:22 UTC (permalink / raw)
To: linux-input, linux-usb, Benjamin Tissoires, Jiri Kosina,
Mahad Ibrahim
Cc: LKML, kernel-janitors, Woradorn Laodhanadhaworn
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 25 Jun 2026 11:11:26 +0200
A single space character should occasionally be put into a sequence buffer.
Thus use the function “seq_buf_putc” in these implementations.
The source code was transformed by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/hid/usbhid/hid-core.c | 2 +-
drivers/hid/usbhid/usbkbd.c | 2 +-
drivers/hid/usbhid/usbmouse.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 96b0181cf819..a755102b8cfe 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1412,7 +1412,7 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
if (dev->product) {
if (dev->manufacturer)
- seq_buf_puts(&hid_name, " ");
+ seq_buf_putc(&hid_name, ' ');
seq_buf_puts(&hid_name, dev->product);
}
diff --git a/drivers/hid/usbhid/usbkbd.c b/drivers/hid/usbhid/usbkbd.c
index 83d4df0d7a45..02abd54eca49 100644
--- a/drivers/hid/usbhid/usbkbd.c
+++ b/drivers/hid/usbhid/usbkbd.c
@@ -302,7 +302,7 @@ static int usb_kbd_probe(struct usb_interface *iface,
if (dev->product) {
if (dev->manufacturer)
- seq_buf_puts(&kbd_name, " ");
+ seq_buf_putc(&kbd_name, ' ');
seq_buf_puts(&kbd_name, dev->product);
}
diff --git a/drivers/hid/usbhid/usbmouse.c b/drivers/hid/usbhid/usbmouse.c
index b3b2abeee614..9fce51d65cd6 100644
--- a/drivers/hid/usbhid/usbmouse.c
+++ b/drivers/hid/usbhid/usbmouse.c
@@ -150,7 +150,7 @@ static int usb_mouse_probe(struct usb_interface *intf, const struct usb_device_i
if (dev->product) {
if (dev->manufacturer)
- seq_buf_puts(&mouse_name, " ");
+ seq_buf_putc(&mouse_name, ' ');
seq_buf_puts(&mouse_name, dev->product);
}
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v9 6/7] power: supply: Add driver for ASUS Transformer battery
From: sashiko-bot @ 2026-06-25 8:31 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: conor+dt, robh, devicetree, linux-leds, dmitry.torokhov,
linux-input, lee
In-Reply-To: <20260625081529.22447-7-clamor95@gmail.com>
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>
> Driver implements one battery cell per EC controller and supports reading
> of battery status for ASUS Transformer's pad and mobile dock.
>
> Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625081529.22447-1-clamor95@gmail.com?part=6
^ permalink raw reply
* Re: [PATCH v6 5/6 RESEND] mfd: motorola-cpcap: diverge configuration per-board
From: sashiko-bot @ 2026-06-25 8:29 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: devicetree, linux-input, conor+dt, linux-leds, robh, lee,
dmitry.torokhov
In-Reply-To: <20260625081812.33474-6-clamor95@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Repurposing the `motorola,cpcap` compatible string and removing `st,6556002` breaks backwards compatibility with existing Device Trees.
- [Low] Direct cast from `const void *` to `enum cpcap_variant` will cause a `-Wpointer-to-int-cast` compiler warning on 64-bit architectures.
--
commit bcc834e6accda9ec696d0ed365c9e5ada87bff4f
Author: Svyatoslav Ryhel <clamor95@gmail.com>
mfd: motorola-cpcap: diverge configuration per-board
MFD have rigid subdevice structure which does not allow flexible dynamic
subdevice linking. Address this by diverging CPCAP subdevice composition
to take into account board specific configuration.
> diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> index d8243b956f875..5b6ca6b81c232 100644
> --- a/drivers/mfd/motorola-cpcap.c
> +++ b/drivers/mfd/motorola-cpcap.c
[ ... ]
> @@ -241,62 +229,58 @@ static int cpcap_resume(struct device *dev)
[ ... ]
> +static const struct mfd_cell cpcap_default_devices[] = {
> + MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,cpcap-adc"),
> + MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0, "motorola,cpcap-regulator"),
> + MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0, "motorola,cpcap-usb-phy"),
> +};
[Severity: Critical]
Does repurposing the default configuration break backwards compatibility with
existing Device Trees?
Existing hardware using the "motorola,cpcap" compatible string will now
use this generic array. It completely omits the charger and uses non-Mapphone
of_compatible strings, which will prevent existing child nodes (like
"motorola,mapphone-cpcap-adc") from successfully binding.
[ ... ]
> static int cpcap_probe(struct spi_device *spi)
> {
> struct cpcap_ddata *cpcap;
> + const struct mfd_cell *cells;
> + unsigned int num_cells;
> int ret;
>
> cpcap = devm_kzalloc(&spi->dev, sizeof(*cpcap), GFP_KERNEL);
> if (!cpcap)
> return -ENOMEM;
>
> + cpcap->variant = (enum cpcap_variant)spi_get_device_match_data(spi);
[Severity: Low]
Will this direct cast from const void * to enum cpcap_variant cause a
-Wpointer-to-int-cast compiler warning when building on 64-bit architectures?
[ ... ]
> @@ -331,10 +315,29 @@ static int cpcap_probe(struct spi_device *spi)
[ ... ]
> +static const struct of_device_id cpcap_of_match[] = {
> + { .compatible = "motorola,cpcap", .data = (void *)CPCAP_DEFAULT },
> + { .compatible = "motorola,mapphone-cpcap", .data = (void *)CPCAP_MAPPHONE },
> + { /* sentinel */ }
> +};
[Severity: Critical]
Does removing the "st,6556002" compatible string and re-mapping
"motorola,cpcap" to CPCAP_DEFAULT break existing devices?
Devices in the wild using "motorola,cpcap" or "st,6556002" in their
device tree may fail to probe critical PMIC functionality because they
are no longer correctly matched with their required Mapphone components.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625081812.33474-1-clamor95@gmail.com?part=5
^ permalink raw reply
* Re: [PATCH v6 4/6 RESEND] dt-bindings: mfd: motorola-cpcap: document Mapphone and Mot CPCAP
From: sashiko-bot @ 2026-06-25 8:27 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: robh, devicetree, lee, conor+dt, dmitry.torokhov, linux-input,
linux-leds
In-Reply-To: <20260625081812.33474-5-clamor95@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly added device tree compatible strings `motorola,mapphone-cpcap` and `motorola,mot-cpcap` are defined as a standalone enum without a fallback, forcing device trees to either break schema validation or break OMAP platform initialization.
--
commit 6926040210bec2501eaa0528dd29bde0ac318c6c
Author: Svyatoslav Ryhel <clamor95@gmail.com>
dt-bindings: mfd: motorola-cpcap: document Mapphone and Mot CPCAP
Add compatibles for Mapphone and Mot CPCAP subdevice compositions. Both
variations cannot use st,6556002 fallback since they may be based on
different controllers.
> diff --git a/Documentation/devicetree/bindings/mfd/motorola,cpcap.yaml b/Documentation/devicetree/bindings/mfd/motorola,cpcap.yaml
> --- a/Documentation/devicetree/bindings/mfd/motorola,cpcap.yaml
> +++ b/Documentation/devicetree/bindings/mfd/motorola,cpcap.yaml
[ ... ]
> @@ -14,9 +14,14 @@ allOf:
>
> properties:
> compatible:
> - items:
> - - const: motorola,cpcap
> - - const: st,6556002
> + oneOf:
> + - enum:
> + - motorola,mapphone-cpcap
> + - motorola,mot-cpcap
[Severity: High]
Will this schema change break OMAP platform initialization for devices
using these new variants?
The schema defines the new compatibles as a standalone enum. However, the
OMAP platform initialization code explicitly relies on finding a node with the
generic motorola,cpcap compatible to configure the PMIC:
arch/arm/mach-omap2/pmic-cpcap.c:omap4_cpcap_init() {
...
if (!of_find_compatible_node(NULL, NULL, "motorola,cpcap"))
return -ENODEV;
...
}
If a device tree conforms to this new schema (using only the specific variant
compatible), the platform code will fail to find the node, breaking PMIC setup
and CPU voltage scaling.
Conversely, if a device tree defines an items array using motorola,cpcap as a
fallback to successfully initialize the platform code, it will fail schema
validation because this oneOf block expects either a standalone enum or the
specific ["motorola,cpcap", "st,6556002"] items list.
Should the new variants include motorola,cpcap as a fallback in an items
array, or does the OMAP platform code need an update to recognize these
new standalone compatibles?
> +
> + - items:
> + - const: motorola,cpcap
> + - const: st,6556002
>
> reg:
> maxItems: 1
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625081812.33474-1-clamor95@gmail.com?part=4
^ permalink raw reply
* Re: [PATCH v9 3/7] input: serio: Add driver for ASUS Transformer dock keyboard and touchpad
From: sashiko-bot @ 2026-06-25 8:27 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: dmitry.torokhov, conor+dt, linux-input, devicetree, robh,
linux-leds, lee
In-Reply-To: <20260625081529.22447-4-clamor95@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Premature registration of the event notifier before callback initialization causes a race condition leading to a NULL pointer dereference.
- [High] Incorrect devres ordering leaves the notifier active after serio ports are destroyed, causing a Use-After-Free during teardown.
- [High] Incorrect bounds check on the EC packet length allows an out-of-bounds heap read and exposes uninitialized memory.
--
commit 27a9e7909b45cc654374940aa7f4c4574172729b
Author: Michał Mirosław <mirq-linux@rere.qmqm.pl>
input: serio: Add driver for ASUS Transformer dock keyboard and touchpad
Add input driver for ASUS Transformer dock keyboard and touchpad. Modify
incoming keyboard data to send proper scancodes for keys that don't match
their position.
> diff --git a/drivers/input/serio/asus-transformer-ec-kbc.c b/drivers/input/serio/asus-transformer-ec-kbc.c
> new file mode 100644
> index 0000000000000..3ddfa9925b2b0
> --- /dev/null
> +++ b/drivers/input/serio/asus-transformer-ec-kbc.c
[ ... ]
> +static int asus_ec_kbc_notify(struct notifier_block *nb,
> + unsigned long action, void *data_)
> +{
[ ... ]
> + /*
> + * The data[0] is the length of the packet including itself. The data[]
> + * buffer has to be at least 3 bytes (length + ctrl + 1 data byte) and
> + * must not exceed the EC buffer size.
> + */
> + if (data[0] < 2 || data[0] > ASUSEC_ENTRY_BUFSIZE)
> + return NOTIFY_BAD;
> +
> + n = data[0] - 1;
> + data += 2;
[ ... ]
> + while (n--)
> + serio_interrupt(priv->sdev[port_idx], *data++, 0);
[Severity: High]
If data[0] is ASUSEC_ENTRY_BUFSIZE (33), n becomes 32, and data is advanced
by 2. Will the loop read 32 bytes from data[2] up to data[33]?
Since the buffer is exactly 33 bytes long, could this read past the end of
the array or access uninitialized data?
> +
> + return NOTIFY_OK;
> +}
[ ... ]
> +static int asus_ec_kbc_probe(struct platform_device *pdev)
> +{
> + struct asusec_core *ec = dev_get_drvdata(pdev->dev.parent);
> + struct asus_ec_kbc_data *priv;
> + int error;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, priv);
> +
> + priv->ec = ec;
> + priv->parent = to_i2c_client(pdev->dev.parent);
> +
> + error = blocking_notifier_chain_register(&ec->notify_list, &priv->nb);
[Severity: High]
Is it safe to register the notifier here before priv->nb.notifier_call is
initialized?
If an interrupt fires immediately after registration, could the notifier core
attempt to call a NULL pointer?
> + if (error)
> + return dev_err_probe(&pdev->dev, error,
> + "failed to register blocking notifier chain");
> +
> + error = devm_add_action_or_reset(&pdev->dev,
> + asus_ec_notifier_chain_unregister,
> + priv);
[Severity: High]
Since managed resources (devm) are released in LIFO order, will the serio
ports registered below be unregistered and freed before this notifier is
unregistered?
If an interrupt arrives during the unbind window or if the probe function
fails after port registration, could the notifier loop in asus_ec_kbc_notify()
dereference the freed serio port pointers?
> + if (error)
> + return error;
> +
> + error = asus_ec_register_serio(pdev, 0, "Keyboard", 0);
> + if (error)
> + return error;
> +
> + error = asus_ec_register_serio(pdev, 1, "Touchpad", I8042_CMD_AUX_SEND);
> + if (error)
> + return error;
> +
> + priv->nb.notifier_call = asus_ec_kbc_notify;
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625081529.22447-1-clamor95@gmail.com?part=3
^ permalink raw reply
* Re: [PATCH v9 2/7] mfd: Add driver for ASUS Transformer embedded controller
From: sashiko-bot @ 2026-06-25 8:26 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: conor+dt, lee, linux-input, devicetree, robh, linux-leds,
dmitry.torokhov
In-Reply-To: <20260625081529.22447-3-clamor95@gmail.com>
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>
> Support Nuvoton NPCE795-based ECs as used in Asus Transformer TF201,
> TF300T, TF300TG, TF300TL and TF700T pad and dock, as well as TF101 dock
> and TF600T, P1801-T and TF701T pad. This is a glue driver handling
> detection and common operations for EC's functions.
>
> Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625081529.22447-1-clamor95@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v9 4/7] input: keyboard: Add driver for ASUS Transformer dock multimedia keys
From: sashiko-bot @ 2026-06-25 8:25 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: robh, linux-input, dmitry.torokhov, lee, devicetree, conor+dt,
linux-leds
In-Reply-To: <20260625081529.22447-5-clamor95@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Explicitly calling `input_unregister_device()` on a device allocated with `devm_input_allocate_device()`.
--
commit b1b4cf8c044b790c2019bf0bb44a52ad2de52358
Author: Michał Mirosław <mirq-linux@rere.qmqm.pl>
input: keyboard: Add driver for ASUS Transformer dock multimedia keys
This patch adds support for the multimedia top button row of the ASUS
Transformer's Mobile Dock keyboard. It tracks keyboard input events for
AltGr presses to switch to multimedia keys.
> diff --git a/drivers/input/keyboard/asus-transformer-ec-keys.c b/drivers/input/keyboard/asus-transformer-ec-keys.c
> new file mode 100644
> index 0000000000000..53aff3ce7146d
> --- /dev/null
> +++ b/drivers/input/keyboard/asus-transformer-ec-keys.c
[ ... ]
> +static int asus_ec_keys_probe(struct platform_device *pdev)
> +{
> + struct i2c_client *parent = to_i2c_client(pdev->dev.parent);
> + struct asusec_core *ec = dev_get_drvdata(pdev->dev.parent);
> + struct device *dev = &pdev->dev;
> + struct asus_ec_keys_data *priv;
> + int error;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, priv);
> + priv->ec = ec;
> +
> + priv->xidev = devm_input_allocate_device(dev);
[ ... ]
> + error = input_register_device(priv->xidev);
> + if (error)
> + return dev_err_probe(dev, error,
> + "failed to register extension keys\n");
> +
> + error = asus_ec_keys_register_handler(dev, priv);
> + if (error) {
> + input_unregister_device(priv->xidev);
[Severity: Medium]
Since priv->xidev is allocated with devm_input_allocate_device() earlier in
asus_ec_keys_probe(), does explicitly calling input_unregister_device()
here in the error path lead to a double-unregistration when the managed API
automatic cleanup triggers?
> + return error;
> + }
> +
> + priv->nb.notifier_call = asus_ec_keys_notify;
> +
> + error = blocking_notifier_chain_register(&ec->notify_list, &priv->nb);
> + if (error) {
> + input_unregister_device(priv->xidev);
[Severity: Medium]
Could manually unregistering the devm-managed input device here in the
notifier registration error path similarly cause a double-unregistration?
> + input_unregister_handler(&priv->input_handler);
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +static void asus_ec_keys_remove(struct platform_device *pdev)
> +{
> + struct asus_ec_keys_data *priv = platform_get_drvdata(pdev);
> + struct asusec_core *ec = priv->ec;
> +
> + blocking_notifier_chain_unregister(&ec->notify_list, &priv->nb);
> + input_unregister_handler(&priv->input_handler);
> + input_unregister_device(priv->xidev);
[Severity: Medium]
Does bypassing the automatic devm lifecycle by explicitly calling
input_unregister_device() in asus_ec_keys_remove() on a managed device lead
to double-unregistration when the device is unbound?
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625081529.22447-1-clamor95@gmail.com?part=4
^ permalink raw reply
* Re: [PATCH 3/4] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: Konrad Dybcio @ 2026-06-25 8:25 UTC (permalink / raw)
To: Fenglin Wu, linux-arm-msm, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Stephen Boyd,
Bjorn Andersson, Konrad Dybcio
Cc: David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <82253653-bd85-45b8-8520-e2bb213ca48f@oss.qualcomm.com>
On 6/17/26 12:12 PM, Fenglin Wu wrote:
>
> On 6/17/2026 5:30 PM, Konrad Dybcio wrote:
>> On 6/17/26 4:31 AM, Fenglin Wu wrote:
>>>>> + ret = ptn_bulk_write(h, HAP_PTN_FIFO_DIN_0_REG, &data[i], 4);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + for (; i < len; i++) {
>>>>> + ret = ptn_write(h, HAP_PTN_FIFO_DIN_1B_REG, (u8)data[i]);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> + }
>>>> So if i'm reading this right, the first loop will always write
>>>> 4*(len//4) bytes and the second one will be entered at most once,
>>>> to write len rem 4 bytes.. should this be an if instead?
>>> I should put a comment for clarification. Here’s some background: FIFO data writing supports both 4-byte bulk writes using registers [HAP_PTN_FIFO_DIN_0_REG ... HAP_PTN_FIFO_DIN_3_REG], and 1-byte writes using the HAP_PTN_FIFO_DIN_1B_REG register. The 4-byte bulk write is more efficient, especially for waveform which has several Kb data, and it helps to reduce software latency when loading effects and reduce the delay in triggering vibration. It also helps prevent the FIFO from running dry during data refill in FIFO-empty interrupts. Typically, we use 4-byte writes for the initial 4-byte aligned data, and 1-byte writes for any trailing remainder.
>>>
>>> So it still needs a 'for' loop here since the remainder could be more than 1 byte.
>> Right, I mentioned len rem 4 but failed to notice it's a
>> single-byte write.. anyway, a comment here would be good
>>
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Configure the hardware FIFO memory boundary.
>>>>> + * FIFO occupies addresses [0, fifo_len).
>>>>> + */
>>>>> +static int haptics_configure_fifo_mmap(struct qcom_haptics *h)
>>>>> +{
>>>>> + u32 fifo_len, fifo_units;
>>>>> +
>>>>> + /* Config all memory space for FIFO usage for now */
>>>> What's the not-"for now" endgame for this?
>>> The hardware supports more modes than the two currently supported in the driver. One of these, called 'PAT_MEM' mode, also shares memory space with FIFO mode. However, 'PAT_MEM' requires memory to be pre-reserved and waveform data to be pre-loaded. The entire 8K bytes of memory can be divided into partitions, and it is configurable, with FIFO mode always using the first partition [0, fifo_len], where 'fifo_len' is set via the 'MMAP_FIFO_REG' register. 'PAT_MEM' mode plays waveform using data preloaded in a memory bank defined by the registers 'PATX_MEM_START_ADDR_REG' and 'PATTERN_SPMI_PATX_LEN_REG' (they are not defined in the driver). Since PAT_MEM is mainly intended for hardware-triggered vibrations, such as a signal from a dedicated GPIO triggering a short vibration with a preloaded waveform, and although it also supports software triggers, I haven't found a suitable way to support it well into the driver under input FF framework yet. So, I am currently allocating the
>>> entire 8K FIFO memory for FIFO mode only. We can adjust this later if we find a better way to incorporate 'PAT_MEM' mode into the driver.
>> Sounds like a plan.
>>
>> For the other mode, would that GPIO trigger need any OS intervention?
>> Could you speak a bit more about how that works?
>>
>> Konrad
>
> I'll try to clarify the 'PAT_MEM' mode further. 'PAT_MEM' is useful for latency-sensitive vibrations because it preloads the waveform into a fixed memory bank, then it doesn't need to load the data of the effect in the HW before triggering the play. When playback is triggered, it plays the waveform from the specified memory address and length. This memory should be preserved, and the data is preloaded during boot. Unlike FIFO mode, it doesn't allow data refilling. The trigger can come from hardware via dedicated GPIOs—currently, three are supported, each mapping to a memory bank set through specific registers. Software configuration can be done in the bootloader or in the driver probe, but the 'fifo_len' should be adjusted accordingly. After setup, software doesn't need to manage it further, relying on the GPIO signal to activate the playback (for example, a pressure sensor triggering vibration to simulate a physical key press). The trigger can also come from software using
> SPMI commands by setting the play mode, start address, and data length. I previously tried using the 'FF_HAPTIC' effect by mapping 'hid_usage' to a predefined effect in the devicetree, but later I found it unsuitable since 'FF_HAPTIC' is mainly for USB HID touch devices and not general vibration usage. If you have any suggestions for supporting 'PAT_MEM' mode through the input FF framework, please let me know.
I don't really know much about this part of the kernel, but at a glance
FF_HAPTIC seems to be roughly what we're after - quoting the cover letter
of the series that introduced it:
<quote>
Haptic control
..............
The HID protocol described in HUTRR63[3] must be used.
The following waveforms should be supported:
| WAVEFORMNONE | Implicit waveforms required by protocol |
| WAVEFORMSTOP | |
| ------------------------ | ------------------------------------------------- |
| WAVEFORMPRESS | To be used to simulate button press. In device- |
| | controlled mode, it will also be used to simulate |
| | button release. |
| ------------------------ | ------------------------------------------------- |
| WAVEFORMRELEASE | To be used to simulate button release. |
All waveforms will have an associated duration; continuous waveforms will be
ignored by the kernel.
</quote>
(i.e. reword 'press' to 'trigger' and we're mostly there?)
I'd start prototyping in that area (maybe taking it out of drivers/hid/)
and see where it goes. However as I mentioned, this may as well be my
wishful thinking.
Konrad
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: sdm845-oneplus: Update compatible to include model
From: Krzysztof Kozlowski @ 2026-06-25 8:25 UTC (permalink / raw)
To: Dmitry Torokhov, David Heidelberg
Cc: Krzysztof Kozlowski, Konrad Dybcio, Rob Herring, Conor Dooley,
Jason A. Donenfeld, Matthias Schiffer, Vincent Huang,
Bjorn Andersson, Konrad Dybcio, linux-input, devicetree,
linux-kernel, linux-arm-msm, phone-devel
In-Reply-To: <f81e4d83-90d9-47c8-aee9-319df3f8b0fb@kernel.org>
On 25/06/2026 10:23, Krzysztof Kozlowski wrote:
> On 25/06/2026 06:53, Dmitry Torokhov wrote:
>> On Wed, Jun 24, 2026 at 04:37:25PM +0200, David Heidelberg wrote:
>>> On 24/06/2026 06:28, Dmitry Torokhov wrote:
>>>> Hi David,
>>>>
>>>> On Sun, Jun 21, 2026 at 07:11:45PM +0200, David Heidelberg wrote:
>>>>> On 28/05/2026 00:13, David Heidelberg wrote:
>>>>>> On 27/05/2026 23:56, Dmitry Torokhov wrote:
>>>>>>> Hi David,
>>>>>>>
>>>>>>> On Sat, May 23, 2026 at 11:45:35AM +0200, David Heidelberg via B4 Relay wrote:
>>>>>>>> From: David Heidelberg <david@ixit.cz>
>>>>>>>>
>>>>>>>> We know the driver is reporting s3706b, introduce the compatible so we
>>>>>>>> can more easily introduce quirks for weird touchscreen replacements in
>>>>>>>> followup series.
>>>>>>>>
>>>>>>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>>>>> Signed-off-by: David Heidelberg <david@ixit.cz>
>>>>>>>> ---
>>>>>>>> arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi | 2 +-
>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>>>>> b/arch/ arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>>>>> index 6b7378cf4d493..148164d456a5a 100644
>>>>>>>> --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>>>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>>>>> @@ -475,17 +475,17 @@ bq27441_fg: bq27441-battery@55 {
>>>>>>>> };
>>>>>>>> };
>>>>>>>> &i2c12 {
>>>>>>>> status = "okay";
>>>>>>>> clock-frequency = <400000>;
>>>>>>>> synaptics-rmi4-i2c@20 {
>>>>>>>> - compatible = "syna,rmi4-i2c";
>>>>>>>> + compatible = "syna,rmi4-s3706b", "syna,rmi4-i2c";
>>>>>>>
>>>>>>> So I believe we established that this device (s3706b) does not in fact
>>>>>>> implement rmi4 protocol properly. Why do we have "syna,rmi4-i2c" as a
>>>>>>> fallback? Shouldn't it be just "syna,rmi4-s3706b"?
>>>>>>
>>>>>> The vendor supplies s3706b which does implement the RMI4 properly.
>>>>>>
>>>>>> The 3rd party replacement impersonating original parts may not implement
>>>>>> it properly, but I don't address this issue in this initial submission.
>>>>>>
>>>>>> With this compatible we know which original part is used by the vendor
>>>>>> and installed in the phones, so later we can deduct specific sequences
>>>>>> for the replacement aftermarket parts to keep phone touchscreen working
>>>>>> same as they do on Android without affecting other devices.
>>>>>
>>>>> Hello Dmitry.
>>>>>
>>>>> May I ask what is currently preventing this series from moving forward?
>>>>>
>>>>> The first version was posted in 2023 [1]. I picked it up again in 2025 [2]
>>>>> and am now on the 9th iteration (this patchset). At this point, the series
>>>>> has been under discussion for well over a year, with relatively little
>>>>> feedback and increasingly long gaps between review rounds.
>>>>>
>>>>> The current approach is based on the guidance I have received so far,
>>>>> including suggestions from the device-tree maintainers. When concerns were
>>>>> raised, I tried to address them and rework the series accordingly.
>>>>>
>>>>> What I am struggling with is understanding what specific issue still needs
>>>>> to be resolved before these patches can be accepted. If there are remaining
>>>>> requirements, objections to the approach, or technical concerns that I have
>>>>> not addressed, I would appreciate having them stated explicitly so I can
>>>>> work on them.
>>>>>
>>>>> I also split out the straightforward, self-contained changes in the hope
>>>>> that at least those could progress independently while I continued working
>>>>> on any follow-up requirements. However, even those patches do not appear to
>>>>> be moving forward.
>>>>>
>>>>> Could you please clarify what outcome you would like to see from this
>>>>> series, and what concrete changes would be required to get it accepted?
>>>>
>>>> I am still confused about how you want to differentiate between the full
>>>> RMI4 support vs the OnePlus flavor. The "syna,rmi4-s3706b", as you
>>>> mentioned, implements RMI4 protocol properly, so we do not need to
>>>> actually have it documented neither in binding nor in DTS.
>>>
>>> --- part 1 ---
>>>
>>> This series addresses identification within device-tree. It's normal
>>> recommended practice.
>>>
>>> If we know, the device ships specific, but **compliant** variant, we just
>>> put it as compatible = "more-specific", "less-specific"; in this case
>>> "syna,rmi4-s3706b", "syna,rmi4-i2c"
>>>
>>> This approach is used everywhere. This has nothing to do with after-market parts.
>>
>> We do this in many cases, sometimes when a part has different timings or
>> maybe additional functionality compared to the base model.
>
> Generic expectation is to have always dedicated front compatible for
> every device. rmi4-i2c is not really specific enough, more like a
> family, thus a specific device compatible is essential by the DT rules.
>
> It does not matter if that specific compatible is ever used.
>
>>
>> How does this new compatible for controller that fully implements RMI4
>> protocol help here?
>
> It does not matter. This is a different device, thus it needs
> front-specific compatible.
>
> Also, the commit msg actually did mention how this helps: allowing
> further quirks (I did not verify that in practice, but explanation is
> plausible).
>
>>
>>>
>>> --- part 2 (irrelevant for this series) ---
>>>
>>>>
>>>> The issue you have with after-market parts that are not compliant and we
>>>> need to figure out how to deal with them. Inside the driver I
>>>
>>> As was suggested by device-tree folks, this is the first step, there isn't
>>> better one available. If there is, please suggest one, and I'll apply it.
>>
>> Was it clearly communicated to DT folks that the compatible you are
>> adding is fully compatible with the base "syna,rmi4-i2c" but other ones
>> will not be compatible?
>
> That was not communicated but also did not have to. You can install in
> your board whatever you wish, e.g. replacing foo device with bar being
> something completely different and incompatible. Does not matter really
> if this is after-market or a person just swapped things.
>
> DT does not solve that problem simply, because we describe static
> hardware configuration.
>
>>
>>>
>>>> essentially need a"incomplete protocol" flag that we can use to
>>>> implement additional checks or skip known to be not implemented
>>>> functions/queries. In DT we could introduce something like
>>>> "oneplus,rmi4-i2c" that is decidedly not compatible with "syna,rmi4-i2c"
>>>> and neither one should be a fallback for the other.
>>>>
>>>> This of course needs buy-in from DT maintainers.
>>>
>>> As you can see, this still holds Acked-by and Reviewed-by from the relevant
>>> people - Krzysztof and Konrad.
>>
>> I see that but the commit does not explain how exactly you are planning
>> to deal with knockoffs.
>
> I think it does not have to. David does not need to solve all possible
> problems. He is solving his problems and not preventing the other
> problems to be solved in the future. Unless you expect that the second
> part - future problems - will be blocked by this code.
>
> But then I think no one ever solved in DT a problem of replacement of
> valid parts with after-market incompatible parts. Therefore I do not
> even know what solution I could suggest to David.
... maybe except DT overlays. This DTS would be correct and if anyone
installs replacement part which is not compatible, then just use overlay
changing the compatible. The idea of manually (per-user) altering
hardware is matching the concept behind DT overlays.
Best regards,
Krzysztof
^ 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