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;
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
