out of range LBA using sg_raw

Hi -

Need help to understand if below is something we should consider to be fixed in megaraid_sas driver or call as unreal exposure.

I have created slice VD of size 10GB (raid 1) using 2 drives. Each Physical Drive size is 256GB.

Last LBA of the VD and actual Physical disk associated with that VD is different. Actual Physical disk has larger range of LBA compare VD.

Below is readcap detail of VD0

# sg_readcap /dev/sdu
Read Capacity results:
   Last logical block address=20971519 (0x13fffff), Number of
blocks=20971520
   Logical block length=512 bytes
Hence:
   Device size: 10737418240 bytes, 10240.0 MiB, 10.74 GB

Using below sg_raw command, we should see "LBA out of range" sense. In CDB 0x28, pass LBA beyond last lba of VD 0x13fffff.

sg_raw -r 4k /dev/sdx 28 00 01 4f ff ff 00 00 08 00

It works if VD created behind MR controller does not support Fast Path Write.
In case of Fast Path Write, driver convert LBA of VD to underlying Physical disk and send IO direct to the physical disk. Since Physical disk has enough LBA range to respond, it will not send "LBA out of range sense".

Megaraid_Sas driver never validate range of LBA for VD as it assume to be validated by upper layer in scsi stack. Other sg_tool method like sg_dd, sg_write, dd etc has checks of LBA range and driver never receive out of range LBA.

What is a suggestion ? Shall I add check in megaraid_sas driver or it is not a valid scenario as "sg_raw" tool can send any type of command which does not require multiple sanity in driver.

Thanks, Kashyap

Hi Kashyap,

for SG_IO passthrough requests we can't validate command validity for commands as the block layer treats them as opaque. The SCSI device implementation needs to handle incorrect parameter to be robust.

For your fast path bypass the megaraid driver assumes part of the SCSI device implementation, so it will have to check for validity.

device

Thanks Chris. It is understood to have sanity in driver, but how critical such checks where SG_IO type interface send pass-through request. ? Are you suggesting as good to have sanity or very important as there may be a real-time exposure other than SG_IO interface ? I am confused over must or good to have check.
Also one more fault I can generate using below sg_raw command -

"sg_raw -r 32k /dev/sdx 28 00 01 4f ff ff 00 00 08 00"

Provide more scsi data length compare to actual SG buffer. Do you suggest such SG_IO interface vulnerability is good to be captured in driver.

I am just curious to know how badly we have to scrutinize each packet before sending to Fast Path as we are in IO path and recommend only important checks to be added.

Thanks, Kashyap

That's not a vulnerability of the SG I/O interface. A SCSI device has to set the residual count correctly if the SCSI data length does not match the size of the data buffer.

Bart.

driver.

set the

of the

Thanks Bart. I will pass this information to Broadcom firmware dev. May be a Tx/Rx (DMA) related code in MR (also for Fusion IT HBA) cannot handle due to some sanity checks are not passed.

SCSI _devices_ need to sanity check any input and fail commands instead of crashing or causing other problems. Normal SCSI HBA drivers don't need to do that as they don't interpret CDBs. Megaraid (and a few other raid drivers) are special in that they take on part of the device functionality and do interpret CDBs sometimes. In that case you'll need to do all that sanity checking and generate proper errors.

It would be nice to have come common helpers for this shared between everyone interpreting SCSI CBD (e.g. the SCSI target code, the NVMe SCSI emulation and the various RAID drivers).

request.

of

to

drivers)

sanity

Thanks Chris. I will continue on this and will come back with changes. Let me check with Broadcom internally and figure out all possible scenarios for megaraid_sas.

Thanks, Kashyap

Kashyap,

Kashyap> I am just curious to know how badly we have to scrutinize each Kashyap> packet before sending to Fast Path as we are in IO path and Kashyap> recommend only important checks to be added.

As Christoph pointed out, when the fast path is in use you assume the role of the SCSI device. And therefore it is your responsibility to ensure that the VD's capacity and other relevant constraints are being honored. Just like the MR firmware and any attached disks would.

It is a feature that there is no sanity checking in the sg interface. The intent is to be able to pass through commands directly to a device and have the device act upon them. Including fail them if they don't make any sense.

PS. I'm really no fan of the fast path. It's super messy to have the VD layout handled in two different places.

Martin K. Petersen	Oracle Linux Engineering

role of

the VD's

MR

Martin -

Agree on this point. I am planning to study all possible such sanity in driver for VD and not trying to fix one specific scenario as described here.
Do you think fix in this area is good for kernel-stable as well OR just keep in linux-next as it is not so severe considering real time exposure ? Trying to understand priority and severity of this issue.

and

any

Understood as sg_raw is not design to sanity check.

layout

Kashyap,

Kashyap> Agree on this point. I am planning to study all possible such Kashyap> sanity in driver for VD and not trying to fix one specific Kashyap> scenario as described here. Do you think fix in this area is Kashyap> good for kernel-stable as well OR just keep in linux-next as it Kashyap> is not so severe considering real time exposure ? Trying to Kashyap> understand priority and severity of this issue.

Since you're presumably only handling reads and writes in the fast path, I assume the sanity checking will be fairly simple and thus a candidate for stable.

Martin K. Petersen	Oracle Linux Engineering