Re: [PATCH 1/2] scsi: myrb: Fix a potential string truncation in rebuild_show()

[Bart Van Assche <bvanassche@acm.org>]

On 12/12/23 10:09, Christophe JAILLET wrote:
> "physical device - not rebuilding\n" is 34 bytes long. When written in
> 'buf' with a limit of 32 bytes, it is truncated.
> 
> When building with W=1, it leads to:
>     drivers/scsi/myrb.c: In function ‘rebuild_show’:
>     drivers/scsi/myrb.c:1906:24: error: ‘physical device - not rebuil...’ directive output truncated writing 33 bytes into a region of size 32 [-Werror=format-truncation=]
>      1906 |                 return snprintf(buf, 32, "physical device - not rebuilding\n");
>           |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     drivers/scsi/myrb.c:1906:24: note: ‘snprintf’ output 34 bytes into a destination of size 32
> 
> Change the allowed size to 64 to fix the issue.
> 
> Fixes: 081ff398c56c ("scsi: myrb: Add Mylex RAID controller (block interface)")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
>   drivers/scsi/myrb.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c
> index ca2e932dd9b7..ca2380d2d6d3 100644
> --- a/drivers/scsi/myrb.c
> +++ b/drivers/scsi/myrb.c
> @@ -1903,15 +1903,15 @@ static ssize_t rebuild_show(struct device *dev,
>   	unsigned char status;
>   
>   	if (sdev->channel < myrb_logical_channel(sdev->host))
> -		return snprintf(buf, 32, "physical device - not rebuilding\n");
> +		return snprintf(buf, 64, "physical device - not rebuilding\n");
>   
>   	status = myrb_get_rbld_progress(cb, &rbld_buf);
>   
>   	if (rbld_buf.ldev_num != sdev->id ||
>   	    status != MYRB_STATUS_SUCCESS)
> -		return snprintf(buf, 32, "not rebuilding\n");
> +		return snprintf(buf, 64, "not rebuilding\n");
>   
> -	return snprintf(buf, 32, "rebuilding block %u of %u\n",
> +	return snprintf(buf, 64, "rebuilding block %u of %u\n",
>   			rbld_buf.ldev_size - rbld_buf.blocks_left,
>   			rbld_buf.ldev_size);
>   }

Anyone who sees the resulting code without having seen the above patch will
wonder where the magic number '64' comes from. Please use sysfs_emit() instead
of snprintf(buf, 64, ...).

Thanks,

Bart.