Re: [PATCH] HID: wacom: replace sprintf calls with sysfs_emit calls in wacom_sys.c

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] HID: wacom: invalid sysfs_emit() call in __wacom_initialize_battery()
--

commit 4c50e84abd55ee59728372c4dfa52c46c6e8c7c7
Author: Nehuen Lian Bova <nehuenlian.kernel@gmail.com>

HID: wacom: replace sprintf calls with sysfs_emit calls in wacom_sys.c

This commit replaces sprintf() calls with sysfs_emit() in drivers/hid/wacom_sys.c
and includes <linux/sysfs.h>.

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 628372ab818a2..bb51ea4b34f89 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c

[ ... ]

> @@ -1838,7 +1839,7 @@ static int __wacom_initialize_battery(struct wacom *wacom,
>  	bat_desc->properties = wacom_battery_props;
>  	bat_desc->num_properties = ARRAY_SIZE(wacom_battery_props);
>  	bat_desc->get_property = wacom_battery_get_property;
> -	sprintf(battery->bat_name, "wacom_battery_%ld", n);
> +	sysfs_emit(battery->bat_name, "wacom_battery_%ld", n);
>  	bat_desc->name = battery->bat_name;

[Severity: High]
Does using sysfs_emit() here break battery registration?

sysfs_emit() is meant for sysfs show callbacks and requires a page-aligned
buffer:

fs/sysfs/file.c:sysfs_emit() {
	if (WARN(!buf || offset_in_page(buf),
		 "invalid sysfs_emit: buf:%p\n", buf))
		return 0;
}

However, battery->bat_name is a 64-byte array embedded inside struct
wacom_battery:

drivers/hid/wacom.h:
struct wacom_battery {
	struct wacom *wacom;
	struct power_supply_desc bat_desc;
	struct power_supply *battery;
	char bat_name[WACOM_NAME_MAX];
	...
};

When __wacom_initialize_battery() runs during device probe or hotplug,
offset_in_page(battery->bat_name) will be non-zero, causing sysfs_emit() to
trigger a WARN() and return 0 without writing the name, leaving
bat_desc->name empty for devm_power_supply_register(). Furthermore, if it
ever were page-aligned, sysfs_emit() assumes a PAGE_SIZE buffer rather than
sizeof(battery->bat_name).

Should this use snprintf(battery->bat_name, sizeof(battery->bat_name), ...)
instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101011439.261812-1-nehuenlian.kernel@gmail.com?part=1