Re: [PATCH v2 1/5] scsi: ufs: bsg: Complete the job on error

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: ufs: bsg: heap out-of-bounds write/read in RPMB reply
--

commit 500adcab38ae23977180188472414e7c66ab1588
Author: Stanley Jhu <stanleyjhu@google.com>

scsi: ufs: bsg: Complete the job on error

This patch updates ufs_bsg_request to always complete the job via
bsg_job_done() and return 0, matching other BSG transports. It prevents
leaking the job's device reference and mapped scatterlists on error.
Positive status codes continue to return 0 from SG_IO with the reply header.

Cc: stable@vger.kernel.org

> diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
> index 7a66917b3fc0e..18c318d781224 100644
> --- a/drivers/ufs/core/ufs_bsg.c
> +++ b/drivers/ufs/core/ufs_bsg.c
> @@ -195,14 +195,12 @@ static int ufs_bsg_request(struct bsg_job *job)
>  	ufshcd_rpm_put_sync(hba);
>  	kfree(buff);
>  	bsg_reply->result = ret;
> -	/* complete the job here only if no error */
> -	if (ret == 0) {
> +	if (ret == 0)
>  		job->reply_len = rpmb ? sizeof(struct ufs_rpmb_reply) :
>  					sizeof(struct ufs_bsg_reply);

[Severity: High]
Does setting job->reply_len to sizeof(struct ufs_rpmb_reply) (which is 104
bytes) cause an out-of-bounds write and read?

The job->reply buffer is allocated with a fixed size of 96 bytes
(SCSI_SENSE_BUFFERSIZE) in bsg_init_rq().

During a UFS Advanced RPMB request, ufshcd_advanced_rpmb_req_handler() casts
the 96-byte job->reply buffer to a struct ufs_rpmb_reply and writes up to
104 bytes, overflowing the allocation by 8 bytes.

Later, bsg_complete_op() uses job->reply_len to copy the reply back to
userspace:

block/bsg-lib.c:bsg_complete_op() {
        ...
        int len = min(hdr->max_response_len, job->reply_len);
        if (copy_to_user(uptr64(hdr->response), job->reply, len))
        ...
}

Because job->reply_len is set to 104, this copies 8 bytes of adjacent
slab memory to userspace, which could lead to heap corruption and
memory disclosure.

> -		bsg_job_done(job, ret, bsg_reply->reply_payload_rcv_len);
> -	}
> +	bsg_job_done(job, ret, bsg_reply->reply_payload_rcv_len);
>  
> -	return ret;
> +	return 0;
>  }

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