Re: [Patch] NetworkPkg: Fix Duplicate FreePool Error in WCM

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>


> -----Original Message-----
> From: Wang, Fan
> Sent: Friday, March 1, 2019 9:57 AM
> To: Fu, Siyuan <siyuan.fu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: RE: [Patch] NetworkPkg: Fix Duplicate FreePool Error in WCM
> 
> Thanks, Siyuan, will update commit message for this change.
> 
> Best Regards
> Fan
> 
> -----Original Message-----
> From: Fu, Siyuan
> Sent: Thursday, February 28, 2019 6:35 PM
> To: Wang, Fan <fan.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: RE: [Patch] NetworkPkg: Fix Duplicate FreePool Error in WCM
> 
> Hi, Fan
> 
> The patch also cancel a timer in driver binding stop, please describe this
> change in commit message, or separate it to another patch.
> 
> Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
> 
> > -----Original Message-----
> > From: Wang, Fan
> > Sent: Thursday, February 28, 2019 5:10 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
> > Wu, Jiaxin <jiaxin.wu@intel.com>
> > Subject: [Patch] NetworkPkg: Fix Duplicate FreePool Error in WCM
> >
> > * REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1577
> >
> > In WiFi Connection Manager scan process, the result received from WiFi
> > device driver will be freed twice, and will cause unexpected errors,
> > and even system crash.
> >
> > This issue also exists in some other places potentially, this patch is
> > to fix these issues.
> >
> > Cc: Ye Ting <ting.ye@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Wang Fan <fan.wang@intel.com>
> > ---
> >  NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrDriver.c    | 1
> +
> >  NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrFileUtil.c  | 1
> > +  .../WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAccess.c  |
> > 9
> > +++++++++
> >  NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrImpl.c      | 1
> -
> >  4 files changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git
> > a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrDriver.c
> > b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrDriver.c
> > index 1431cdc7ea..63b0670c63 100644
> > --- a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrDriver.c
> > +++
> b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrDriver.c
> > @@ -409,10 +409,11 @@ WifiMgrDxeDriverBindingStop (
> >    }
> >
> >    //
> >    // Close Event
> >    //
> > +  gBS->SetTimer (Nic->TickTimer, TimerCancel, 0);
> >    gBS->CloseEvent (Nic->TickTimer);
> >
> >    //
> >    // Clean Supported Suites
> >    //
> > diff --git
> > a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrFileUtil.c
> > b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrFileUtil.c
> > index 6db1626f2d..0224823431 100644
> > ---
> a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrFileUtil.c
> > +++
> b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrFileUtil.c
> > @@ -251,10 +251,11 @@ UpdatePage(
> >      }
> >    } else {
> >
> >      if (Private->FileContext->FileName != NULL) {
> >        FreePool (Private->FileContext->FileName);
> > +      Private->FileContext->FileName = NULL;
> >      }
> >      Private->FileContext->FileName = FileName;
> >
> >      if (FormId == FORMID_ENROLL_CERT) {
> >        HiiSetString (Private->RegisteredHandle, diff --git
> >
> a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAcc
> ess
> > .c
> >
> b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAcc
> ess
> > .c
> > index bfb6b6e5ca..d0d55f46da 100644
> > ---
> >
> a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAcc
> ess
> > .c
> > +++
> b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAc
> > +++ cess.c
> > @@ -445,10 +445,12 @@ WifiMgrRefreshNetworkList (
> >          UnicodeSPrint (PortString, PortStringSize, L"AKMSuite: %s
> > CipherSuite: %s", AKMListDisplay, CipherListDisplay);
> >          PortHelpToken = HiiSetString (Private->RegisteredHandle, 0,
> > PortString, NULL);
> >        }
> >        FreePool (AKMListDisplay);
> >        FreePool (CipherListDisplay);
> > +      AKMListDisplay    = NULL;
> > +      CipherListDisplay = NULL;
> >
> >        HiiCreateGotoOpCode (
> >          StartOpCodeHandle,
> >          FORMID_CONNECT_NETWORK,
> >          PortPromptToken,
> > @@ -530,10 +532,12 @@ WifiMgrRefreshNetworkList (
> >            goto Exit;
> >          }
> >        }
> >        FreePool (AKMListDisplay);
> >        FreePool (CipherListDisplay);
> > +      AKMListDisplay    = NULL;
> > +      CipherListDisplay = NULL;
> >
> >        HiiCreateGotoOpCode (
> >          StartOpCodeHandle,
> >          FORMID_CONNECT_NETWORK,
> >          PortPromptToken,
> > @@ -610,10 +614,12 @@ WifiMgrRefreshNetworkList (
> >            AKMListDisplay, CipherListDisplay
> >            );
> >        }
> >        FreePool (AKMListDisplay);
> >        FreePool (CipherListDisplay);
> > +      AKMListDisplay    = NULL;
> > +      CipherListDisplay = NULL;
> >
> >        PortHelpToken = HiiSetString (Private->RegisteredHandle, 0,
> > PortString, NULL);
> >
> >        HiiCreateGotoOpCode (
> >          StartOpCodeHandle,
> > @@ -1655,10 +1661,11 @@ WifiMgrDxeHiiConfigAccessCallback (
> >          Profile = Private->CurrentNic->UserSelectedProfile;
> >          if (Profile->PrivateKeyData != NULL) {
> >
> >            ZeroMem (Profile->PrivateKeyData, Profile->PrivateKeyDataSize);
> >            FreePool (Profile->PrivateKeyData);
> > +          Profile->PrivateKeyData = NULL;
> >          }
> >
> >          Status = WifiMgrReadFileToBuffer (
> >                     Private->FileContext,
> >                     &TempData,
> > @@ -1698,16 +1705,18 @@ WifiMgrDxeHiiConfigAccessCallback (
> >          if (Private->FileType == FileTypeCACert) {
> >            if (Profile->CACertData != NULL) {
> >
> >              ZeroMem (Profile->CACertData, Profile->CACertSize);
> >              FreePool (Profile->CACertData);
> > +            Profile->CACertData = NULL;
> >            }
> >          } else if (Private->FileType == FileTypeClientCert) {
> >            if (Profile->ClientCertData != NULL) {
> >
> >              ZeroMem (Profile->ClientCertData, Profile->ClientCertSize);
> >              FreePool (Profile->ClientCertData);
> > +            Profile->ClientCertData = NULL;
> >            }
> >          } else {
> >            break;
> >          }
> >
> > diff --git
> > a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrImpl.c
> > b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrImpl.c
> > index 52ce18dfd8..9417440ac5 100644
> > --- a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrImpl.c
> > +++ b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrImpl.c
> > @@ -210,11 +210,10 @@ WifiMgrOnScanFinished (
> >        }
> >        continue;
> >      }
> >    }
> >
> > -  FreePool (Result);
> >    gBS->SignalEvent (Nic->Private->NetworkListRefreshEvent);
> >
> >    //
> >    // The current connected network should always be available until
> > disconnection
> >    // happens in Wifi FW layer, even when it is not in this time's scan result.
> > --
> > 2.16.2.windows.1