On 2019/10/21 21:06, Hannes Reinecke wrote: > On 10/21/19 3:49 AM, zhengbin (A) wrote: >> On 2019/10/18 21:43, Martin K. Petersen wrote: >>> Hannes, >>> >>>> The one thing which I patently don't like is the ambivalence between >>>> DRIVER_SENSE and scsi_sense_valid(). What shall we do if only _one_ >>>> of them is set? IE what would be the correct way of action if >>>> DRIVER_SENSE is not set, but we have a valid sense code? Or the other >>>> way around? >>> I agree, it's a mess. >>> >>> (Sorry, zhengbin, you opened a can of worms. This is some of our oldest >>> and most arcane code in SCSI) >>> >>>> But more important, from a quick glance not all drivers set the >>>> DRIVER_SENSE bit; so for things like hpsa or smartpqi the sense code is >>>> never evaluated after this patchset. >>> And yet we appear to have several code paths where sense evaluation is >>> contingent on DRIVER_SENSE. So no matter what, behavior might >>> change if we enforce consistent semantics. *sigh* >> So what should we do to prevent unit-value access of sshdr? >> > Where do you see it? > >From my reading, __scsi_execute() is clearing sshdr by way of > > __scsi_execute() > -> scsi_normalize_sense() > -> memset(sshdr) __scsi_execute req = blk_get_request(sdev->request_queue, data_direction == DMA_TO_DEVICE ? REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN, BLK_MQ_REQ_PREEMPT); if (IS_ERR(req)) return ret; -->just return rq = scsi_req(req); if (bufflen && blk_rq_map_kern(sdev->request_queue, req, buffer, bufflen, GFP_NOIO)) goto out; -->just goto out > > Cheers, > > Hannes
