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

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

[Nehuen Lian Bova]

Replace 'sprintf' calls with safer 'sysfs_emit' calls in wacom_sys.c, also
include the header '<linux/sysfs.h>'. No functional changes.

Signed-off-by: Nehuen Lian Bova <nehuenlian.kernel@gmail.com>
---
 drivers/hid/wacom_sys.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 628372ab818a..bb51ea4b34f8 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -6,6 +6,7 @@
 #include "wacom_wac.h"
 #include "wacom.h"
 #include <linux/input/mt.h>
+#include <linux/sysfs.h>
 
 #define WAC_MSG_RETRIES		5
 #define WAC_CMD_RETRIES		10
@@ -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;
 	bat_desc->type = POWER_SUPPLY_TYPE_BATTERY;
 	bat_desc->use_for_apm = 0;
@@ -1928,7 +1929,7 @@ static ssize_t wacom_show_remote_mode(struct kobject *kobj,
 	u8 mode;
 
 	mode = wacom->led.groups[index].select;
-	return sprintf(buf, "%d\n", mode < 3 ? mode : -1);
+	return sysfs_emit(buf, "%d\n", mode < 3 ? mode : -1);
 }
 
 #define DEVICE_EKR_ATTR_GROUP(SET_ID)					\
-- 
2.43.0

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

[sashiko-bot]

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

[PATCH v2] HID: wacom: replace sprintf calls with safer alternatives

[Nehuen Lian Bova]

Replace sprintf() calls with sysfs_emit() and snprintf() calls in
wacom_sys.c, which are safer and avoid buffer overflows.
Also include the header '<linux/sysfs.h>'

Signed-off-by: Nehuen Lian Bova <nehuenlian.kernel@gmail.com>
---
Changes in v2:
- Use snprintf() instead of sysfs_emit() in __wacom_initialize_battery()
  as suggested by Sashiko bot.
 drivers/hid/wacom_sys.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 628372ab818a..bb5bb9c15f93 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -6,6 +6,7 @@
 #include "wacom_wac.h"
 #include "wacom.h"
 #include <linux/input/mt.h>
+#include <linux/sysfs.h>
 
 #define WAC_MSG_RETRIES		5
 #define WAC_CMD_RETRIES		10
@@ -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);
+	snprintf(battery->bat_name, "wacom_battery_%ld", n);
 	bat_desc->name = battery->bat_name;
 	bat_desc->type = POWER_SUPPLY_TYPE_BATTERY;
 	bat_desc->use_for_apm = 0;
@@ -1928,7 +1929,7 @@ static ssize_t wacom_show_remote_mode(struct kobject *kobj,
 	u8 mode;
 
 	mode = wacom->led.groups[index].select;
-	return sprintf(buf, "%d\n", mode < 3 ? mode : -1);
+	return sysfs_emit(buf, "%d\n", mode < 3 ? mode : -1);
 }
 
 #define DEVICE_EKR_ATTR_GROUP(SET_ID)					\
-- 
2.43.0

Re: [PATCH v2] HID: wacom: replace sprintf calls with safer alternatives

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] HID: wacom: missing size argument in __wacom_initialize_battery() snprintf()
--

commit 3ae99c758bf47da8936a8a4c38001b6a0d39a084
Author: Nehuen Lian Bova <nehuenlian.kernel@gmail.com>

HID: wacom: replace sprintf calls with safer alternatives

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

> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 628372ab818a2..bb5bb9c15f935 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);
> +	snprintf(battery->bat_name, "wacom_battery_%ld", n);

[Severity: High]
Is the buffer size argument missing in this snprintf() call in
__wacom_initialize_battery()?

Because snprintf() expects the buffer size as its second parameter, passing
"wacom_battery_%ld" as the second argument and n as the format string pointer
causes a build failure due to incompatible pointer/integer types (or an
invalid pointer dereference when initializing a battery on device probe or
wireless connection).

Should this pass sizeof(battery->bat_name) as the second argument instead:

	snprintf(battery->bat_name, sizeof(battery->bat_name),
		 "wacom_battery_%ld", n);

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

[PATCH v3] HID: wacom: replace sprintf calls with safer alternatives in wacom_sys.c

[Nehuen Lian Bova]

 Replace sprintf() calls with sysfs_emit() and snprintf() calls in
 wacom_sys.c, which are safer and avoid buffer overflows.
 Also include the header '<linux/sysfs.h>'

Signed-off-by: Nehuen Lian Bova <nehuenlian.kernel@gmail.com>
---
Changes in v2:
- Use snprintf() instead of sysfs_emit() in __wacom_initialize_battery()
  as suggested by Sashiko bot.

Changes in v3:
- Pass sizeof(battery->bat_name) to snprintf() as suggested by
  Sashiko bot.
 drivers/hid/wacom_sys.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 628372ab818a..7d9bbcb13ae0 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -6,6 +6,7 @@
 #include "wacom_wac.h"
 #include "wacom.h"
 #include <linux/input/mt.h>
+#include <linux/sysfs.h>
 
 #define WAC_MSG_RETRIES		5
 #define WAC_CMD_RETRIES		10
@@ -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);
+	snprintf(battery->bat_name, sizeof(battery->bat_name), "wacom_battery_%ld", n);
 	bat_desc->name = battery->bat_name;
 	bat_desc->type = POWER_SUPPLY_TYPE_BATTERY;
 	bat_desc->use_for_apm = 0;
@@ -1928,7 +1929,7 @@ static ssize_t wacom_show_remote_mode(struct kobject *kobj,
 	u8 mode;
 
 	mode = wacom->led.groups[index].select;
-	return sprintf(buf, "%d\n", mode < 3 ? mode : -1);
+	return sysfs_emit(buf, "%d\n", mode < 3 ? mode : -1);
 }
 
 #define DEVICE_EKR_ATTR_GROUP(SET_ID)					\
-- 
2.43.0