From 4f8b2f9d727d0035a2ca8d7371629418684bf80f Mon Sep 17 00:00:00 2001 From: Ruiyu Ni Date: Tue, 11 Sep 2018 18:13:05 +0800 Subject: [PATCH] MdeModulePkg/UsbMass: Fix integer overflow when BlockSize is 1 UsbBootReadWriteBlocks() and UsbBootReadWriteBlocks16() use a UINT16 local variable to hold the value of USB_BOOT_MAX_CARRY_SIZE (=0x10000) / BlockSize. When BlockSize is 1, the UINT16 local variable is set to 0x10000 but the high-16 bits are truncated resulting the final value be 0. It causes the while-loop in the two functions accesses 0 block in each loop, resulting the loop never ends. The patch fixes the two functions to make sure no integer overflow happens. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Cc: Star Zeng Cc: Steven Shi Reviewed-by: Star Zeng --- .../Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c index 07a376440c..0b35cbacf0 100644 --- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c +++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c @@ -815,14 +815,14 @@ UsbBootReadWriteBlocks ( { USB_BOOT_READ_WRITE_10_CMD Cmd; EFI_STATUS Status; - UINT16 Count; - UINT16 CountMax; + UINT32 Count; + UINT32 CountMax; UINT32 BlockSize; UINT32 ByteSize; UINT32 Timeout; BlockSize = UsbMass->BlockIoMedia.BlockSize; - CountMax = (UINT16)(USB_BOOT_MAX_CARRY_SIZE / BlockSize); + CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize; Status = EFI_SUCCESS; while (TotalBlock > 0) { @@ -831,8 +831,9 @@ UsbBootReadWriteBlocks ( // on the device. We must split the total block because the READ10 // command only has 16 bit transfer length (in the unit of block). // - Count = (UINT16)((TotalBlock < CountMax) ? TotalBlock : CountMax); - ByteSize = (UINT32)Count * BlockSize; + Count = (UINT32)MIN (TotalBlock, CountMax); + Count = MIN (MAX_UINT16, Count); + ByteSize = Count * BlockSize; // // USB command's upper limit timeout is 5s. [USB2.0-9.2.6.1] @@ -847,7 +848,7 @@ UsbBootReadWriteBlocks ( Cmd.OpCode = Write ? USB_BOOT_WRITE10_OPCODE : USB_BOOT_READ10_OPCODE; Cmd.Lun = (UINT8) (USB_BOOT_LUN (UsbMass->Lun)); WriteUnaligned32 ((UINT32 *) Cmd.Lba, SwapBytes32 (Lba)); - WriteUnaligned16 ((UINT16 *) Cmd.TransferLen, SwapBytes16 (Count)); + WriteUnaligned16 ((UINT16 *) Cmd.TransferLen, SwapBytes16 ((UINT16)Count)); Status = UsbBootExecCmdWithRetry ( UsbMass, @@ -867,7 +868,7 @@ UsbBootReadWriteBlocks ( Lba, Count )); Lba += Count; - Buffer += Count * BlockSize; + Buffer += ByteSize; TotalBlock -= Count; } @@ -897,22 +898,22 @@ UsbBootReadWriteBlocks16 ( { UINT8 Cmd[16]; EFI_STATUS Status; - UINT16 Count; - UINT16 CountMax; + UINT32 Count; + UINT32 CountMax; UINT32 BlockSize; UINT32 ByteSize; UINT32 Timeout; BlockSize = UsbMass->BlockIoMedia.BlockSize; - CountMax = (UINT16)(USB_BOOT_MAX_CARRY_SIZE / BlockSize); + CountMax = USB_BOOT_MAX_CARRY_SIZE / BlockSize; Status = EFI_SUCCESS; while (TotalBlock > 0) { // // Split the total blocks into smaller pieces. // - Count = (UINT16)((TotalBlock < CountMax) ? TotalBlock : CountMax); - ByteSize = (UINT32)Count * BlockSize; + Count = (UINT32)MIN (TotalBlock, CountMax); + ByteSize = Count * BlockSize; // // USB command's upper limit timeout is 5s. [USB2.0-9.2.6.1] @@ -947,7 +948,7 @@ UsbBootReadWriteBlocks16 ( Lba, Count )); Lba += Count; - Buffer += Count * BlockSize; + Buffer += ByteSize; TotalBlock -= Count; }