Hi chasel,
> -----Original Message-----
> From: Chiu, Chasel
> Sent: Wednesday, September 4, 2019 8:13 PM
> To: devel@edk2.groups.io; Zhang, Shenglei <shenglei.zh...@intel.com>
> Cc: Kubacki, Michael A <michael.a.kuba...@intel.com>; Desimone, Nathaniel
> L <nathaniel.l.desim...@intel.com>; Gao, Liming <liming....@intel.com>
> Subject: RE: [edk2-devel] [PATCH]
> MinPlatformPkg/SecFspWrapperPlatformSecLib: Change
> TopOfTemporaryRam type
>
>
> Hi Shenglei,
>
> Would you please elaborate a little on how casting to UINTN can resolve the
> overflow scenario and why 64bits OS will affect this code?
Actually casting to UINTN can't resolve the overflow.
What I mean is the result of (TopOfTemporaryRam - sizeof (UINT32)) may overflow.
While it's meaningless to cast an already overflowed result to another type. So
I update
the code to cast the variable to UINT before it is arithmetically operated.
Under 64bits OS, the size of UINTN is 64-bit and the original
"(TopOfTemporaryRam - sizeof (UINT32)) "
is 32-bit. So the operation for casting will be performed. That's why 64bits OS
will affect this code.
Thanks,
Shenglei
>
> Thanks!
> Chasel
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Zhang,
> > Shenglei
> > Sent: Monday, September 2, 2019 8:35 PM
> > To: devel@edk2.groups.io
> > Cc: Kubacki, Michael A <michael.a.kuba...@intel.com>; Chiu, Chasel
> > <chasel.c...@intel.com>; Desimone, Nathaniel L
> > <nathaniel.l.desim...@intel.com>; Gao, Liming <liming....@intel.com>
> > Subject: [edk2-devel] [PATCH]
> MinPlatformPkg/SecFspWrapperPlatformSecLib:
> > Change TopOfTemporaryRam type
> >
> > Cast TopOfTemporaryRam's from UINT32 to UINTN in the expression.
> > The original code (TopOfTemporaryRam - sizeof (UINT32)) may cause
> > overflow. As a result the operation under 64-bit OS environment,
> (UINT)(...),
> > may cast a overflowed 4-byte result to 8-byte one.
> >
> > Cc: Michael Kubacki <michael.a.kuba...@intel.com>
> > Cc: Chasel Chiu <chasel.c...@intel.com>
> > Cc: Nate DeSimone <nathaniel.l.desim...@intel.com>
> > Cc: Liming Gao <liming....@intel.com>
> > Signed-off-by: Shenglei Zhang <shenglei.zh...@intel.com>
> > ---
> > .../Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c | 2 +-
> > .../SecFspWrapperPlatformSecLib/SecPlatformInformation.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git
> >
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
> or
> > mSecLib/SecGetPerformance.c
> >
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
> or
> > mSecLib/SecGetPerformance.c
> > index c4eeb2b1..0cc42f96 100644
> > ---
> >
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
> or
> > mSecLib/SecGetPerformance.c
> > +++
> > b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
> > +++ formSecLib/SecGetPerformance.c
> > @@ -79,7 +79,7 @@ SecGetPerformance (
> > //
> > TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi -
> > sizeof(UINT32);
> > TopOfTemporaryRam -= sizeof(UINT32) * 2;
> > - Count = *(UINT32 *) (UINTN) (TopOfTemporaryRam - sizeof
> > (UINT32));
> > + Count = *(UINT32 *)((UINTN)TopOfTemporaryRam - sizeof
> > (UINT32));
> > Size = Count * sizeof (UINT32);
> >
> > Ticker = *(UINT64 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32) -
> Size
> > - sizeof (UINT32) * 2); diff --git
> >
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
> or
> > mSecLib/SecPlatformInformation.c
> >
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
> or
> > mSecLib/SecPlatformInformation.c
> > index 5b94ed2b..1bcee5f4 100644
> > ---
> >
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatf
> or
> > mSecLib/SecPlatformInformation.c
> > +++
> > b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
> > +++ formSecLib/SecPlatformInformation.c
> > @@ -61,7 +61,7 @@ SecPlatformInformation (
> > //
> > TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi - sizeof
> > (UINT32);
> > TopOfTemporaryRam -= sizeof(UINT32) * 2;
> > - Count = *((UINT32 *)(UINTN) (TopOfTemporaryRam - sizeof
> > (UINT32)));
> > + Count = *((UINT32 *)((UINTN)TopOfTemporaryRam - sizeof
> > (UINT32)));
> > Size = Count * sizeof (IA32_HANDOFF_STATUS);
> >
> > if ((*StructureSize) < (UINT64) Size) {
> > --
> > 2.18.0.windows.1
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46857): https://edk2.groups.io/g/devel/message/46857
Mute This Topic: https://groups.io/mt/33110619/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
