Skip to contentSkip to conversation

Input: wacom_w8001 - replace strlcat() with a strscpy() helper

PATCH Applied Ian Bridges 4 participants 4 messages Jul 15 – Jul 16

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Input: wacom_w8001 - replace strlcat() with a strscpy() helper

In preparation for removing the strlcat() API[1], replace its five uses with a small append helper built on strnlen() and strscpy().

The five calls append device name fragments to a basename buffer that grows in place across the setup functions. The helper takes the same arguments as strlcat() and writes the same bytes, including when a fragment is truncated.

---
The produced device names are unchanged.

A seq_buf conversion was considered instead of the helper and set aside for two reasons. seq_buf_puts() drops a whole fragment when it does not fit, while strlcat() writes the partial fragment, so the produced bytes would change at the truncation boundary. The setup functions would also change signature to take a struct seq_buf pointer. A v2 can adopt seq_buf if that tradeoff is preferred.

The patch was tested as follows.

- W=1 build of drivers/input/touchscreen/, zero warnings.
- A userspace differential harness compiled the old and the new functions side by side. The name buffers were byte identical over their full length, and the return values and id assignments matched in every case.

 drivers/input/touchscreen/wacom_w8001.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)
drivers/input/touchscreen/wacom_w8001.c +12 -5
index 45930d731873..d8d1cdc3f09e 100644
@@ -417,6 +417,13 @@ 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)
 {
@@ -453,7 +460,7 @@ static int w8001_setup_pen(struct w8001 *w8001, char *basename,
 	}
 
 	w8001->id = 0x90;
-	strlcat(basename, " Penabled", basename_sz);
+	w8001_append_suffix(basename, " Penabled", basename_sz);
 
 	return 0;
 }
@@ -503,14 +510,14 @@ static int w8001_setup_touch(struct w8001 *w8001, char *basename,
 	case 2:
 		w8001->pktlen = W8001_PKTLEN_TOUCH93;
 		w8001->id = 0x93;
-		strlcat(basename, " 1FG", basename_sz);
+		w8001_append_suffix(basename, " 1FG", basename_sz);
 		break;
 
 	case 1:
 	case 3:
 	case 4:
 		w8001->pktlen = W8001_PKTLEN_TOUCH9A;
-		strlcat(basename, " 1FG", basename_sz);
+		w8001_append_suffix(basename, " 1FG", basename_sz);
 		w8001->id = 0x9a;
 		break;
 
@@ -534,7 +541,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);
 
-		strlcat(basename, " 2FG", basename_sz);
+		w8001_append_suffix(basename, " 2FG", basename_sz);
 		if (w8001->max_pen_x && w8001->max_pen_y)
 			w8001->id = 0xE3;
 		else
@@ -542,7 +549,7 @@ static int w8001_setup_touch(struct w8001 *w8001, char *basename,
 		break;
 	}
 
-	strlcat(basename, " Touchscreen", basename_sz);
+	w8001_append_suffix(basename, " Touchscreen", basename_sz);
 
 	return 0;
 }
Signature · 1 line
2.47.3
#1 lore raw
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

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 after input_unregister_device() in the error path leads to a Use-After-Free / Double Free vulnerability. --

--- Patch [1]: [PATCH] Input: wacom_w8001 - replace strlcat() with a strscpy() helper --- commit e4f21712f81562322908d563a501b6a8bead0360
Author: Ian Bridges <[email protected]>

Input: wacom_w8001 - replace strlcat() with a strscpy() helper

This commit replaces five uses of strlcat() with a custom strscpy()-based helper, w8001_append_suffix(). This prepares for the eventual removal of the strlcat() API.

Quoted 4 lines
drivers/input/touchscreen/wacom_w8001.c +0 -0
index 45930d7318733..d8d1cdc3f09e3 100644

[ ... ]

Quoted 10 lines
drivers/input/touchscreen/wacom_w8001.c +1 -1
@@ -534,7 +541,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);
 
-		strlcat(basename, " 2FG", basename_sz);
+		w8001_append_suffix(basename, " 2FG", basename_sz);
 		if (w8001->max_pen_x && w8001->max_pen_y)
 			w8001->id = 0xE3;
 		else

drivers/input/touchscreen/wacom_w8001.c static int w8001_setup_touch(struct w8001 *w8001, char *basename, · line 555

Quoted 8 lines
drivers/input/touchscreen/wacom_w8001.c +1 -1
@@ -542,7 +549,7 @@ static int w8001_setup_touch(struct w8001 *w8001, char *basename,
 		break;
 	}
 
-	strlcat(basename, " Touchscreen", basename_sz);
+	w8001_append_suffix(basename, " Touchscreen", basename_sz);
 
 	return 0;
 }

[Severity: High]
This isn't a bug introduced by this patch, but calling input_free_device() on a successfully registered device after input_unregister_device() in the error path of w8001_connect() leads to a double free.

If w8001_connect() successfully registers the pen device but subsequently fails to register the touch device, execution jumps to the fail4 label:

drivers/input/touchscreen/wacom_w8001.c:w8001_connect() {
...

	if (!err_touch) {
		...
		err = input_register_device(w8001->touch_dev);
		if (err)
			goto fail4;
...
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);

...
}

The fail4 block calls input_unregister_device(w8001->pen_dev), which drops the device reference. Execution then falls through to the fail1 label, where input_free_device(input_dev_pen) is unconditionally called. Because input_dev_pen and w8001->pen_dev point to the same device, this results in a second free.

Could this error path be updated to avoid the double free?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/albg4Rv7QxvLJD05@dev?part=1
maintainer Applied
#2 lore raw
In reply to Ian Bridges
Quoted 10 lines · Ian Bridges

In preparation for removing the strlcat() API[1], replace its five uses with a small append helper built on strnlen() and strscpy().

The five calls append device name fragments to a basename buffer that grows in place across the setup functions. The helper takes the same arguments as strlcat() and writes the same bytes, including when a fragment is truncated.

Applied, thank you.

Signature · 1 line
Dmitry
#3 lore raw
In reply to Ian Bridges
Quoted 7 lines · On Tue, 14 Jul 2026 20:22:41 -0500 Ian Bridges <[email protected]> wrote:

In preparation for removing the strlcat() API[1], replace its five uses with a small append helper built on strnlen() and strscpy().

The five calls append device name fragments to a basename buffer that grows in place across the setup functions. The helper takes the same arguments as strlcat() and writes the same bytes, including when a fragment is truncated.

You can do much better than that...

Change w8001_setup_touch/pen() to return the 'const char *' name, then put everything together in the final snprintf().

David