From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.85.221.68, mailfrom: philmd@redhat.com) Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by groups.io with SMTP; Mon, 15 Apr 2019 10:23:32 -0700 Received: by mail-wr1-f68.google.com with SMTP id j9so22982664wrn.6 for ; Mon, 15 Apr 2019 10:23:31 -0700 (PDT) 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:openpgp:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=wi7lGZ+Ed+otv3a/i6sUHfdJjJLJXVzRipG0k426yAU=; b=iId1jeFgtns5bwZWSUgKjfaNo1rHKjSODdStHXOQrFb7xlLv5CjjHeBJlrwlaQXipl xWhkfZrcsiIjNPcJuPxj2djiLqJQafAgHP8LM3WsWK1rZQ1e+fWyvFDNH/rdyuKgnsbX yUBPCyiBbKqjbVN2Yd516mAVDBO3NdmDIdSx9YKFb6m6JpIRXz4548O4WoCa2vRpU0FI 42wCl2zLrv+7qF0Y+Iya+iafBpoF6tz/QWzu3ZI/IedGE0LnvmXsBK6R8rIJYXDtBVpv WNU2CohV40yg6mUS7gu0HPyp7qghcEM5Oo8y11DlBA2ZNQt8to1uIun517WgBBtTMG1f SMPA== X-Gm-Message-State: APjAAAVOOR7vMCya2DeEp46R+SQ4ER1+tT8yl1PNkKEyY4Y9hCL+uqHh KU9qouLzl2A80mWtTn5VyYOLTA== X-Google-Smtp-Source: APXvYqzSuuGehXumLlT4oDSw3fkBDJNXsZMu+R/IjlRcOGqM+7wL4MhotX43BWsubOJ+zJmGhA9dPA== X-Received: by 2002:a5d:6a0b:: with SMTP id m11mr46945997wru.290.1555349010482; Mon, 15 Apr 2019 10:23:30 -0700 (PDT) Return-Path: Received: from [10.32.224.40] (red-hat-inc.vlan560.asr1.mad1.gblx.net. [159.63.51.90]) by smtp.gmail.com with ESMTPSA id a8sm20965352wmf.33.2019.04.15.10.23.29 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Mon, 15 Apr 2019 10:23:29 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH 04/10] MdePkg/PiFirmwareFile: fix undefined behavior in FFS_FILE_SIZE To: devel@edk2.groups.io, lersek@redhat.com Cc: Liming Gao , Michael D Kinney References: <20190412233128.4756-1-lersek@redhat.com> <20190412233128.4756-5-lersek@redhat.com> From: =?UTF-8?B?UGhpbGlwcGUgTWF0aGlldS1EYXVkw6k=?= Openpgp: id=89C1E78F601EE86C867495CBA2A3FD6EDEADC0DE; url=http://pgp.mit.edu/pks/lookup?op=get&search=0xA2A3FD6EDEADC0DE Message-ID: <1c1717c3-9239-70ff-5678-0a1be5caa92f@redhat.com> Date: Mon, 15 Apr 2019 19:23:28 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <20190412233128.4756-5-lersek@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 4/13/19 1:31 AM, Laszlo Ersek wrote: > Accessing "EFI_FFS_FILE_HEADER.Size", which is of type UINT8[3], through a > (UINT32*), is undefined behavior. Fix it by accessing the array elements > individually. > > (We can't use a union here, unfortunately, as easily as with > "EFI_COMMON_SECTION_HEADER", given the fields in "EFI_FFS_FILE_HEADER".) > > Cc: Liming Gao > Cc: Michael D Kinney > Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710 > Signed-off-by: Laszlo Ersek > --- > MdePkg/Include/Pi/PiFirmwareFile.h | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/MdePkg/Include/Pi/PiFirmwareFile.h b/MdePkg/Include/Pi/PiFirmwareFile.h > index 4fce8298d1c0..0668f3fa9af4 100644 > --- a/MdePkg/Include/Pi/PiFirmwareFile.h > +++ b/MdePkg/Include/Pi/PiFirmwareFile.h > @@ -174,18 +174,26 @@ typedef struct { > /// If FFS_ATTRIB_LARGE_FILE is not set then EFI_FFS_FILE_HEADER is used. > /// > UINT64 ExtendedSize; > } EFI_FFS_FILE_HEADER2; > > #define IS_FFS_FILE2(FfsFileHeaderPtr) \ > (((((EFI_FFS_FILE_HEADER *) (UINTN) FfsFileHeaderPtr)->Attributes) & FFS_ATTRIB_LARGE_FILE) == FFS_ATTRIB_LARGE_FILE) > > +#define FFS_FILE_SIZE_ARRAY(FfsFileHeaderPtr) \ > + (((EFI_FFS_FILE_HEADER *) (UINTN) (FfsFileHeaderPtr))->Size) > + > +#define FFS_FILE_SIZE_ELEMENT(FfsFileHeaderPtr, Index) \ > + ((UINT32) FFS_FILE_SIZE_ARRAY (FfsFileHeaderPtr)[(Index)]) > + > #define FFS_FILE_SIZE(FfsFileHeaderPtr) \ > - ((UINT32) (*((UINT32 *) ((EFI_FFS_FILE_HEADER *) (UINTN) FfsFileHeaderPtr)->Size) & 0x00ffffff)) > + ((FFS_FILE_SIZE_ELEMENT ((FfsFileHeaderPtr), 0) << 0) | \ > + (FFS_FILE_SIZE_ELEMENT ((FfsFileHeaderPtr), 1) << 8) | \ > + (FFS_FILE_SIZE_ELEMENT ((FfsFileHeaderPtr), 2) << 16)) > > #define FFS_FILE2_SIZE(FfsFileHeaderPtr) \ > ((UINT32) (((EFI_FFS_FILE_HEADER2 *) (UINTN) FfsFileHeaderPtr)->ExtendedSize)) > > typedef UINT8 EFI_SECTION_TYPE; > > /// > /// Pseudo type. It is used as a wild card when retrieving sections. > Reviewed-by: Philippe Mathieu-Daude