[PATCH v1 1/1] leds: core: Bail out when composed name can't fit the buffer

[PATCH v1 1/1] leds: core: Bail out when composed name can't fit the buffer

[Andy Shevchenko]

GCC compiler complains about snprintf() calls that may potentially cut
the output:

 drivers/leds/led-core.c:551:78: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=]
 drivers/leds/led-core.c:554:78: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=]
 ...

Fix these by checking for the potential overflow. This requires
to align all the branches to use the same callee, i.e. snprintf(),
otherwise the code will be blown up and return different error codes
for the different branches.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/leds/led-core.c | 44 ++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index f6c46d2e5276..0eaa19edd6a2 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -515,6 +515,7 @@ int led_compose_name(struct device *dev, struct led_init_data *init_data,
 	struct led_properties props = {};
 	struct fwnode_handle *fwnode = init_data->fwnode;
 	const char *devicename = init_data->devicename;
+	int n;
 
 	if (!led_classdev_name)
 		return -EINVAL;
@@ -528,45 +529,48 @@ int led_compose_name(struct device *dev, struct led_init_data *init_data,
 		 * Otherwise the label is prepended with devicename to compose
 		 * the final LED class device name.
 		 */
-		if (!devicename) {
-			strscpy(led_classdev_name, props.label,
-				LED_MAX_NAME_SIZE);
+		if (devicename) {
+			n = snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
+				     devicename, props.label);
 		} else {
-			snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
-				 devicename, props.label);
+			n = snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s", props.label);
 		}
 	} else if (props.function || props.color_present) {
 		char tmp_buf[LED_MAX_NAME_SIZE];
 
 		if (props.func_enum_present) {
-			snprintf(tmp_buf, LED_MAX_NAME_SIZE, "%s:%s-%d",
-				 props.color_present ? led_colors[props.color] : "",
-				 props.function ?: "", props.func_enum);
+			n = snprintf(tmp_buf, LED_MAX_NAME_SIZE, "%s:%s-%d",
+				     props.color_present ? led_colors[props.color] : "",
+				     props.function ?: "", props.func_enum);
 		} else {
-			snprintf(tmp_buf, LED_MAX_NAME_SIZE, "%s:%s",
-				 props.color_present ? led_colors[props.color] : "",
-				 props.function ?: "");
+			n = snprintf(tmp_buf, LED_MAX_NAME_SIZE, "%s:%s",
+				     props.color_present ? led_colors[props.color] : "",
+				     props.function ?: "");
 		}
-		if (init_data->devname_mandatory) {
-			snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
-				 devicename, tmp_buf);
-		} else {
-			strscpy(led_classdev_name, tmp_buf, LED_MAX_NAME_SIZE);
+		if (n >= LED_MAX_NAME_SIZE)
+			return -E2BIG;
 
+		if (init_data->devname_mandatory) {
+			n = snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
+				     devicename, tmp_buf);
+		} else {
+			n = snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s", tmp_buf);
 		}
 	} else if (init_data->default_label) {
 		if (!devicename) {
 			dev_err(dev, "Legacy LED naming requires devicename segment");
 			return -EINVAL;
 		}
-		snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
-			 devicename, init_data->default_label);
+		n = snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%s:%s",
+			     devicename, init_data->default_label);
 	} else if (is_of_node(fwnode)) {
-		strscpy(led_classdev_name, to_of_node(fwnode)->name,
-			LED_MAX_NAME_SIZE);
+		n = snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%pfw", fwnode);
 	} else
 		return -EINVAL;
 
+	if (n >= LED_MAX_NAME_SIZE)
+		return -E2BIG;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(led_compose_name);
-- 
2.47.2

Re: [PATCH v1 1/1] leds: core: Bail out when composed name can't fit the buffer

[Andy Shevchenko]

On Tue, Mar 18, 2025 at 05:51:25PM +0200, Andy Shevchenko wrote:
> GCC compiler complains about snprintf() calls that may potentially cut
> the output:
> 
>  drivers/leds/led-core.c:551:78: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=]
>  drivers/leds/led-core.c:554:78: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=]
>  ...
> 
> Fix these by checking for the potential overflow. This requires
> to align all the branches to use the same callee, i.e. snprintf(),
> otherwise the code will be blown up and return different error codes
> for the different branches.

...

> -		strscpy(led_classdev_name, to_of_node(fwnode)->name,
> -			LED_MAX_NAME_SIZE);
> +		n = snprintf(led_classdev_name, LED_MAX_NAME_SIZE, "%pfw", fwnode);

Actually this is not an equivalent, should still have that ugly
to_of_node()->name...

I'll fix it in v2.

-- 
With Best Regards,
Andy Shevchenko