Re: [PATCH v7 3/4] Input: stmfts - support FTS5

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] The FTS5 driver silently drops all stylus and palm tool-type events because the ABS_MT_TOOL_TYPE capability is never configured on the input device.
- [Medium] Prematurely exiting the event parsing loop on non-terminal events can cause dropped touch events and partial frames.

Pre-existing issues:
- [High] sysfs attribute returns uninitialized kernel memory due to incorrect error check.
--

commit aa502b398bf6eeb31cabf6ce200968e3d20a68a7
Author: David Heidelberg [off-list ref]
Subject: Input: stmfts - support FTS5

This commit introduces support for the FTS5 protocol in the stmfts
touchscreen driver. It adds FTS5-specific functions for device configuration,
input setup (including initial stylus handling), and event parsing.

This is a pre-existing issue, but while reviewing I noticed a potential
memory leak in the sysfs read handler stmfts_sysfs_read_status() that
was not introduced by this patch:

drivers/input/touchscreen/stmfts.c:stmfts_sysfs_read_status() {
    ...
    err = i2c_smbus_read_i2c_block_data(sdata->client, STMFTS_READ_STATUS,
                                        sizeof(status), status);
    if (err)
        return err;

    return sysfs_emit(buf, "%#02x\n", status[0]);
}

[Severity: High]
Does this error check leak uninitialized memory? The function
i2c_smbus_read_i2c_block_data() returns the positive number of bytes read
on success. By treating any non-zero return value as an error, a successful
read will immediately return this positive byte count to the sysfs core,
bypassing sysfs_emit() and potentially leaking up to 4 bytes of uninitialized
kernel memory to userspace.

[ ... ]

+static void stmfts5_report_contact_event(struct stmfts_data *sdata,
+					 const u8 event[])
+{

[ ... ]

+	switch (touch_type) {
+	case STMFTS_TOUCH_TYPE_STYLUS:
+		if (sdata->stylus_enabled) {
+			tool = MT_TOOL_PEN;
+			break;
+		}
+		fallthrough; /* Report as finger if stylus not enabled */

[ ... ]

+	input_mt_slot(sdata->input, touch_id);
+	input_mt_report_slot_state(sdata->input, tool, true);

[ ... ]

+static void stmfts5_parse_events(struct stmfts_data *sdata)
+{
+	for (int i = 0; i < STMFTS_STACK_DEPTH; i++) {
+		u8 *event = &sdata->data[i * STMFTS_EVENT_SIZE];

[ ... ]

+		switch (event[0] & STMFTS5_MASK_EVENT_ID) {
+		case STMFTS5_EV_STATUS_UPDATE:
+		case STMFTS5_EV_USER_REPORT:
+		case STMFTS5_EV_DEBUG:
+			goto sync;

[Severity: Medium]
Could this premature loop exit cause subsequent touch events in the same
buffer to be dropped?

If STMFTS5_EV_STATUS_UPDATE, STMFTS5_EV_USER_REPORT, or STMFTS5_EV_DEBUG 
occurs alongside standard touch events in a single read buffer, the 
goto sync; statement will break out of the event parsing loop entirely. 
Would using continue be more appropriate here to skip the non-touch event 
and continue processing the rest of the buffer?

+
+		case STMFTS5_EV_MULTI_TOUCH_ENTER:
+		case STMFTS5_EV_MULTI_TOUCH_MOTION:
+			stmfts5_report_contact_event(sdata, event);
+			break;

[ ... ]

+static int stmfts5_setup_input(struct stmfts_data *sdata)
+{

[ ... ]

+	input_set_abs_params(sdata->input, ABS_MT_DISTANCE, 0, 255, 0, 0);
+
+	/* Enable stylus support if requested */
+	sdata->stylus_enabled = device_property_read_bool(dev, "stylus-enabled");
+
+	return input_mt_init_slots(sdata->input, STMFTS_MAX_FINGERS,
+				   INPUT_MT_DIRECT);
+}

[Severity: Medium]
Does this driver need to configure the ABS_MT_TOOL_TYPE capability for
stylus and palm events to be processed?

In stmfts5_report_contact_event() above, the tool type is reported to the
input core. However, because input_set_abs_params() or input_set_capability()
is not called for ABS_MT_TOOL_TYPE here in stmfts5_setup_input(), the input 
subsystem will silently drop these tool type events, degrading the hardware 
functionality to basic finger touches.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-stmfts5-v7-0-4a53381cd038@ixit.cz?part=3