From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (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 1D8D62095BB65 for ; Wed, 6 Sep 2017 10:17:31 -0700 (PDT) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Sep 2017 10:20:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,354,1500966000"; d="scan'208";a="148239281" Received: from jljusten-skl.jf.intel.com (HELO localhost) ([10.54.75.25]) by fmsmga005.fm.intel.com with ESMTP; 06 Sep 2017 10:20:20 -0700 MIME-Version: 1.0 To: "Song, Ge" , "edk2-devel@lists.01.org" , "songgebird@gmail.com" Message-ID: <150471841914.27362.4844206984309034539@jljusten-skl.jf.intel.com> From: Jordan Justen In-Reply-To: Cc: Laszlo Ersek , Ard Biesheuvel References: <20170906031135.10358-1-ge.song@hxt-semitech.com> <20170906031135.10358-2-ge.song@hxt-semitech.com> User-Agent: alot/0.6 Date: Wed, 06 Sep 2017 10:20:19 -0700 Subject: Re: [PATCH v3 1/1] OvmfPkg/SecMain: Fix stack switching to permanent memory X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Sep 2017 17:17:31 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2017-09-05 20:13:19, Song, Ge wrote: > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ge Song > = Pushed as 89796c69d9fdaa9bd13d37b6b1687df5e0104ed5. Thanks for noticing that bug and fixing it! I also hope your IT might be to help you configure git send-email to make future contributions from your hxt-semitech.com email a bit easier. Thanks again! -Jordan > = > -----Original Message----- > From: songgebird@gmail.com [mailto:songgebird@gmail.com] > Sent: 2017=E5=B9=B49=E6=9C=886=E6=97=A5 11:12 > To: edk2-devel@lists.01.org > Cc: Jordan Justen ; Laszlo Ersek ; Ard Biesheuvel ; Song, Ge > Subject: [PATCH v3 1/1] OvmfPkg/SecMain: Fix stack switching to permanent= memory > = > From: Ge Song > = > In earlier PEI stage, temporary memory at PcdOvmfSecPeiTempRamBase is > employed as stack and heap. We move them to the new room and do some > relocation fixup when permanent memory becomes available. > TemporaryRamMigration() is responsible for switching the stack. > = > Before entering TemporaryRamMigration(), Ebp/Rbp is populated with the > content of Esp/Rsp and used as frame pointer. > = > After the execution of SetJump/LongJump, stack migrates to new position > while the context keeps unchanged. > = > But when TemporaryRamMigration() exits, Esp/Rsp is filled with > the content of Ebp/Rbp to destroy this stack frame. > = > The result is, stack switches back to previous temporary memory. > = > When permanent memory becomes available, modules that have registered > themselves for shadowing will be scheduled to execute. Some of them > need to consume more memory(heap/stack). Contrast to temporary stack, > permanent stack possesses larger space. > = > The potential risk is overflowing the stack if stack staying in > temporary memory. When it happens, system may crash during S3 resume. > = > More detailed information: > > (gdb) disassemble /r > > Dump of assembler code for function TemporaryRamMigration: > > 0x00000000fffcd29c <+0>:55push %rbp > > 0x00000000fffcd29d <+1>:48 89 e5mov %rsp,%rbp > > 0x00000000fffcd2a0 <+4>:48 81 ec 70 01 00 00sub > > $0x170,%rsp > > ... > > ... > > 0x00000000fffcd425 <+393>:e8 80 10 00 00callq 0xfffce4aa > > > > =3D> 0x00000000fffcd42a <+398>:b8 00 00 00 00mov $0x0,%eax > > 0x00000000fffcd42f <+403>:c9leaveq > > 0x00000000fffcd430 <+404>:c3retq > > End of assembler dump. > = > See the description of leave(opcode: c9), from > Intel 64 and IA-32 Architectures Software Developer's Manual, Volume 2A > = > "Releases the stack frame set up by an earlier ENTER instruction. The > LEAVE instruction copies the frame pointer (in the EBP register) into > the stack pointer register (ESP), which releases the stack space > allocated to the stack frame. The old frame pointer (the frame pointer > for the calling procedure that was saved by the ENTER instruction) is > then popped from the stack into the EBP register, restoring the calling > procedure=E2=80=99s stack frame." > = > To solve this, update Ebp/Rbp too when Esp/Rsp is updated > = > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ge Song > Tested-by: Laszlo Ersek > Reviewed-by: Laszlo Ersek > --- > OvmfPkg/Sec/SecMain.c | 2 ++ > 1 file changed, 2 insertions(+) > = > diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c > index e1993ec347b5..f7fec3d8c03b 100644 > --- a/OvmfPkg/Sec/SecMain.c > +++ b/OvmfPkg/Sec/SecMain.c > @@ -931,9 +931,11 @@ TemporaryRamMigration ( > if (SetJump (&JumpBuffer) =3D=3D 0) { > #if defined (MDE_CPU_IA32) > JumpBuffer.Esp =3D JumpBuffer.Esp + DebugAgentContext.StackMigrateOf= fset; > + JumpBuffer.Ebp =3D JumpBuffer.Ebp + DebugAgentContext.StackMigrateOf= fset; > #endif > #if defined (MDE_CPU_X64) > JumpBuffer.Rsp =3D JumpBuffer.Rsp + DebugAgentContext.StackMigrateOf= fset; > + JumpBuffer.Rbp =3D JumpBuffer.Rbp + DebugAgentContext.StackMigrateOf= fset; > #endif > LongJump (&JumpBuffer, (UINTN)-1); > } > -- > 2.11.0 > = > = > = > = > This email is intended only for the named addressee. It may contain infor= mation that is confidential/private, legally privileged, or copyright-prote= cted, and you should handle it accordingly. If you are not the intended rec= ipient, you do not have legal rights to retain, copy, or distribute this em= ail or its contents, and should promptly delete the email and all electroni= c copies in your system; do not retain copies in any media. If you have rec= eived this email in error, please notify the sender promptly. Thank you. > = >=20