Re: [PATCH v1] scsi: ufs: scsi_get_lba error fix by check cmd opcode

[Peter Wang <peter.wang@mediatek.com>]

Hi Bart,


When scsi scan in scsi_probe_lun function, there have much INQUIRY(0x12) 
command
with sector_size is 0.
unsigned int shift will get 4294967286 (signed -10) and an sector_t type 
is 64 bit.
Shift 64bit right 4294967286 will have ubsan error because ubsan think
shift number should be wrong and return 0 always.
BTW, we only need the lba information when read/write/unmap. Other 
command such
as INQUIRY is useless.

static inline sector_t scsi_get_lba(struct scsi_cmnd *scmd)
{
     unsigned int shift = ilog2(scmd->device->sector_size) - 
SECTOR_SHIFT; <=  shift is 4294967286 (-1-9=-10)

     return blk_rq_pos(scsi_cmd_to_rq(scmd)) >> shift;  <= sector_t type 
 >> 4294967286 will always get 0.
}


On 3/8/22 1:52 AM, Bart Van Assche wrote:
>
> Hmm ... how can it happen that sector_size has not been set? I think 
> that can only happen for LUNs of type SCSI DISK if sd_read_capacity() 
> fails? If sd_read_capacity() fails I think the sd driver is expected 
> to set the capacity to zero?
>
> rq->__sector == -1 for flush requests and the type of that member 
> (sector_t) is unsigned. I think that it is allowed for a shift left of 
> an unsigned type to overflow. From the C standard: "The result of E1 
> << E2 is E1 left-shifted E2 bit positions; vacated bits are filled with
> zeros. If E1 has an unsigned type, the value of the result is E1 × 2E2 
> , reduced modulo one more than the maximum value representable in the 
> result type."
>
> Thanks,
>
> Bart.