Re: [PATCH v2 1/2] scsi: scsi_debug: scsi: scsi_debug: fix some bugs in sdebug_error_write()

[Wenchao Hao <haowenchao2@huawei.com>]

On 2023/11/6 22:04, Dan Carpenter wrote:
> There are two bug in this code:
> 1) If count is zero, then it will lead to a NULL dereference.  The
>     kmalloc() will successfully allocate zero bytes and the test for
>     "if (buf[0] == '-')" will read beyond the end of the zero size buffer
>     and Oops.
> 2) The code does not ensure that the user's string is properly NUL
>     terminated which could lead to a read overflow.
> 
> Fixes: a9996d722b11 ("scsi: scsi_debug: Add interface to manage error injection for a single device")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> v2: At first I tried to use strndup_user() but that only accepts NUL
>      terminated strings and the user string is normally not terminated.
> 
>   drivers/scsi/scsi_debug.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 67922e2c4c19..0dd21598f7b6 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -1019,7 +1019,7 @@ static ssize_t sdebug_error_write(struct file *file, const char __user *ubuf,
>   	struct sdebug_err_inject *inject;
>   	struct scsi_device *sdev = (struct scsi_device *)file->f_inode->i_private;
>   
> -	buf = kmalloc(count, GFP_KERNEL);
> +	buf = kzalloc(count + 1, GFP_KERNEL);
>   	if (!buf)
>   		return -ENOMEM;
>   
Looks good,

Reviewed-by: Wenchao Hao <haowenchao2@huawei.com>