Re: [PATCH 3/3] scsi: sd: Rework asynchronous resume support

[Bart Van Assche <bvanassche@acm.org>]

On 6/30/22 11:57, Bart Van Assche wrote:
> On 6/30/22 09:23, John Garry wrote:
>> On 28/06/2022 23:21, Bart Van Assche wrote:
>>> +/* A START command finished. May be called from interrupt context. */
>>> +static void sd_start_done(struct request *req, blk_status_t status)
>>> +{
>>> +    const struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
>>> +    struct scsi_disk *sdkp = scsi_disk(req->q->disk);
>>> +
>>> +    sdkp->start_result = scmd->result;
>>> +    WARN_ON_ONCE(scmd->sense_len > SCSI_SENSE_BUFFERSIZE);
>>
>> If scmd->sense_len > SCSI_SENSE_BUFFERSIZE, do you really want to go 
>> on to copy at sdkp->start_sense_buffer (which is of size 
>> SCSI_SENSE_BUFFERSIZE)? Won't that cause a corruption?
> 
> scsi_mq_init_request() allocates a buffer with size 
> SCSI_SENSE_BUFFERSIZE. SCSI LLDs copy sense data into that buffer. I am 
> not aware of any SCSI LLD that modifies the cmd->sense_buffer pointer. 
> So if scmd->sense_len would be larger than SCSI_SENSE_BUFFERSIZE that 
> either indicates that the LLD reported a sense length that is too large 
> or that it wrote outside the bounds of the sense buffer. Do we really 
> need to add a protection in the SCSI core against buggy LLDs?

A result of the above is that SCSI_SENSE_BUFFERSIZE bytes can be copied 
instead of scmd->sense_len. I will make that change.

Bart.