From: "Wu, Hao A" <hao.a.wu@intel.com>
To: "Wang, Jian J" <jian.j.wang@intel.com>,
"Justen, Jordan L" <jordan.l.justen@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Bi, Dandan" <dandan.bi@intel.com>,
Laszlo Ersek <lersek@redhat.com>,
"Dong, Eric" <eric.dong@intel.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [PATCH] UefiCpuPkg/CpuMpPei: fix unsafe way to get stack pointer
Date: Wed, 19 Sep 2018 01:21:51 +0000 [thread overview]
Message-ID: <B80AF82E9BFB8E4FBD8C89DA810C6A0931E449E1@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <D827630B58408649ACB04F44C510003624E365F0@SHSMSX103.ccr.corp.intel.com>
> -----Original Message-----
> From: Wang, Jian J
> Sent: Wednesday, September 19, 2018 9:13 AM
> To: Justen, Jordan L; edk2-devel@lists.01.org
> Cc: Wu, Hao A; Bi, Dandan; Laszlo Ersek; Dong, Eric; Kinney, Michael D
> Subject: RE: [edk2] [PATCH] UefiCpuPkg/CpuMpPei: fix unsafe way to get stack
> pointer
>
> Jordan,
>
> No, it didn't help. But thanks for the help before. And I saw there's another guy
> having the same issue as mine. I tend to believe it's outlook (client/server) issue
> now.
>
> I think you're right about the "unsafe". The question is it's almost impossible for
> the static code checker to know how you use it. So it tend to give warning
> anyway.
>
> Hao, do you have comment on adding the exception?
Hi,
It is doable to tell the checker to ignore this issue.
I think it will be better to also hear Mike's comment on this one.
Best Regards,
Hao Wu
>
> Regards,
> Jian
>
>
> > -----Original Message-----
> > From: Justen, Jordan L
> > Sent: Wednesday, September 19, 2018 2:03 AM
> > To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Bi, Dandan <dandan.bi@intel.com>;
> > Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>;
> Kinney,
> > Michael D <michael.d.kinney@intel.com>
> > Subject: Re: [edk2] [PATCH] UefiCpuPkg/CpuMpPei: fix unsafe way to get
> stack
> > pointer
> >
> > I guess the git config sendemail.from setting did not help your
> > patches. ?? It still is coming through with a From field of
> > <edk2-devel-bounces@lists.01.org>.
> >
> > 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=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 <dandan.bi@intel.com>
> > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > Cc: Eric Dong <eric.dong@intel.com>
> > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> > > ---
> > > 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
next prev parent reply other threads:[~2018-09-19 1:22 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-18 9:04 [PATCH] UefiCpuPkg/CpuMpPei: fix unsafe way to get stack pointer Jian J Wang
2018-09-18 16:41 ` Laszlo Ersek
2018-09-19 0:46 ` Wang, Jian J
2018-09-18 18:02 ` Jordan Justen
2018-09-19 1:12 ` Wang, Jian J
2018-09-19 1:21 ` Wu, Hao A [this message]
2018-09-19 11:17 ` Laszlo Ersek
2018-09-26 2:18 ` Wang, Jian J
2018-09-26 8:30 ` Laszlo Ersek
2018-09-26 8:54 ` Wang, Jian J
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=B80AF82E9BFB8E4FBD8C89DA810C6A0931E449E1@SHSMSX104.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox