From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=jordan.l.justen@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (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 58F052112FAAD for ; Tue, 18 Sep 2018 11:02:53 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Sep 2018 11:02:52 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,390,1531810800"; d="scan'208";a="89882614" Received: from kaichan1-mobl2.amr.corp.intel.com (HELO localhost) ([10.254.111.91]) by fmsmga004.fm.intel.com with ESMTP; 18 Sep 2018 11:02:52 -0700 MIME-Version: 1.0 To: Jian J Wang , edk2-devel@lists.01.org Message-ID: <153729377116.1021.13677753557698494356@jljusten-skl> From: Jordan Justen In-Reply-To: <20180918090448.7324-1-jian.j.wang@intel.com> Cc: Hao A Wu , Dandan Bi , Laszlo Ersek , Eric Dong , Michael Kinney References: <20180918090448.7324-1-jian.j.wang@intel.com> User-Agent: alot/0.7 Date: Tue, 18 Sep 2018 11:02:51 -0700 Subject: Re: [PATCH] UefiCpuPkg/CpuMpPei: fix unsafe way to get stack pointer 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, 18 Sep 2018 18:02:53 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable I guess the git config sendemail.from setting did not help your patches. ?? It still is coming through with a From field of . Regarding this patch, I suppose it is worth asking if &StackBase in the old code could possibly be an address not on the stack. I don't think it is possible, and I'm guessing the C specification would probably back that up. It can be unsafe to get an address of something on the stack and then refer to that address after the variable is no longer in scope. I suspect this is what the static checker is noticing. By calling SetJump, aren't we just doing the same thing, but hiding what we are doing from the static checker? So, can't we just tell the static checker to ignore the error because we know what we are doing? -Jordan On 2018-09-18 02:04:48, wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1186 > = > This patch uses SetJump() to get the stack pointer from esp/rsp > register to replace local variable way, which was marked by static > code checker as an unsafe way. > = > Cc: Dandan Bi > Cc: Hao A Wu > Cc: Eric Dong > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang > --- > UefiCpuPkg/CpuMpPei/CpuMpPei.h | 8 ++++++++ > UefiCpuPkg/CpuMpPei/CpuPaging.c | 9 +++++++-- > 2 files changed, 15 insertions(+), 2 deletions(-) > = > diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPe= i.h > index d097a66aa8..fe61f5e3bc 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h > +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h > @@ -35,6 +35,14 @@ > = > extern EFI_PEI_PPI_DESCRIPTOR mPeiCpuMpPpiDesc; > = > +#if defined (MDE_CPU_IA32) > +#define CPU_STACK_POINTER(Context) ((Context).Esp) > +#elif defined (MDE_CPU_X64) > +#define CPU_STACK_POINTER(Context) ((Context).Rsp) > +#else > +#error CPU type not supported! > +#endif > + > /** > This service retrieves the number of logical processor in the platform > and the number of those logical processors that are enabled on this bo= ot. > diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPag= ing.c > index c7e0822452..997c20c26e 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c > +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c > @@ -517,9 +517,14 @@ GetStackBase ( > IN OUT VOID *Buffer > ) > { > - EFI_PHYSICAL_ADDRESS StackBase; > + EFI_PHYSICAL_ADDRESS StackBase; > + BASE_LIBRARY_JUMP_BUFFER Context; > = > - StackBase =3D (EFI_PHYSICAL_ADDRESS)(UINTN)&StackBase; > + // > + // Retrieve stack pointer from current processor context. > + // > + SetJump (&Context); > + StackBase =3D (EFI_PHYSICAL_ADDRESS)CPU_STACK_POINTER (Context); > StackBase +=3D BASE_4KB; > StackBase &=3D ~((EFI_PHYSICAL_ADDRESS)BASE_4KB - 1); > StackBase -=3D PcdGet32(PcdCpuApStackSize); > -- = > 2.16.2.windows.1 > = > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel