Re: [PATCH] Input: wacom_w8001 - replace strlcat() with a strscpy() helper

[sashiko-bot@kernel.org]

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 <icb@fastmail.org>

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.

Link: https://github.com/KSPP/linux/issues/370 [1]

> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> index 45930d7318733..d8d1cdc3f09e3 100644
> --- a/drivers/input/touchscreen/wacom_w8001.c
> +++ b/drivers/input/touchscreen/wacom_w8001.c

[ ... ]

> @@ -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;
>  }

[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?

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