On 2018/10/15 14:38, Ruiyu Ni wrote:
From: Hao Wu <hao.a...@intel.com>

This commit adds checks to make sure the UFS devices do not return more
data than the driver expected.

Cc: Ruiyu Ni <ruiyu...@intel.com>
Cc: Jiewen Yao <jiewen....@intel.com>
Cc: Star Zeng <star.z...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a...@intel.com>

Thanks for the patch.
Reviewed-by: Star Zeng <star.z...@intel.com>

Star

---
  MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c        | 19 ++++++++++++--
  .../Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c        | 30 +++++++++++++++++++---
  2 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c 
b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index 936f25da5e..9b87835ed8 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -857,6 +857,14 @@ UfsRwDeviceDesc (
      SwapLittleEndianToBigEndian ((UINT8*)&ReturnDataSize, sizeof (UINT16));
if (Read) {
+      //
+      // Make sure the hardware device does not return more data than expected.
+      //
+      if (ReturnDataSize > Packet.InTransferLength) {
+        Status = EFI_DEVICE_ERROR;
+        goto Exit;
+      }
+
        CopyMem (Packet.InDataBuffer, (QueryResp + 1), ReturnDataSize);
        Packet.InTransferLength = ReturnDataSize;
      } else {
@@ -1170,8 +1178,15 @@ UfsExecScsiCmds (
    SwapLittleEndianToBigEndian ((UINT8*)&SenseDataLen, sizeof (UINT16));
if ((Packet->SenseDataLength != 0) && (Packet->SenseData != NULL)) {
-    CopyMem (Packet->SenseData, Response->SenseData, SenseDataLen);
-    Packet->SenseDataLength = (UINT8)SenseDataLen;
+    //
+    // Make sure the hardware device does not return more data than expected.
+    //
+    if (SenseDataLen <= Packet->SenseDataLength) {
+      CopyMem (Packet->SenseData, Response->SenseData, SenseDataLen);
+      Packet->SenseDataLength = (UINT8)SenseDataLen;
+    } else {
+      Packet->SenseDataLength = 0;
+    }
    }
//
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c 
b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 5756f637fd..0238264878 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -833,6 +833,7 @@ UfsStopExecCmd (
    @param[in] QueryResp  Pointer to the query response.
@retval EFI_INVALID_PARAMETER Packet or QueryResp are empty or opcode is invalid.
+  @retval EFI_DEVICE_ERROR      Data returned from device is invalid.
    @retval EFI_SUCCESS           Data extracted.
**/
@@ -853,6 +854,13 @@ UfsGetReturnDataFromQueryResponse (
      case UtpQueryFuncOpcodeRdDesc:
        ReturnDataSize = QueryResp->Tsf.Length;
        SwapLittleEndianToBigEndian ((UINT8*)&ReturnDataSize, sizeof (UINT16));
+      //
+      // Make sure the hardware device does not return more data than expected.
+      //
+      if (ReturnDataSize > Packet->TransferLength) {
+        return EFI_DEVICE_ERROR;
+      }
+
        CopyMem (Packet->DataBuffer, (QueryResp + 1), ReturnDataSize);
        Packet->TransferLength = ReturnDataSize;
        break;
@@ -1469,8 +1477,15 @@ UfsExecScsiCmds (
    SwapLittleEndianToBigEndian ((UINT8*)&SenseDataLen, sizeof (UINT16));
if ((Packet->SenseDataLength != 0) && (Packet->SenseData != NULL)) {
-    CopyMem (Packet->SenseData, Response->SenseData, SenseDataLen);
-    Packet->SenseDataLength = (UINT8)SenseDataLen;
+    //
+    // Make sure the hardware device does not return more data than expected.
+    //
+    if (SenseDataLen <= Packet->SenseDataLength) {
+      CopyMem (Packet->SenseData, Response->SenseData, SenseDataLen);
+      Packet->SenseDataLength = (UINT8)SenseDataLen;
+    } else {
+      Packet->SenseDataLength = 0;
+    }
    }
//
@@ -2226,8 +2241,15 @@ ProcessAsyncTaskList (
          SwapLittleEndianToBigEndian ((UINT8*)&SenseDataLen, sizeof (UINT16));
if ((Packet->SenseDataLength != 0) && (Packet->SenseData != NULL)) {
-          CopyMem (Packet->SenseData, Response->SenseData, SenseDataLen);
-          Packet->SenseDataLength = (UINT8)SenseDataLen;
+          //
+          // Make sure the hardware device does not return more data than 
expected.
+          //
+          if (SenseDataLen <= Packet->SenseDataLength) {
+            CopyMem (Packet->SenseData, Response->SenseData, SenseDataLen);
+            Packet->SenseDataLength = (UINT8)SenseDataLen;
+          } else {
+            Packet->SenseDataLength = 0;
+          }
          }
//


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to