Re: [PATCH v5 00/13] scsi: core: fix uninit-value access of variable sshdr

["zhengbin (A)" <zhengbin13@huawei.com>]

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