From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web09.1091.1581618808293827799 for ; Thu, 13 Feb 2020 10:33:28 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=DB+dNNAL; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: philmd@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1581618807; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=yP+1H9kx+SQjJ1IL7QNOWIsknnwy6D2cT6B1wmD6u80=; b=DB+dNNALlNSqntoe/MB0uczHL2sbYWJDQG3QfSB4ZmhwsGnECH4UOhv2sO5SRub4us+f8q F48hhfMBIy/m3wNpmFq1qUwuXSko3/10Bm2phGP2WAjODCBSYRICZW8U5oBk8tmpyYbXo/ NCSsvF4kmGg4CCfzzHKptmOtwm3qIII= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-85-yWO-t25YNVO26jC650188Q-1; Thu, 13 Feb 2020 13:33:23 -0500 Received: by mail-wm1-f69.google.com with SMTP id n17so2346731wmk.1 for ; Thu, 13 Feb 2020 10:33:23 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=yP+1H9kx+SQjJ1IL7QNOWIsknnwy6D2cT6B1wmD6u80=; b=fAUC+ypWLNwC3UPBr1I4PkUaByca4ildm32W5Jg86HDR2wZDDR/CqVxYUTr1apHAcf r+idXbvzYLi/1wv3OnziDNI4cRKbPrrVE3ngogWDWzeI5i8Pgzcl4TnYwnbKjtXGRb60 8GG4/DzKE9AdpUwahX5Zq9gJetEauFSM7DdVZJLg9hSGM5NAEVbxVJzjZXt//TnN57vm fgM4Qd8GTdzDA/ws2vuUxZ21aYDz/4NLkAOoxWcW4Ye/CDwiSI+rEKsP8MNHc1OtjYVc 165SBV03eaMMRyRrfRSpBd/Z2Xzdf/pB1ew7871N1WdHLCFLsMsD4Q+UNYn59RFvEHoz VNFA== X-Gm-Message-State: APjAAAVPiJMGXMt1Xq0T13ekoBAfwcVlZ+xBQZHD03XjZ88BsmOIlM6f NBJamKXdrNrPZ6DqI5lTD8iNkVskPcFbQRldcinFMiA3fkSkY4C6URlyiHoieawI380IF5nJD++ jsYqh4Qug96m8Jw== X-Received: by 2002:a5d:614a:: with SMTP id y10mr8559642wrt.73.1581618801846; Thu, 13 Feb 2020 10:33:21 -0800 (PST) X-Google-Smtp-Source: APXvYqzA6OnVdAHjdib+dGXixEpMJF9wArgJHvUUe32xupKO78FgHT0Q+tkSO9W+t/j55Z65y4ghFQ== X-Received: by 2002:a5d:614a:: with SMTP id y10mr8559620wrt.73.1581618801560; Thu, 13 Feb 2020 10:33:21 -0800 (PST) Return-Path: Received: from [192.168.1.35] (78.red-88-21-202.staticip.rima-tde.net. [88.21.202.78]) by smtp.gmail.com with ESMTPSA id c15sm3934990wrt.1.2020.02.13.10.33.20 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 13 Feb 2020 10:33:21 -0800 (PST) Subject: Re: [RFC PATCH 1/1] MdeModulePkg/PiDxeS3BootScriptLib: Use SafeIntLib to avoid truncation To: devel@edk2.groups.io Cc: Jian J Wang , Hao A Wu , Eric Dong , Laszlo Ersek References: <20200213182935.26663-1-philmd@redhat.com> <20200213182935.26663-2-philmd@redhat.com> From: =?UTF-8?B?UGhpbGlwcGUgTWF0aGlldS1EYXVkw6k=?= Message-ID: Date: Thu, 13 Feb 2020 19:33:20 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: <20200213182935.26663-2-philmd@redhat.com> X-MC-Unique: yWO-t25YNVO26jC650188Q-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 2/13/20 7:29 PM, Philippe Mathieu-Daude wrote: > Math expressions written in terms of SafeIntLib function calls > are easily readable, making review trivial. Convert the truncation > checks added by commit 322ac05f8 to SafeIntLib calls. > > Cc: Jian J Wang > Cc: Hao A Wu > Cc: Eric Dong > Suggested-by: Laszlo Ersek > Signed-off-by: Philippe Mathieu-Daude > --- > .../DxeS3BootScriptLib.inf | 1 + > .../InternalBootScriptLib.h | 1 + > .../PiDxeS3BootScriptLib/BootScriptSave.c | 114 +++++++++++------- > 3 files changed, 73 insertions(+), 43 deletions(-) > > diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf > index 2b894c99da55..698039fe8e69 100644 > --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf > +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf > @@ -40,15 +40,16 @@ [Packages] > [LibraryClasses] > UefiBootServicesTableLib > BaseLib > BaseMemoryLib > TimerLib > DebugLib > PcdLib > UefiLib > SmbusLib > PciSegmentLib > IoLib > LockBoxLib > + SafeIntLib > > [Protocols] > gEfiSmmBase2ProtocolGuid ## SOMETIMES_CONSUMES > diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/InternalBootScriptLib.h b/MdeModulePkg/Library/PiDxeS3BootScriptLib/InternalBootScriptLib.h > index 9485994087d0..7513220c15ac 100644 > --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/InternalBootScriptLib.h > +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/InternalBootScriptLib.h > @@ -1,49 +1,50 @@ > /** @file > Support for S3 boot script lib. This file defined some internal macro and internal > data structure > > Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > #ifndef __INTERNAL_BOOT_SCRIPT_LIB__ > #define __INTERNAL_BOOT_SCRIPT_LIB__ > > #include > > #include > #include > #include > #include > #include > #include > > #include > > #include > #include > #include > #include > #include > #include > #include > #include > #include > #include > #include > +#include > > #include "BootScriptInternalFormat.h" > > #define MAX_IO_ADDRESS 0xFFFF > > // > // Macro to convert a UEFI PCI address + segment to a PCI Segment Library PCI address > // > #define PCI_ADDRESS_ENCODE(S, A) PCI_SEGMENT_LIB_ADDRESS( \ > S, \ > ((((UINTN)(A)) & 0xff000000) >> 24), \ > ((((UINTN)(A)) & 0x00ff0000) >> 16), \ > ((((UINTN)(A)) & 0xff00) >> 8), \ > ((RShiftU64 ((A), 32) & 0xfff) | ((A) & 0xff)) \ > ) > diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c > index 9315fc9f0188..d229263638fc 100644 > --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c > +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c > @@ -995,55 +995,60 @@ EFIAPI > S3BootScriptSaveIoWrite ( > IN S3_BOOT_SCRIPT_LIB_WIDTH Width, > IN UINT64 Address, > IN UINTN Count, > IN VOID *Buffer > ) > > { > + EFI_STATUS Status; > UINT8 Length; > UINT8 *Script; > UINT8 WidthInByte; > EFI_BOOT_SCRIPT_IO_WRITE ScriptIoWrite; > > - WidthInByte = (UINT8) (0x01 << (Width & 0x03)); > + Status = SafeUintnToUint8 (Count, &Length); > + if (EFI_ERROR (Status)) { > + return RETURN_OUT_OF_RESOURCES; > + } > + > + Status = SafeUint8Mult (Length, 0x01 << (Width & 0x03), &Length); > + if (EFI_ERROR (Status)) { > + return RETURN_OUT_OF_RESOURCES; > + } > > - // > - // Truncation check > - // > - if ((Count > MAX_UINT8) || > - (WidthInByte * Count > MAX_UINT8 - sizeof (EFI_BOOT_SCRIPT_IO_WRITE))) { > + Status = SafeUint8Add (Length, sizeof (EFI_BOOT_SCRIPT_IO_WRITE), &Length); > + if (EFI_ERROR (Status)) { > return RETURN_OUT_OF_RESOURCES; > } > - Length = (UINT8)(sizeof (EFI_BOOT_SCRIPT_IO_WRITE) + (WidthInByte * Count)); > > Script = S3BootScriptGetEntryAddAddress (Length); > if (Script == NULL) { > return RETURN_OUT_OF_RESOURCES; > } > // > // save script data > // > ScriptIoWrite.OpCode = EFI_BOOT_SCRIPT_IO_WRITE_OPCODE; > ScriptIoWrite.Length = Length; > ScriptIoWrite.Width = Width; > ScriptIoWrite.Address = Address; > ScriptIoWrite.Count = (UINT32) Count; > CopyMem ((VOID*)Script, (VOID*)&ScriptIoWrite, sizeof(EFI_BOOT_SCRIPT_IO_WRITE)); > CopyMem ((VOID*)(Script + sizeof (EFI_BOOT_SCRIPT_IO_WRITE)), Buffer, WidthInByte * Count); > > SyncBootScript (Script); > > return RETURN_SUCCESS; > } Oops wrong version (WidthInByte is uninitialized).