From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.100; helo=mga07.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id F17632116823B for ; Sun, 14 Oct 2018 23:37:32 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Oct 2018 23:37:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,383,1534834800"; d="scan'208";a="78046731" Received: from ray-dev.ccr.corp.intel.com ([10.239.9.11]) by fmsmga007.fm.intel.com with ESMTP; 14 Oct 2018 23:37:31 -0700 From: Ruiyu Ni To: edk2-devel@lists.01.org Cc: Star Zeng , Steven Shi Date: Mon, 15 Oct 2018 14:38:24 +0800 Message-Id: <20181015063833.61304-3-ruiyu.ni@intel.com> X-Mailer: git-send-email 2.16.1.windows.1 In-Reply-To: <20181015063833.61304-1-ruiyu.ni@intel.com> References: <20181015063833.61304-1-ruiyu.ni@intel.com> Subject: [PATCH 02/11] MdeModulePkg/UsbMass: Fix integer overflow when BlockSize is 1 X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Oct 2018 06:37:33 -0000 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 --- .../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 581571ab45..37fbeedbeb 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; } -- 2.16.1.windows.1