Re: [PATCH] scsi: bsg: Pass reply_len to bsg_setup_queue() to fit UFS ARPMB replies

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] block/bsg: infoleak of recycled reply buffer on synchronous LLD error
--

commit 52ec043e68f598e70d8f2fbea510e68eefe133cc
Author: Stanley Jhu <stanleyjhu@google.com>

scsi: bsg: Pass reply_len to bsg_setup_queue() to fit UFS ARPMB replies

This commit updates bsg_setup_queue() to accept a reply_len parameter,
allowing BSG transports like UFS to specify custom reply buffer sizes.
It modifies bsg-lib to use this dynamic length for memory allocation
and reply copying.

> diff --git a/block/bsg-lib.c b/block/bsg-lib.c
> index 895db30a7033d..536cee4430a76 100644
> --- a/block/bsg-lib.c
> +++ b/block/bsg-lib.c
> @@ -50,7 +53,7 @@ static int bsg_transport_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
>  	reply = job->reply;
>  	memset(job, 0, sizeof(*job));
>  	job->reply = reply;
> -	job->reply_len = SCSI_SENSE_BUFFERSIZE;
> +	job->reply_len = bset->reply_len;

[Severity: Medium]
Does this code leak the contents of the recycled reply buffer to user space
on a synchronous low level driver error?

The job->reply buffer is allocated once per tag during queue setup and is
reused across requests. While the job struct itself is cleared with memset()
here, the job->reply buffer is not zeroed per-request.

If the transport's job handler fails synchronously, the request is completed
with an error, but job->result might not be updated and remains 0.

Later in bsg_transport_sg_io_fn(), the code relies on job->result to restrict
the copy length:

    if (job->result < 0) {
        /* we're only returning the result field in the reply */
        job->reply_len = sizeof(u32);
        ret = job->result;
    }

    if (job->reply_len && hdr->response) {
        int len = min(hdr->max_response_len, job->reply_len);

        if (copy_to_user(uptr64(hdr->response), job->reply, len))

Since job->result is 0, this check is bypassed and the full bset->reply_len
bytes of the uninitialized or recycled job->reply buffer will be copied to
user space. For UFS Advanced RPMB, this could leak up to 104 bytes of the
previous request's data.

Should the reply buffer be cleared when initializing the job, or should
job->result be explicitly set on all error paths?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013132708.1669415-1-stanleyjhu@google.com?part=1