From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web11.36257.1601908399446018147 for ; Mon, 05 Oct 2020 07:33:19 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0EA98106F; Mon, 5 Oct 2020 07:33:19 -0700 (PDT) Received: from [192.168.1.81] (unknown [10.37.8.98]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 5421B3F70D; Mon, 5 Oct 2020 07:33:17 -0700 (PDT) Subject: Re: [PATCH v2 1/1] MdePkg/BaseLib: Fix invalid memory access in AArch64 SetJump/LongJump To: Jan Bobek , devel@edk2.groups.io, Michael D Kinney , Liming Gao Cc: Leif Lindholm , Zhiguang Liu , Jeff Brasen , Ashish Singhal References: <20201001161507.48710-1-jbobek@nvidia.com> <20201001161507.48710-2-jbobek@nvidia.com> From: "Ard Biesheuvel" Message-ID: <91988ebd-49cd-4a47-1c95-0e9e970d819d@arm.com> Date: Mon, 5 Oct 2020 16:33:15 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20201001161507.48710-2-jbobek@nvidia.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 10/1/20 6:15 PM, Jan Bobek wrote: > Correct the memory offsets used in REG_ONE/REG_PAIR macros to > synchronize them with definition of the BASE_LIBRARY_JUMP_BUFFER > structure on AArch64. > > The REG_ONE macro declares only a single 64-bit register be > read/written; however, the subsequent offset is 16 bytes larger, > creating an unused memory gap in the middle of the structure and > causing SetJump/LongJump functions to read/write 8 bytes of memory > past the end of the jump buffer struct. > > Signed-off-by: Jan Bobek Thanks Jan, Reviewed-by: Ard Biesheuvel Liming, Michael: any concerns? Thanks, > --- > MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.S | 8 ++++---- > MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.asm | 8 ++++---- > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.S b/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.S > index 72cea259e913..deefdf526b95 100644 > --- a/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.S > +++ b/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.S > @@ -20,10 +20,10 @@ GCC_ASM_EXPORT(InternalLongJump) > REG_ONE (x16, 96) /*IP0*/ > > #define FPR_LAYOUT \ > - REG_PAIR ( d8, d9, 112); \ > - REG_PAIR (d10, d11, 128); \ > - REG_PAIR (d12, d13, 144); \ > - REG_PAIR (d14, d15, 160); > + REG_PAIR ( d8, d9, 104); \ > + REG_PAIR (d10, d11, 120); \ > + REG_PAIR (d12, d13, 136); \ > + REG_PAIR (d14, d15, 152); > > #/** > # Saves the current CPU context that can be restored with a call to LongJump() and returns 0.# > diff --git a/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.asm b/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.asm > index 20dd0f1b850f..df70f298998e 100644 > --- a/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.asm > +++ b/MdePkg/Library/BaseLib/AArch64/SetJumpLongJump.asm > @@ -19,10 +19,10 @@ > REG_ONE (x16, #96) /*IP0*/ > > #define FPR_LAYOUT \ > - REG_PAIR ( d8, d9, #112); \ > - REG_PAIR (d10, d11, #128); \ > - REG_PAIR (d12, d13, #144); \ > - REG_PAIR (d14, d15, #160); > + REG_PAIR ( d8, d9, #104); \ > + REG_PAIR (d10, d11, #120); \ > + REG_PAIR (d12, d13, #136); \ > + REG_PAIR (d14, d15, #152); > > ;/** > ; Saves the current CPU context that can be restored with a call to LongJump() and returns 0.# >