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

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

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?

Thanks,

Bart.