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.31; helo=mga06.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (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 AB0BD211799C5 for ; Mon, 15 Oct 2018 22:42:27 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Oct 2018 22:42:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,387,1534834800"; d="scan'208";a="81760156" Received: from ray-dev.ccr.corp.intel.com ([10.239.9.11]) by orsmga008.jf.intel.com with ESMTP; 15 Oct 2018 22:42:26 -0700 From: Ruiyu Ni To: edk2-devel@lists.01.org Cc: Star Zeng , Steven Shi Date: Tue, 16 Oct 2018 13:43:25 +0800 Message-Id: <20181016054335.47460-3-ruiyu.ni@intel.com> X-Mailer: git-send-email 2.16.1.windows.1 In-Reply-To: <20181016054335.47460-1-ruiyu.ni@intel.com> References: <20181016054335.47460-1-ruiyu.ni@intel.com> Subject: [PATCH v2 02/12] 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: Tue, 16 Oct 2018 05:42:27 -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 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; } -- 2.16.1.windows.1