From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 3F1592114399C for ; Wed, 19 Sep 2018 04:17:17 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 23D073003995; Wed, 19 Sep 2018 11:17:17 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-175.rdu2.redhat.com [10.10.120.175]) by smtp.corp.redhat.com (Postfix) with ESMTP id 76DA45D96E; Wed, 19 Sep 2018 11:17:15 +0000 (UTC) To: Jordan Justen , Jian J Wang , edk2-devel@lists.01.org Cc: Hao A Wu , Dandan Bi , Eric Dong , Michael Kinney References: <20180918090448.7324-1-jian.j.wang@intel.com> <153729377116.1021.13677753557698494356@jljusten-skl> From: Laszlo Ersek Message-ID: <47392a62-419a-45e2-6a09-5cec8b35cd55@redhat.com> Date: Wed, 19 Sep 2018 13:17:14 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <153729377116.1021.13677753557698494356@jljusten-skl> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.46]); Wed, 19 Sep 2018 11:17:17 +0000 (UTC) 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: Wed, 19 Sep 2018 11:17:18 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 09/18/18 20:02, Jordan Justen wrote: > 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? Yep, we're totally doing that. Thanks, Laszlo > > 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=1186 >> >> 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/CpuMpPei.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 boot. >> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.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 = (EFI_PHYSICAL_ADDRESS)(UINTN)&StackBase; >> + // >> + // Retrieve stack pointer from current processor context. >> + // >> + SetJump (&Context); >> + StackBase = (EFI_PHYSICAL_ADDRESS)CPU_STACK_POINTER (Context); >> StackBase += BASE_4KB; >> StackBase &= ~((EFI_PHYSICAL_ADDRESS)BASE_4KB - 1); >> StackBase -= PcdGet32(PcdCpuApStackSize); >> -- >> 2.16.2.windows.1 >> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel