On Fri, May 20, 2016 at 12:21:39AM +0000, Wu, Jiaxin wrote:
> Gary, good catching! Wrap->TcpWrap.IsRxDone is invalid if it's freed in
> HttpTcpReceiveNotifyDpc. But, if you remove it directly, Wrap will not be
> freed for each TcpReceive in your patch. Actually, we should free it before
> return HttpResponseWorker.
> //
> // We still need receive more data when there is no cache data and
> MsgParser is not NULL;
> //
> Status = HttpTcpReceiveBody (Wrap, HttpMsg, HttpInstance->TimeoutEvent);
> gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);
> if (EFI_ERROR (Status)) {
> goto Error;
> }
> FreePool (Wrap);<---- Free it here.
> return Status;
>
Ah, right! I should add that. I'll send a V2.
Thanks,
Gary Lin
> Thanks.
> Jiaxin
>
> > -----Original Message-----
> > From: Gary Lin [mailto:[email protected]]
> > Sent: Thursday, May 19, 2016 11:49 AM
> > To: [email protected]
> > Cc: Wu, Jiaxin <[email protected]>; Fu, Siyuan <[email protected]>; El-
> > Haj-Mahmoud, Samer <[email protected]>; Laszlo Ersek
> > <[email protected]>; Hegde, Nagaraj P <[email protected]>
> > Subject: [PATCH 1/2] NetworkPkg/HttpDxe: Don't free Wrap in
> > HttpTcpReceiveNotifyDpc
> >
> > 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" <[email protected]>
> > Cc: "Siyuan Fu" <[email protected]>
> > Cc: "El-Haj-Mahmoud, Samer" <[email protected]>
> > Cc: "Laszlo Ersek" <[email protected]>
> > Cc: "Hegde, Nagaraj P" <[email protected]>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Gary Lin <[email protected]>
> > ---
> > 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);
> > }
> >
> > /**
> > --
> > 2.8.2
>
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
