Re: [PATCH v2 1/2] scsi: 3w-9xxx: sanitize passthrough SGLs

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing validation of `driver_command.buffer_length` allows out-of-bounds writes on the DMA-allocated bounce buffer during various ioctls.
--

commit 13bf465bbd4ac1f90fcf8616ebc1740e8a86ac93
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>

scsi: 3w-9xxx: sanitize passthrough SGLs

TW_IOCTL_FIRMWARE_PASS_THROUGH copies a full command packet from userspace
and then overwrites the first SGL entry. The SGL location and command
size remain user-controlled for legacy commands.

> diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
> index 9b93a2440af8f..c27c7b632ec6a 100644
> --- a/drivers/scsi/3w-9xxx.c
> +++ b/drivers/scsi/3w-9xxx.c
> @@ -686,6 +688,8 @@ static long twa_chrdev_ioctl(struct file *file, unsigned int cmd, unsigned long
>  	}
>  
>  	tw_ioctl = (TW_Ioctl_Buf_Apache *)cpu_addr;
> +	memset(tw_ioctl, 0, sizeof(TW_Ioctl_Buf_Apache) +
> +	       data_buffer_length_adjusted);

[Severity: High]
This is a pre-existing issue, but does twa_chrdev_ioctl() missing a minimum
size check for driver_command.buffer_length allow out-of-bounds writes?

If a user supplies a buffer_length of 0, data_buffer_length_adjusted
evaluates to 0, which allocates no extra space for the flexible array member
data_buffer.

However, several ioctls blindly write fixed-size structures into this buffer.
For example, TW_IOCTL_GET_COMPATIBILITY_INFO:

	case TW_IOCTL_GET_COMPATIBILITY_INFO:
		tw_ioctl->driver_command.status = 0;
		/* Copy compatibility struct into ioctl data buffer */
		tw_compat_info = (TW_Compatibility_Info *)tw_ioctl->data_buffer;
		memcpy(tw_compat_info, &tw_dev->tw_compat_info, sizeof(TW_Compatibility_Info));
		break;

Does this result in out-of-bounds writes that corrupt neighboring memory on
the DMA heap?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625085842.4522-1-alhouseenyousef@gmail.com?part=1