Re: [PATCH 2/4] scsi: proc: use kmalloc() in proc writers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The buffer allocated for the SCSI host write_info callback is not zero-initialized or explicitly null-terminated before being passed to drivers. This causes drivers that use string-parsing functions like simple_strtoul() to read past the user-provided length into uninitialized memory.
--

commit 781e3d263bb9419478bfe3f51bf7dd6d8c93c647
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>

scsi: proc: use kmalloc() in proc writers

proc_scsi_host_write(), proc_scsi_write() and proc_scsi_devinfo_write()
allocate temporary buffers for /proc writes using __get_free_page().
This commit replaces the __get_free_page() calls with kmalloc().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c
> index 1799dcae775c0..fdc355d783da3 100644
> --- a/drivers/scsi/scsi_proc.c
> +++ b/drivers/scsi/scsi_proc.c
[ ... ]
> @@ -74,7 +75,7 @@ static ssize_t proc_scsi_host_write(struct file *file, const char __user *buf,
>  	if (!shost->hostt->write_info)
>  		return -EINVAL;
>  
> -	page = (char *)__get_free_page(GFP_KERNEL);
> +	page = kmalloc(PAGE_SIZE, GFP_KERNEL);
>  	if (page) {
>  		ret = -EFAULT;
>  		if (copy_from_user(page, buf, count))
>  			goto out;
>  		ret = shost->hostt->write_info(shost, page, count);

[Severity: High]
This is a pre-existing issue, but the buffer allocated for the SCSI host
write_info callback is not zero-initialized or explicitly null-terminated
before being passed to drivers.

If a user writes data to a SCSI host /proc file without appending a non-digit
character (like a newline or null terminator), driver callbacks that use
string parsing functions like simple_strtoul() will continue reading past
the user input into uninitialized memory.

For example, this can happen in drivers/scsi/imm.c:imm_write_info():

	dev->mode = simple_strtoul(buffer + 5, NULL, 0);

Could we explicitly null-terminate the buffer or use kzalloc() here, so that
these string parsing functions don't read garbage data from uninitialized
memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-b4-scsi-v1-0-494fb37ebe7b@kernel.org?part=2