On 2019/10/17 10:45, Bart Van Assche wrote: > On 2019-10-11 20:25, zhengbin wrote: >> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c >> index 5447738..d5e29c5 100644 >> --- a/drivers/scsi/scsi_lib.c >> +++ b/drivers/scsi/scsi_lib.c >> @@ -255,6 +255,13 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, >> struct scsi_request *rq; >> int ret = DRIVER_ERROR << 24; >> >> + /* >> + * Zero-initialize sshdr for those callers that check the *sshdr >> + * contents even if no sense data is available. >> + */ >> + if (sshdr) >> + memset(sshdr, 0, sizeof(struct scsi_sense_hdr)); >> + >> req = blk_get_request(sdev->request_queue, >> data_direction == DMA_TO_DEVICE ? >> REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN, BLK_MQ_REQ_PREEMPT); > Although I don't have a strong opinion about this, I'm still wondering > whether 'sshdr' should be initialized in __scsi_execute() or by its caller. @jejb, @martin, any suggestion? > >> diff --git a/drivers/scsi/sr_ioctl.c b/drivers/scsi/sr_ioctl.c >> index ffcf902..335cfdd 100644 >> --- a/drivers/scsi/sr_ioctl.c >> +++ b/drivers/scsi/sr_ioctl.c >> @@ -206,6 +206,11 @@ int sr_do_ioctl(Scsi_CD *cd, struct packet_command *cgc) >> >> /* Minimal error checking. Ignore cases we know about, and report the rest. */ >> if (driver_byte(result) != 0) { >> + if (!scsi_sense_valid(sshdr)) { >> + err = -EIO; >> + goto out; >> + } >> + >> switch (sshdr->sense_key) { >> case UNIT_ATTENTION: >> SDev->changed = 1; > Shouldn't this be a separate patch? OK, will send a separate patch > > Thanks, > > Bart. > > . >