From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=SK4vBOBm; spf=pass (domain: linaro.org, ip: 209.85.166.194, mailfrom: ard.biesheuvel@linaro.org) Received: from mail-it1-f194.google.com (mail-it1-f194.google.com [209.85.166.194]) by groups.io with SMTP; Wed, 24 Apr 2019 00:07:28 -0700 Received: by mail-it1-f194.google.com with SMTP id x132so4531791itf.2 for ; Wed, 24 Apr 2019 00:07:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=+avo2W7rwX6TVWe+zJugKoz/kw+7EcGTbKENsnkf2kc=; b=SK4vBOBmtHq84b2pXZ145oUxtv6cGAX7qmoOQ+9hmNs6zHM2peW8PH3mNKSMrFjwWq PxfmzQt0lpofmQOhujO2a3T2h7INaMJ+I+r6lZJ5vlexGZpfL3haQs+uirVpD9WmKX3Q i/sNXlExspL35uFfSObb751V23F1Dz4y2ebhO3beCFTr8McDonkhPralP2xYR3uArCDK z5IVB1EK+Q3pcuM0CLWXo6Mho0Fu3gOVXB5Y68EGrv0tNzRsSL3EwM3N9e71wVDM+ENA 223bU2H99MqYVGwZs4pwOBy4s4rBm5SKG4sZeANN35FuYzVaoVBBUMkMmL0SWN/gED77 PxsA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=+avo2W7rwX6TVWe+zJugKoz/kw+7EcGTbKENsnkf2kc=; b=Y/dhsDrUAuL4szqfkOw1HnbClEUW4mNOsIHflM/ZzWZQFew/hVvlDAymKV59Ij9IFP 8oBiuqwo/xsBF1+TDXU+FAZpW8YwHbdcguI2Bs/MPX4pyIl8iIKYyoNoqLD2jI5cC5Lq p+pnI5thPx1olzu+AMznJZ1fvzdxP+juaAIuqnFEpQVzyWUWJvWcpl84SOOlhAEfd7PX ALJ0V+txMElzOW07DP1JEEhO7CC4EHMee84MqzRW8po+KEkFVHtnRA9tsus5H0S28Bgr aXiRQkrpp3vJOdUeRfI1F0Xs3DgSCzi/OOX3xg4+AMo9eaesPiH6YLaYamB0e1fbuHAq 7b7A== X-Gm-Message-State: APjAAAUUJlXSeco75nqzNs/5E8vfRtS24ea0t0UcHwbfbfXLUhLUrRn/ S7ucJUNv+I8K8+vMpnud7WDHJwvFh6PrS/Vj3dKW4g== X-Google-Smtp-Source: APXvYqxpq1yydXi9VZRk5EQ0dYhZ4Y7eahBhrO6pTt6/fgpJj7z1lS+D6EwyrgbqX1GN/Aywkpx4LSQtytMHW8gZ55s= X-Received: by 2002:a24:1312:: with SMTP id 18mr4931434itz.121.1556089647735; Wed, 24 Apr 2019 00:07:27 -0700 (PDT) MIME-Version: 1.0 References: <20190424050549.6760-1-hao.a.wu@intel.com> In-Reply-To: <20190424050549.6760-1-hao.a.wu@intel.com> From: "Ard Biesheuvel" Date: Wed, 24 Apr 2019 09:07:15 +0200 Message-ID: Subject: Re: [PATCH v2] MdeModulePkg/DxeCore: Please static checker for false report To: Hao Wu Cc: edk2-devel-groups-io , Michael D Kinney , Liming Gao , Jian J Wang Content-Type: text/plain; charset="UTF-8" On Wed, 24 Apr 2019 at 07:05, Hao Wu 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 > Cc: Michael D Kinney > Cc: Liming Gao > Cc: Jian J Wang > Signed-off-by: Hao Wu 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 > --- > 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 >