Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix CPU stack guard support by aligning GDT buffer

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hi Vitaly,

Can you please provide some details on the compiler/build command that did not align the array 
correctly.

I agree that the GDT must have the correct alignment.

I do not like the idea of unused bytes at the beginning of the array. I would prefer to see
an array that is aligned correctly by declaration.
 

Mike

> -----Original Message-----
> From: Leif Lindholm <leif@nuviainc.com>
> Sent: Friday, November 5, 2021 12:28 PM
> To: devel@edk2.groups.io; cheptsov@ispras.ru
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Jeff Fan <vanjeff_919@hotmail.com>; Mikhail Krichanov <krichanov@ispras.ru>; Marvin
> Häuser <mhaeuser@posteo.de>
> Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix CPU stack guard support by aligning GDT buffer
> 
> UefiCpuPkg maintainers - please respond.
> 
> Meanwhile, Vitaly, could you please provide a commit message?
> The BZ link is needed, but it's not a substitute.
> 
> /
>     Leif
> 
> On Mon, Sep 20, 2021 at 17:13:47 +0300, Vitaly Cheptsov wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3639
> >
> >
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> >
> > Cc: Eric Dong <eric.dong@intel.com>
> >
> > Cc: Michael Kinney <michael.d.kinney@intel.com>
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> >
> > Cc: Jeff Fan <vanjeff_919@hotmail.com>
> >
> > Cc: Mikhail Krichanov <krichanov@ispras.ru>
> >
> > Cc: Marvin Häuser <mhaeuser@posteo.de>
> >
> > Signed-off-by: Vitaly Cheptsov <cheptsov@ispras.ru>
> >
> > ---
> >
> >  .../Library/CpuExceptionHandlerLib/DxeException.c    | 12 +++++++-----
> >
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> >
> >
> >
> > diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
> >
> > index fd59f09ecd..12874811e1 100644
> >
> > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
> >
> > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
> >
> > @@ -22,7 +22,7 @@ EXCEPTION_HANDLER_DATA      mExceptionHandlerData;
> >
> >
> >
> >  UINT8                       mNewStack[CPU_STACK_SWITCH_EXCEPTION_NUMBER *
> >
> >                                        CPU_KNOWN_GOOD_STACK_SIZE];
> >
> > -UINT8                       mNewGdt[CPU_TSS_GDT_SIZE];
> >
> > +UINT8                       mNewGdt[CPU_TSS_GDT_SIZE + IA32_GDT_ALIGNMENT];
> >
> >
> >
> >  /**
> >
> >    Common exception handler.
> >
> > @@ -238,6 +238,7 @@ InitializeCpuExceptionHandlersEx (
> >
> >    CPU_EXCEPTION_INIT_DATA           EssData;
> >
> >    IA32_DESCRIPTOR                   Idtr;
> >
> >    IA32_DESCRIPTOR                   Gdtr;
> >
> > +  UINT8                             *Gdt;
> >
> >
> >
> >    //
> >
> >    // To avoid repeat initialization of default handlers, the caller should pass
> >
> > @@ -259,6 +260,7 @@ InitializeCpuExceptionHandlersEx (
> >
> >      if (PcdGetBool (PcdCpuStackGuard)) {
> >
> >        if (InitData == NULL) {
> >
> >          SetMem (mNewGdt, sizeof (mNewGdt), 0);
> >
> > +        Gdt = ALIGN_POINTER (mNewGdt, IA32_GDT_ALIGNMENT);
> >
> >
> >
> >          AsmReadIdtr (&Idtr);
> >
> >          AsmReadGdtr (&Gdtr);
> >
> > @@ -270,11 +272,11 @@ InitializeCpuExceptionHandlersEx (
> >
> >          EssData.X64.StackSwitchExceptionNumber = CPU_STACK_SWITCH_EXCEPTION_NUMBER;
> >
> >          EssData.X64.IdtTable = (VOID *)Idtr.Base;
> >
> >          EssData.X64.IdtTableSize = Idtr.Limit + 1;
> >
> > -        EssData.X64.GdtTable = mNewGdt;
> >
> > -        EssData.X64.GdtTableSize = sizeof (mNewGdt);
> >
> > -        EssData.X64.ExceptionTssDesc = mNewGdt + Gdtr.Limit + 1;
> >
> > +        EssData.X64.GdtTable = Gdt;
> >
> > +        EssData.X64.GdtTableSize = CPU_TSS_GDT_SIZE;
> >
> > +        EssData.X64.ExceptionTssDesc = Gdt + Gdtr.Limit + 1;
> >
> >          EssData.X64.ExceptionTssDescSize = CPU_TSS_DESC_SIZE;
> >
> > -        EssData.X64.ExceptionTss = mNewGdt + Gdtr.Limit + 1 + CPU_TSS_DESC_SIZE;
> >
> > +        EssData.X64.ExceptionTss = Gdt + Gdtr.Limit + 1 + CPU_TSS_DESC_SIZE;
> >
> >          EssData.X64.ExceptionTssSize = CPU_TSS_SIZE;
> >
> >
> >
> >          InitData = &EssData;
> >
> > --
> >
> > 2.30.1 (Apple Git-130)
> >
> >
> >
> >
> >
> > 
> >
> >