Re: [PATCH] enclosure: bound sysfs link name construction

[Greg KH]

On Sun, Mar 29, 2026 at 11:09:43AM +0800, Pengpeng Hou wrote:
> enclosure_link_name() prefixes the component device name with "enclosure_device:" in a fixed 64-byte stack buffer. The helper currently uses strcpy() and strcat() with no remaining-space check even though the component device name itself can approach the same length.

Please wrap your changelog lines at 72 columns.

And the buffer size is fine, no need to change that logic at all, right?

> 
> Switch the helper to bounded formatting and propagate an error when the prefixed link name does not fit.
> 
> Fixes: cb6b7f40630f ("[SCSI] ses: fix up functionality after class_device->device conversion")
> Signed-off-by: Pengpeng Hou <pengpeng@xxxxxxxxxxx>
> ---
>  drivers/misc/enclosure.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
> index cf6382981777..f09707ca02e8 100644
> --- a/drivers/misc/enclosure.c
> +++ b/drivers/misc/enclosure.c
> @@ -182,17 +182,21 @@ EXPORT_SYMBOL_GPL(enclosure_unregister);
>  #define ENCLOSURE_NAME_SIZE	64
>  #define COMPONENT_NAME_SIZE	64
>  
> -static void enclosure_link_name(struct enclosure_component *cdev, char *name)
> +static int enclosure_link_name(struct enclosure_component *cdev, char *name)
>  {
> -	strcpy(name, "enclosure_device:");
> -	strcat(name, dev_name(&cdev->cdev));
> +	if (snprintf(name, ENCLOSURE_NAME_SIZE, "enclosure_device:%s",
> +		     dev_name(&cdev->cdev)) >= ENCLOSURE_NAME_SIZE)
> +		return -EINVAL;

How can this ever be triggered?

> +
> +	return 0;
>  }
>  
>  static void enclosure_remove_links(struct enclosure_component *cdev)
>  {
>  	char name[ENCLOSURE_NAME_SIZE];
>  
> -	enclosure_link_name(cdev, name);
> +	if (enclosure_link_name(cdev, name))
> +		return;
>  
>  	/*
>  	 * In odd circumstances, like multipath devices, something else may
> @@ -214,7 +218,12 @@ static int enclosure_add_links(struct enclosure_component *cdev)
>  	if (error)
>  		return error;
>  
> -	enclosure_link_name(cdev, name);
> +	error = enclosure_link_name(cdev, name);
> +	if (error) {
> +		sysfs_remove_link(&cdev->cdev.kobj, "device");

This looks like an actual real bugfix, how about just sending this
portion?

thanks,

greg k-h