On Wed, 24 Apr 2019 at 07:05, Hao Wu <[email protected]> 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 <[email protected]> > Cc: Michael D Kinney <[email protected]> > Cc: Liming Gao <[email protected]> > Cc: Jian J Wang <[email protected]> > Signed-off-by: Hao Wu <[email protected]>
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 <[email protected]> > --- > 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 (#39467): https://edk2.groups.io/g/devel/message/39467 Mute This Topic: https://groups.io/mt/31318926/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
