Input: wacom_w8001 - compose the device basename in one place

PATCH New Ian Bridges 2 participants 2 messages Jul 29

Subsystems (2)

HID Input drivers

lore

Share & tools

lore

Ian Bridges

author

2026-07-29 23:03 UTC #0 lore raw

PATCH 1/1 Input: wacom_w8001 - compose the device basename in one place

The device basename is assembled by appends spread across w8001_setup_pen() and w8001_setup_touch(). Each append restates the buffer and its size.

Record the touch name suffix in struct w8001 beside the other probe results, and compose the whole basename in w8001_connect() with a single scnprintf(). The setup functions lose their buffer and size arguments, and the append helper is no longer needed. The produced names are unchanged.

The single compose point makes the possible length of basename visible to the compiler, and the pen and touch name writes then trip the format truncation warning. Write those two names with scnprintf() as well.

Link https://lore.kernel.org/r/20260716103236.28c1b5bb@pumpkin open here

Signed-off-by Ian Bridges <[email protected]>

---
This is an optional cleanup on top of the already applied strlcat() removal. That patch resolved the unsafe calls. This one only simplifies the result, following David Laight's suggestion in that thread. If the churn is not wanted, no action is needed.

The suggestion was to return the name from the setup functions. The functions already return error codes, so the touch suffix is recorded in struct w8001 beside the other probe results instead, and the pen suffix follows from the error code that w8001_connect() already has.

The patch was tested as follows.

- W=1 build of drivers/input/touchscreen/, zero warnings.
- A differential harness compiled the applied code and this patch side by side and ran both connect flows against identical fabricated query replies, exhaustively over the device state space. The composed names, return values and id assignments were identical in every case.

 drivers/input/touchscreen/wacom_w8001.c | 42 +++++++++++--------------
 1 file changed, 18 insertions(+), 24 deletions(-)

drivers/input/touchscreen/wacom_w8001.c +18 ~~-24~~

index d8d1cdc3f09e..320ef8ddb331 100644
@@ -94,6 +94,7 @@ struct w8001 {
 	char phys[W8001_MAX_PHYS];
 	int type;
 	unsigned int pktlen;
+	const char *touch_sfx;
 	u16 max_touch_x;
 	u16 max_touch_y;
 	u16 max_pen_x;

@@ -417,15 +418,7 @@ static int w8001_detect(struct w8001 *w8001)
 	return 0;
 }
 
-static void w8001_append_suffix(char *dest, const char *suffix, size_t dest_sz)
-{
-	size_t used = strnlen(dest, dest_sz);
-
-	strscpy(dest + used, suffix, dest_sz - used);
-}
-
-static int w8001_setup_pen(struct w8001 *w8001, char *basename,
-			   size_t basename_sz)
+static int w8001_setup_pen(struct w8001 *w8001)
 {
 	struct input_dev *dev = w8001->pen_dev;
 	struct w8001_coord coord;

@@ -460,13 +453,11 @@ static int w8001_setup_pen(struct w8001 *w8001, char *basename,
 	}
 
 	w8001->id = 0x90;
-	w8001_append_suffix(basename, " Penabled", basename_sz);
 
 	return 0;
 }
 
-static int w8001_setup_touch(struct w8001 *w8001, char *basename,
-			     size_t basename_sz)
+static int w8001_setup_touch(struct w8001 *w8001)
 {
 	struct input_dev *dev = w8001->touch_dev;
 	struct w8001_touch_query touch;

@@ -505,19 +496,20 @@ static int w8001_setup_touch(struct w8001 *w8001, char *basename,
 	input_abs_set_res(dev, ABS_X, touch.panel_res);
 	input_abs_set_res(dev, ABS_Y, touch.panel_res);
 
+	w8001->touch_sfx = " Touchscreen";
 	switch (touch.sensor_id) {
 	case 0:
 	case 2:
 		w8001->pktlen = W8001_PKTLEN_TOUCH93;
 		w8001->id = 0x93;
-		w8001_append_suffix(basename, " 1FG", basename_sz);
+		w8001->touch_sfx = " 1FG Touchscreen";
 		break;
 
 	case 1:
 	case 3:
 	case 4:
 		w8001->pktlen = W8001_PKTLEN_TOUCH9A;
-		w8001_append_suffix(basename, " 1FG", basename_sz);
+		w8001->touch_sfx = " 1FG Touchscreen";
 		w8001->id = 0x9a;
 		break;

@@ -541,7 +533,7 @@ static int w8001_setup_touch(struct w8001 *w8001, char *basename,
 		input_abs_set_res(dev, ABS_MT_POSITION_X, touch.panel_res);
 		input_abs_set_res(dev, ABS_MT_POSITION_Y, touch.panel_res);
 
-		w8001_append_suffix(basename, " 2FG", basename_sz);
+		w8001->touch_sfx = " 2FG Touchscreen";
 		if (w8001->max_pen_x && w8001->max_pen_y)
 			w8001->id = 0xE3;
 		else

@@ -549,8 +541,6 @@ static int w8001_setup_touch(struct w8001 *w8001, char *basename,
 		break;
 	}
 
-	w8001_append_suffix(basename, " Touchscreen", basename_sz);
-
 	return 0;
 }

@@ -600,7 +590,7 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 	struct w8001 *w8001;
 	struct input_dev *input_dev_pen;
 	struct input_dev *input_dev_touch;
-	char basename[64] = "Wacom Serial";
+	char basename[64];
 	int err, err_pen, err_touch;
 
 	w8001 = kzalloc_obj(*w8001);

@@ -630,16 +620,20 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 	/* For backwards-compatibility we compose the basename based on
 	 * capabilities and then just append the tool type
 	 */
-	err_pen = w8001_setup_pen(w8001, basename, sizeof(basename));
-	err_touch = w8001_setup_touch(w8001, basename, sizeof(basename));
+	err_pen = w8001_setup_pen(w8001);
+	err_touch = w8001_setup_touch(w8001);
 	if (err_pen && err_touch) {
 		err = -ENXIO;
 		goto fail3;
 	}
 
+	scnprintf(basename, sizeof(basename), "Wacom Serial%s%s",
+		  err_pen ? "" : " Penabled",
+		  err_touch ? "" : w8001->touch_sfx);
+
 	if (!err_pen) {
-		snprintf(w8001->pen_name, sizeof(w8001->pen_name),
-			 "%s Pen", basename);
+		scnprintf(w8001->pen_name, sizeof(w8001->pen_name),
+			  "%s Pen", basename);
 		input_dev_pen->name = w8001->pen_name;
 
 		w8001_set_devdata(input_dev_pen, w8001, serio);

@@ -654,8 +648,8 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 	}
 
 	if (!err_touch) {
-		snprintf(w8001->touch_name, sizeof(w8001->touch_name),
-			 "%s Finger", basename);
+		scnprintf(w8001->touch_name, sizeof(w8001->touch_name),
+			  "%s Finger", basename);
 		input_dev_touch->name = w8001->touch_name;
 
 		w8001_set_devdata(input_dev_touch, w8001, serio);

Signature · 1 line

2.47.3

[email protected]

AI review

2026-07-29 23:10 UTC #1 lore raw

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

Pre-existing issues:
- [High] Calling input_free_device() on a successfully registered device pointer during the error path, leading to a double-free. --

--- Patch [1]: [PATCH] Input: wacom_w8001 - compose the device basename in one place --- commit c5b975c9cd0e7f7aaa34347ef2a6ab2d8f4d90b0
Author: Ian Bridges <[email protected]>

Input: wacom_w8001 - compose the device basename in one place

This commit simplifies the device basename composition in the wacom_w8001 driver. It removes the append helper and instead records the touch name suffix in the device structure, allowing the basename to be assembled in a single step using scnprintf(). The patch resolves format truncation warnings while keeping the produced names identical.

Link https://lore.kernel.org/r/20260716103236.28c1b5bb@pumpkin open here

Quoted 3 lines

drivers/input/touchscreen/wacom_w8001.c +0 -0

[ ... ]

drivers/input/touchscreen/wacom_w8001.c static int w8001_connect(struct serio *serio, struct serio_driver *drv) · line 655

Quoted 11 lines

drivers/input/touchscreen/wacom_w8001.c +2 -2

@@ -654,8 +648,8 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 	}
 
 	if (!err_touch) {
-		snprintf(w8001->touch_name, sizeof(w8001->touch_name),
-			 "%s Finger", basename);
+		scnprintf(w8001->touch_name, sizeof(w8001->touch_name),
+			  "%s Finger", basename);
 		input_dev_touch->name = w8001->touch_name;
 
 		w8001_set_devdata(input_dev_touch, w8001, serio);

[Severity: High]
This is a pre-existing issue, but I noticed a potential double-free in the error handling path of w8001_connect() that occurs just after this code block.

If input_register_device(w8001->touch_dev) fails, the code jumps to the fail4 label:

fail4:

        if (w8001->pen_dev)
                input_unregister_device(w8001->pen_dev);
fail3:
        serio_close(serio);
fail2:
        serio_set_drvdata(serio, NULL);
fail1:
        input_free_device(input_dev_pen);
        input_free_device(input_dev_touch);

Since input_unregister_device() manages dropping the reference count for successfully registered devices, falling through to fail1 calls input_free_device(input_dev_pen) on the same non-NULL pointer, which will drop the reference count a second time.

Could this cause a double-free of the input_dev structure? Would it be safer to set input_dev_pen = NULL in the fail4 block after unregistering?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/amqGsb0A6_dr9JLA@dev?part=1