On Wed, 2019-01-16 at 10:57 -0500, Douglas Gilbert wrote:
> The block layer assumes scsi_request:sense is always a valid
> pointer. This is set up once in scsi_mq_init_request() and the
> containing scsi_cmnd object is used often, being re-initialized
> by scsi_init_command(). That works unless some code re-purposes
> part of the scsi_cmnd object for something else. And that is
> what bidi handling does in scsi_mq_prep_fn(). The result is an
> oops at some later time when the partly overwritten object is
> re-used. The overwrite is from d285203cf647d but 'git blame'
> does not show removed code, so that commit may not be the
> culprit.
>
> Signed-off-by: Douglas Gilbert <dgilb...@interlog.com>
> ---
>
> This was found while injecting errors (thus generating sense data)
> into a sequence of bidi commands. At some later time the block
> layer blew up with a scsi_request::sense NULL dereference in
> sg_rq_end_io(). Without testing I'm confident the bsg driver,
> the osd ULD and exofs are exposed to this bug.
>
> drivers/scsi/scsi_lib.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index b13cc9288ba0..71259bd4040a 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1175,6 +1175,7 @@ void scsi_init_command(struct scsi_device *dev, struct
> scsi_cmnd *cmd)
>
> cmd->device = dev;
> cmd->sense_buffer = buf;
> + cmd->req.sense = buf;
> cmd->prot_sdb = prot;
> cmd->flags = flags;
> INIT_DELAYED_WORK(&cmd->abort_work, scmd_eh_abort_handler);
Hi Doug,
The description of this patch does not look correct to me. scsi_init_command()
does not overwrite the sense pointer. From the body of that function:
/* zero out the cmd, except for the embedded scsi_request */
memset((char *)cmd + sizeof(cmd->req), 0,
sizeof(*cmd) - sizeof(cmd->req) + dev->host->hostt->cmd_size);
It is not clear to me which code overwrites the sense pointer. I think that
needs to be figured out before discussion of this patch can continue.
Thanks,
Bart.
