[PATCH] scsi: proc: use snprintf() for the host name

[PATCH] scsi: proc: use snprintf() for the host name

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

scsi_proc_host_add() and scsi_proc_host_rm() format host_no into a
10-byte buffer with sprintf().  host_no is an unsigned int, and once
it grows past nine digits the %d output plus the terminator no longer
fits; use snprintf().

Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c
index fdc355d783da..d1f474c28720 100644
--- a/drivers/scsi/scsi_proc.c
+++ b/drivers/scsi/scsi_proc.c
@@ -230,7 +230,7 @@ void scsi_proc_host_add(struct Scsi_Host *shost)
 	if (!e)
 		goto err;
 
-	sprintf(name,"%d", shost->host_no);
+	snprintf(name, sizeof(name), "%d", shost->host_no);
 	p = proc_create_data(name, S_IRUGO | S_IWUSR, e->proc_dir,
 			     &proc_scsi_ops, shost);
 	if (!p)
@@ -260,7 +260,7 @@ void scsi_proc_host_rm(struct Scsi_Host *shost)
 	if (!e)
 		return;
 
-	sprintf(name,"%d", shost->host_no);
+	snprintf(name, sizeof(name), "%d", shost->host_no);
 	remove_proc_entry(name, e->proc_dir);
 }
 /**

Re: [PATCH] scsi: proc: use snprintf() for the host name

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] scsi: proc: host_no truncation in scsi_proc_host_add() and
  scsi_proc_host_rm()
--

commit 3f018a4fb0e8a43b35d69140bbb854cdcd245155
Author: Shao Mingyin <shao.mingyin@zte.com.cn>

scsi: proc: use snprintf() for the host name

This commit replaces sprintf() with snprintf() when formatting
shost->host_no into the 10-byte name buffer in scsi_proc_host_add() and
scsi_proc_host_rm().

> diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c
> index fdc355d783da3..d1f474c287206 100644
> --- a/drivers/scsi/scsi_proc.c
> +++ b/drivers/scsi/scsi_proc.c
> @@ -230,7 +230,7 @@ void scsi_proc_host_add(struct Scsi_Host *shost)
>  	if (!e)
>  		goto err;
>  
> -	sprintf(name,"%d", shost->host_no);
> +	snprintf(name, sizeof(name), "%d", shost->host_no);
>  	p = proc_create_data(name, S_IRUGO | S_IWUSR, e->proc_dir,
>  			     &proc_scsi_ops, shost);
>  	if (!p)
> @@ -260,7 +260,7 @@ void scsi_proc_host_rm(struct Scsi_Host *shost)
>  	if (!e)
>  		return;
>  
> -	sprintf(name,"%d", shost->host_no);
> +	snprintf(name, sizeof(name), "%d", shost->host_no);
>  	remove_proc_entry(name, e->proc_dir);
>  }
>  /**

[Severity: Medium]
Does keeping char name[10] still truncate 10-digit shost->host_no values to
9 digits?  Once shost->host_no reaches 10 digits, proc_create_data()
collides with the existing 9-digit procfs entry, and later calling
scsi_proc_host_rm() removes that active host's entry instead.

Formatting a 32-bit int with %d into a 10-byte buffer via an unchecked
snprintf() also triggers -Wformat-truncation warnings under W=1 builds.
Sizing name[] to at least 11 bytes and using %u for the unsigned int
shost->host_no avoids both truncation and the compiler warning.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109113810258gK5WuYWQKt0HsI2ikxmHo@zte.com.cn?part=1

Re: [PATCH] scsi: proc: use snprintf() for the host name

[shao.mingyin]

> Does keeping char name[10] still truncate 10-digit shost->host_no values to
> 9 digits?  Once shost->host_no reaches 10 digits, proc_create_data()
> collides with the existing 9-digit procfs entry, and later calling
> scsi_proc_host_rm() removes that active host's entry instead.
>
> Formatting a 32-bit int with %d into a 10-byte buffer via an unchecked
> snprintf() also triggers -Wformat-truncation warnings under W=1 builds.
> Sizing name[] to at least 11 bytes and using %u for the unsigned int
> shost->host_no avoids both truncation and the compiler warning.

Agreed, and thanks for catching it.  snprintf() alone stops the buffer
overflow but still truncates a ten-digit host_no to nine digits, which
would collide with an existing nine-digit procfs entry.  I'll widen the
buffer to 11 bytes, use %u (host_no is an unsigned int), and repost the
patch as v2.