On Thu, May 19, 2016 at 10:01:16AM +0200, Laszlo Ersek wrote:
> On 05/19/16 05:49, Gary Lin wrote:
> > The HTTP Token Wrap is created in EfiHttpResponse() and then passed
> > to the deferred Receive event callback, HttpTcpReceiveNotifyDpc.
> > HttpTcpReceiveHeader and HttpTcpReceiveBody use a Tcp polling loop to
> > monitor the socket status and trigger the Receive event when a new
> > packet arrives. The Receive event brings up HttpTcpReceiveNotifyDpc
> > to process the HTTP message and the function will set Wrap->TcpWrap.IsRxDone
> > to TRUE to break the Tcp polling loop.
> > 
> > However, HttpTcpReceiveNotifyDpc mistakenly freed Wrap, so the Tcp
> > polling loop was actually checking a dead variable, and this led the
> > system into an unstable status.
> > 
> > Given the fact that the HTTP Token Wrap will be freed in EfiHttpResponse
> > or HttpResponseWorker, this commit removes every "FreePool (Wrap)" in
> > HttpTcpReceiveNotifyDpc.
> > 
> > Cc: "Wu, Jiaxin" <jiaxin...@intel.com>
> > Cc: "Siyuan Fu" <siyuan...@intel.com>
> > Cc: "El-Haj-Mahmoud, Samer" <samer.el-haj-mahm...@hpe.com>
> > Cc: "Laszlo Ersek" <ler...@redhat.com>
> > Cc: "Hegde, Nagaraj P" <nagaraj-p.he...@hpe.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Gary Lin <g...@suse.com>
> > ---
> >  NetworkPkg/HttpDxe/HttpProto.c | 4 ----
> >  1 file changed, 4 deletions(-)
> > 
> > diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c
> > index f3992ed..afa7fe4 100644
> > --- a/NetworkPkg/HttpDxe/HttpProto.c
> > +++ b/NetworkPkg/HttpDxe/HttpProto.c
> > @@ -152,7 +152,6 @@ HttpTcpReceiveNotifyDpc (
> >      if (EFI_ERROR (Wrap->TcpWrap.Rx6Token.CompletionToken.Status)) {
> >        Wrap->HttpToken->Status = 
> > Wrap->TcpWrap.Rx6Token.CompletionToken.Status;
> >        gBS->SignalEvent (Wrap->HttpToken->Event);
> > -      FreePool (Wrap);
> >        return ;
> >      }
> >  
> > @@ -162,7 +161,6 @@ HttpTcpReceiveNotifyDpc (
> >      if (EFI_ERROR (Wrap->TcpWrap.Rx4Token.CompletionToken.Status)) {
> >        Wrap->HttpToken->Status = 
> > Wrap->TcpWrap.Rx4Token.CompletionToken.Status;
> >        gBS->SignalEvent (Wrap->HttpToken->Event);
> > -      FreePool (Wrap);
> >        return ;
> >      }
> >    }
> > @@ -234,8 +232,6 @@ HttpTcpReceiveNotifyDpc (
> >    // Check pending RxTokens and receive the HTTP message.
> >    //
> >    NetMapIterate (&Wrap->HttpInstance->RxTokens, HttpTcpReceive, NULL);
> > -  
> > -  FreePool (Wrap);
> >  }
> >  
> >  /**
> > 
> 
> I'll let the NetworkPkg maintainers review this one; just one note /
> question: didn't we recently encounter something similar elsewhere? I
> vaguely recall a similar problem where it wasn't immediately obvious
> whether the callback should free a UDP packet or the generic receive
> function. But I cannot find the message :(
> 

Do you mean this?
http://thread.gmane.org/gmane.comp.bios.edk2.devel/10156

The patch is simple but the sympton is nasty...

Cheers,

Gary Lin
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to