The "SCSI Block Commands - 2" (SBC-2) standard defines bits [7:5] of the
CDB byte 1 as Reserved, for the READ and WRITE commands.

The updated "SCSI Block Commands - 3" (SBC-3) standard defines the same
bitfield as RDPROTECT and WRPROTECT, respectively.

After reviewing the above standards, and the following commits:
- SVN r8331 (git 676e2a32),
- SVN r8334 (git 6b3ecf5c),
we've determined that UefiScsiLib is incorrect in encoding the LUN in this
bitfield for the READ and WRITE commands.

Encoding a nonzero LUN there creates unintended RDPROTECT and WRPROTECT
values, which the recipient device is required to reject if it does not
support protection information, with CHECK CONDITION, ILLEGAL REQUEST,
INVALID FIELD IN CDB:

  ScsiDiskRead10: Check Condition happened!
  ScsiDisk: Sense Key = 0x5 ASC = 0x24!
  ScsiDiskRead10: Check Condition happened!
  ScsiDisk: Sense Key = 0x5 ASC = 0x24!
  ScsiDiskRead10: Check Condition happened!
  ScsiDisk: Sense Key = 0x5 ASC = 0x24!
  ScsiDiskRead10: Check Condition happened!
  ScsiDisk: Sense Key = 0x5 ASC = 0x24!
  FatOpenDevice: read of part_lba failed Device Error

In practice this flaw breaks UefiScsiLib minimally on SCSI disks with
nonzero LUNs that are emulated by QEMU (after QEMU commit 96bdbbab, part
of v1.2.0).

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1166971
Reported-by: Cole Robinson <crobi...@redhat.com>
Suggested-by: Paolo Bonzini <pbonz...@redhat.com>
Suggested-by: Feng Tian <feng.t...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
---

Notes:
    v4:
    - unchanged, just formatted with more context lines
    
    v3:
    - the Lun, Target, TargetArray local variables and the
      ScsiIo->GetDeviceLocation() call are not needed any longer, remove
      them (reverting a larger part of SVN r8334 this way)
    
    v2:
    - permanently remove the LUN encoding [Paolo, Feng]
    
    v1:
    - make encoding dependent on a new Feature PCD

 MdePkg/Library/UefiScsiLib/UefiScsiLib.c | 28 ----------------------------
 1 file changed, 28 deletions(-)

diff --git a/MdePkg/Library/UefiScsiLib/UefiScsiLib.c 
b/MdePkg/Library/UefiScsiLib/UefiScsiLib.c
index bd838c4..1dbe874 100644
--- a/MdePkg/Library/UefiScsiLib/UefiScsiLib.c
+++ b/MdePkg/Library/UefiScsiLib/UefiScsiLib.c
@@ -895,50 +895,43 @@ ScsiRead10Command (
   IN OUT VOID                  *SenseData,   OPTIONAL
   IN OUT UINT8                 *SenseDataLength,
      OUT UINT8                 *HostAdapterStatus,
      OUT UINT8                 *TargetStatus,
   IN OUT VOID                  *DataBuffer,  OPTIONAL
   IN OUT UINT32                *DataLength,
   IN     UINT32                StartLba,
   IN     UINT32                SectorSize
   )
 {
   EFI_SCSI_IO_SCSI_REQUEST_PACKET CommandPacket;
-  UINT64                          Lun;
-  UINT8                           *Target;
-  UINT8                           TargetArray[EFI_SCSI_TARGET_MAX_BYTES];
   EFI_STATUS                      Status;
   UINT8                           Cdb[EFI_SCSI_OP_LENGTH_TEN];
 
   ASSERT (SenseDataLength != NULL);
   ASSERT (HostAdapterStatus != NULL);
   ASSERT (TargetStatus != NULL);
   ASSERT (DataLength != NULL);
   ASSERT (ScsiIo != NULL);
 
   ZeroMem (&CommandPacket, sizeof (EFI_SCSI_IO_SCSI_REQUEST_PACKET));
   ZeroMem (Cdb, EFI_SCSI_OP_LENGTH_TEN);
 
   CommandPacket.Timeout         = Timeout;
   CommandPacket.InDataBuffer    = DataBuffer;
   CommandPacket.SenseData       = SenseData;
   CommandPacket.InTransferLength= *DataLength;
   CommandPacket.Cdb             = Cdb;
   //
   // Fill Cdb for Read (10) Command
   //
-  Target = &TargetArray[0];
-  ScsiIo->GetDeviceLocation (ScsiIo, &Target, &Lun);
-
   Cdb[0]                        = EFI_SCSI_OP_READ10;
-  Cdb[1]                        = (UINT8) (LShiftU64 (Lun, 5) & 
EFI_SCSI_LOGICAL_UNIT_NUMBER_MASK);
   WriteUnaligned32 ((UINT32 *)&Cdb[2], SwapBytes32 (StartLba));
   WriteUnaligned16 ((UINT16 *)&Cdb[7], SwapBytes16 ((UINT16) SectorSize));
 
   CommandPacket.CdbLength       = EFI_SCSI_OP_LENGTH_TEN;
   CommandPacket.DataDirection   = EFI_SCSI_DATA_IN;
   CommandPacket.SenseDataLength = *SenseDataLength;
 
   Status                        = ScsiIo->ExecuteScsiCommand (ScsiIo, 
&CommandPacket, NULL);
 
   *HostAdapterStatus            = CommandPacket.HostAdapterStatus;
   *TargetStatus                 = CommandPacket.TargetStatus;
@@ -993,50 +986,43 @@ ScsiWrite10Command (
   IN OUT VOID                  *SenseData,   OPTIONAL
   IN OUT UINT8                 *SenseDataLength,
      OUT UINT8                 *HostAdapterStatus,
      OUT UINT8                 *TargetStatus,
   IN OUT VOID                  *DataBuffer,  OPTIONAL
   IN OUT UINT32                *DataLength,
   IN     UINT32                StartLba,
   IN     UINT32                SectorSize
   )
 {
   EFI_SCSI_IO_SCSI_REQUEST_PACKET CommandPacket;
-  UINT64                          Lun;
-  UINT8                           *Target;
-  UINT8                           TargetArray[EFI_SCSI_TARGET_MAX_BYTES];
   EFI_STATUS                      Status;
   UINT8                           Cdb[EFI_SCSI_OP_LENGTH_TEN];
 
   ASSERT (SenseDataLength != NULL);
   ASSERT (HostAdapterStatus != NULL);
   ASSERT (TargetStatus != NULL);
   ASSERT (DataLength != NULL);
   ASSERT (ScsiIo != NULL);
 
   ZeroMem (&CommandPacket, sizeof (EFI_SCSI_IO_SCSI_REQUEST_PACKET));
   ZeroMem (Cdb, EFI_SCSI_OP_LENGTH_TEN);
 
   CommandPacket.Timeout         = Timeout;
   CommandPacket.OutDataBuffer    = DataBuffer;
   CommandPacket.SenseData       = SenseData;
   CommandPacket.OutTransferLength= *DataLength;
   CommandPacket.Cdb             = Cdb;
   //
   // Fill Cdb for Write (10) Command
   //
-  Target = &TargetArray[0];
-  ScsiIo->GetDeviceLocation (ScsiIo, &Target, &Lun);
-
   Cdb[0]                        = EFI_SCSI_OP_WRITE10;
-  Cdb[1]                        = (UINT8) (LShiftU64 (Lun, 5) & 
EFI_SCSI_LOGICAL_UNIT_NUMBER_MASK);
   WriteUnaligned32 ((UINT32 *)&Cdb[2], SwapBytes32 (StartLba));
   WriteUnaligned16 ((UINT16 *)&Cdb[7], SwapBytes16 ((UINT16) SectorSize));
 
   CommandPacket.CdbLength       = EFI_SCSI_OP_LENGTH_TEN;
   CommandPacket.DataDirection   = EFI_SCSI_DATA_OUT;
   CommandPacket.SenseDataLength = *SenseDataLength;
 
   Status                        = ScsiIo->ExecuteScsiCommand (ScsiIo, 
&CommandPacket, NULL);
 
   *HostAdapterStatus            = CommandPacket.HostAdapterStatus;
   *TargetStatus                 = CommandPacket.TargetStatus;
@@ -1091,50 +1077,43 @@ ScsiRead16Command (
   IN OUT VOID                  *SenseData,   OPTIONAL
   IN OUT UINT8                 *SenseDataLength,
      OUT UINT8                 *HostAdapterStatus,
      OUT UINT8                 *TargetStatus,
   IN OUT VOID                  *DataBuffer,  OPTIONAL
   IN OUT UINT32                *DataLength,
   IN     UINT64                StartLba,
   IN     UINT32                SectorSize
   )
 {
   EFI_SCSI_IO_SCSI_REQUEST_PACKET CommandPacket;
-  UINT64                          Lun;
-  UINT8                           *Target;
-  UINT8                           TargetArray[EFI_SCSI_TARGET_MAX_BYTES];
   EFI_STATUS                      Status;
   UINT8                           Cdb[EFI_SCSI_OP_LENGTH_SIXTEEN];
 
   ASSERT (SenseDataLength != NULL);
   ASSERT (HostAdapterStatus != NULL);
   ASSERT (TargetStatus != NULL);
   ASSERT (DataLength != NULL);
   ASSERT (ScsiIo != NULL);
 
   ZeroMem (&CommandPacket, sizeof (EFI_SCSI_IO_SCSI_REQUEST_PACKET));
   ZeroMem (Cdb, EFI_SCSI_OP_LENGTH_SIXTEEN);
 
   CommandPacket.Timeout          = Timeout;
   CommandPacket.InDataBuffer     = DataBuffer;
   CommandPacket.SenseData        = SenseData;
   CommandPacket.InTransferLength = *DataLength;
   CommandPacket.Cdb              = Cdb;
   //
   // Fill Cdb for Read (16) Command
   //
-  Target = &TargetArray[0];
-  ScsiIo->GetDeviceLocation (ScsiIo, &Target, &Lun);
-
   Cdb[0]                        = EFI_SCSI_OP_READ16;
-  Cdb[1]                        = (UINT8) (LShiftU64 (Lun, 5) & 
EFI_SCSI_LOGICAL_UNIT_NUMBER_MASK);
   WriteUnaligned64 ((UINT64 *)&Cdb[2], SwapBytes64 (StartLba));
   WriteUnaligned32 ((UINT32 *)&Cdb[10], SwapBytes32 (SectorSize));
 
   CommandPacket.CdbLength       = EFI_SCSI_OP_LENGTH_SIXTEEN;
   CommandPacket.DataDirection   = EFI_SCSI_DATA_IN;
   CommandPacket.SenseDataLength = *SenseDataLength;
 
   Status                        = ScsiIo->ExecuteScsiCommand (ScsiIo, 
&CommandPacket, NULL);
 
   *HostAdapterStatus            = CommandPacket.HostAdapterStatus;
   *TargetStatus                 = CommandPacket.TargetStatus;
@@ -1189,50 +1168,43 @@ ScsiWrite16Command (
   IN OUT VOID                  *SenseData,   OPTIONAL
   IN OUT UINT8                 *SenseDataLength,
      OUT UINT8                 *HostAdapterStatus,
      OUT UINT8                 *TargetStatus,
   IN OUT VOID                  *DataBuffer,  OPTIONAL
   IN OUT UINT32                *DataLength,
   IN     UINT64                StartLba,
   IN     UINT32                SectorSize
   )
 {
   EFI_SCSI_IO_SCSI_REQUEST_PACKET CommandPacket;
-  UINT64                          Lun;
-  UINT8                           *Target;
-  UINT8                           TargetArray[EFI_SCSI_TARGET_MAX_BYTES];
   EFI_STATUS                      Status;
   UINT8                           Cdb[EFI_SCSI_OP_LENGTH_SIXTEEN];
 
   ASSERT (SenseDataLength != NULL);
   ASSERT (HostAdapterStatus != NULL);
   ASSERT (TargetStatus != NULL);
   ASSERT (DataLength != NULL);
   ASSERT (ScsiIo != NULL);
 
   ZeroMem (&CommandPacket, sizeof (EFI_SCSI_IO_SCSI_REQUEST_PACKET));
   ZeroMem (Cdb, EFI_SCSI_OP_LENGTH_SIXTEEN);
 
   CommandPacket.Timeout           = Timeout;
   CommandPacket.OutDataBuffer     = DataBuffer;
   CommandPacket.SenseData         = SenseData;
   CommandPacket.OutTransferLength = *DataLength;
   CommandPacket.Cdb               = Cdb;
   //
   // Fill Cdb for Write (16) Command
   //
-  Target = &TargetArray[0];
-  ScsiIo->GetDeviceLocation (ScsiIo, &Target, &Lun);
-
   Cdb[0]                        = EFI_SCSI_OP_WRITE16;
-  Cdb[1]                        = (UINT8) (LShiftU64 (Lun, 5) & 
EFI_SCSI_LOGICAL_UNIT_NUMBER_MASK);
   WriteUnaligned64 ((UINT64 *)&Cdb[2], SwapBytes64 (StartLba));
   WriteUnaligned32 ((UINT32 *)&Cdb[10], SwapBytes32 (SectorSize));
 
   CommandPacket.CdbLength       = EFI_SCSI_OP_LENGTH_SIXTEEN;
   CommandPacket.DataDirection   = EFI_SCSI_DATA_OUT;
   CommandPacket.SenseDataLength = *SenseDataLength;
 
   Status                        = ScsiIo->ExecuteScsiCommand (ScsiIo, 
&CommandPacket, NULL);
 
   *HostAdapterStatus            = CommandPacket.HostAdapterStatus;
   *TargetStatus                 = CommandPacket.TargetStatus;
-- 
1.8.3.1



------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to