> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Wednesday, April 24, 2019 3:07 PM
> To: Wu, Hao A
> Cc: edk2-devel-groups-io; Kinney, Michael D; Gao, Liming; Wang, Jian J
> Subject: Re: [PATCH v2] MdeModulePkg/DxeCore: Please static checker for
> false report
>
> On Wed, 24 Apr 2019 at 07:05, Hao Wu <hao.a...@intel.com> wrote:
> >
> > After commit 57df17fe26, some static check reports suspicous NULL pointer
> > deference at line:
> >
> > Entry->MachineType = Entry->Emulator->MachineType;
> > ^^^^^^^^^^^^^^^
> >
> > within function PeCoffEmuProtocolNotify().
> >
> > However, 'Entry->Emulator' is guaranteed to have a non-NULL value when
> > previous call to the CoreHandleProtocol() returns EFI_SUCCESS.
> >
> > This commit will re-write the return status check for CoreHandleProtocol()
> > to add explicit NULL pointer check for protocol instance pointer.
> >
> > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> > Cc: Michael D Kinney <michael.d.kin...@intel.com>
> > Cc: Liming Gao <liming....@intel.com>
> > Cc: Jian J Wang <jian.j.w...@intel.com>
> > Signed-off-by: Hao Wu <hao.a...@intel.com>
>
> Again, I think it is rather unfortunate that we need code changes such
> as this one just to remove warnings from a flawed static analyzer. But
> the change looks correct to me, so
>
> Acked-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
Thanks Ard,
Hello Mike, Liming and Jian,
Do you have additional feedbacks on this patch?
If not, I plan to push it with the 'Ack' tag from Ard.
Best Regards,
Hao Wu
>
> > ---
> > MdeModulePkg/Core/Dxe/Image/Image.c | 23 ++++++++++++--------
> > 1 file changed, 14 insertions(+), 9 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
> b/MdeModulePkg/Core/Dxe/Image/Image.c
> > index 08306a73fd..de5b8bed27 100644
> > --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> > +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> > @@ -134,12 +134,14 @@ PeCoffEmuProtocolNotify (
> > IN VOID *Context
> > )
> > {
> > - EFI_STATUS Status;
> > - UINTN BufferSize;
> > - EFI_HANDLE EmuHandle;
> > - EMULATOR_ENTRY *Entry;
> > + EFI_STATUS Status;
> > + UINTN BufferSize;
> > + EFI_HANDLE EmuHandle;
> > + EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL *Emulator;
> > + EMULATOR_ENTRY *Entry;
> >
> > EmuHandle = NULL;
> > + Emulator = NULL;
> >
> > while (TRUE) {
> > BufferSize = sizeof (EmuHandle);
> > @@ -157,16 +159,19 @@ PeCoffEmuProtocolNotify (
> > return;
> > }
> >
> > - Entry = AllocateZeroPool (sizeof (*Entry));
> > - ASSERT (Entry != NULL);
> > -
> > Status = CoreHandleProtocol (
> > EmuHandle,
> > &gEdkiiPeCoffImageEmulatorProtocolGuid,
> > - (VOID **)&Entry->Emulator
> > + (VOID **)&Emulator
> > );
> > - ASSERT_EFI_ERROR (Status);
> > + if (EFI_ERROR (Status) || Emulator == NULL) {
> > + continue;
> > + }
> > +
> > + Entry = AllocateZeroPool (sizeof (*Entry));
> > + ASSERT (Entry != NULL);
> >
> > + Entry->Emulator = Emulator;
> > Entry->MachineType = Entry->Emulator->MachineType;
> >
> > InsertTailList (&mAvailableEmulators, &Entry->Link);
> > --
> > 2.12.0.windows.1
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#39623): https://edk2.groups.io/g/devel/message/39623
Mute This Topic: https://groups.io/mt/31318926/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
