Re: [PATCH 1/2] scsi: bsg: fix TOCTOU in io_uring passthrough command setup

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Kernel memory leak of uninitialized sense buffer bytes to user space.
--

--- Patch [1]: [PATCH 1/2] scsi: bsg: fix TOCTOU in io_uring passthrough command setup ---
commit bb53b6e06613a855cd8556487582a73b36038ec3
Author: Rahul Chandelkar <rc@rexion.ai>

scsi: bsg: fix TOCTOU in io_uring passthrough command setup

This commit fixes a TOCTOU vulnerability in scsi_bsg_uring_cmd() by snapshotting
SQE fields with READ_ONCE() before validation and reuse. The stable map
arguments are then passed into scsi_bsg_map_user_buffer().

Link: https://lore.kernel.org/r/20260527105931.3950913-1-rc@rexion.ai

> diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
> index e80dec53174ea..c57ce01379de5 100644
> --- a/drivers/scsi/scsi_bsg.c
> +++ b/drivers/scsi/scsi_bsg.c
[ ... ]
> @@ -158,8 +164,14 @@ static int scsi_bsg_uring_cmd(struct request_queue *q, struct io_uring_cmd *iouc
>  	scmd->sense_len = cmd->max_response_len ?
>  		min(cmd->max_response_len, SCSI_SENSE_BUFFERSIZE) : SCSI_SENSE_BUFFERSIZE;
>  

[Severity: High]
This isn't a bug introduced by this patch, but does overwriting scmd->sense_len
with the maximum response size leak uninitialized kernel memory to user space?

The SCSI midlayer expects sense_len to hold the actual valid length of the
sense data, but it is overwritten here with the user's max_response_len.

If a command fails, the low-level driver might populate a small amount of sense
data (e.g., 18 bytes) without zeroing the rest of the buffer.

The completion handler in drivers/scsi/scsi_bsg.c retrieves this artificially
inflated sense_len to determine how many bytes to copy:

scsi_bsg_uring_task_cb() {
    ...
    if (pdu->response_addr)
        sense_len_wr = min_t(u8, scmd->sense_len,
                             SCSI_SENSE_BUFFERSIZE);
    ...
    if (copy_to_user(uptr64(pdu->response_addr), scmd->sense_buffer,
                     sense_len_wr))
    ...
}

Could this copy uninitialized trailing bytes from the reused sense_buffer to
user space?

> -	if (cmd->dout_xfer_len || cmd->din_xfer_len) {
> -		ret = scsi_bsg_map_user_buffer(req, ioucmd, issue_flags, gfp_mask);
> +	if (dout_xfer_len || din_xfer_len) {
> +		bool is_write = dout_xfer_len > 0;
> +		u64 buf_addr = is_write ? dout_xferp : din_xferp;
> +		unsigned long buf_len = is_write ? dout_xfer_len : din_xfer_len;
> +
> +		ret = scsi_bsg_map_user_buffer(req, ioucmd, issue_flags,
> +					       gfp_mask, is_write, buf_addr,
> +					       buf_len);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720032338.461681-1-yangxiuwei@kylinos.cn?part=1