[edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.
Saloni Kasbekar
saloni.kasbekar at intel.com
Thu Jul 13 16:12:39 UTC 2023
Mike,
Would you be able to help us merge the patch?
Thanks,
Saloni
-----Original Message-----
From: Nickle Wang <nicklew at nvidia.com>
Sent: Wednesday, July 12, 2023 11:54 PM
To: devel at edk2.groups.io; Kasbekar, Saloni <saloni.kasbekar at intel.com>; Clark-williams, Zachary <zachary.clark-williams at intel.com>
Cc: Abner Chang <abner.chang at amd.com>; Igor Kulchytskyy <igork at ami.com>; Nick Ramirez <nramirez at nvidia.com>
Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.
Hi Saloni,
Could you please help to merge this fix since there is no objection during past weeks?
Thanks,
Nickle
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Saloni
> Kasbekar via groups.io
> Sent: Friday, June 30, 2023 6:28 AM
> To: Nickle Wang <nicklew at nvidia.com>; devel at edk2.groups.io
> Cc: Maciej Rabeda <maciej.rabeda at linux.intel.com>; Siyuan Fu
> <siyuan.fu at intel.com>; Abner Chang <abner.chang at amd.com>; Igor
> Kulchytskyy <igork at ami.com>; Nick Ramirez <nramirez at nvidia.com>
> Subject: Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> binding start issue.
>
> External email: Use caution opening links or attachments
>
>
> Hi Nickle,
>
> That makes sense. Thanks for the clarification.
>
> Reviewed-by: Saloni Kasbekar <saloni.kasbekar at intel.com>
>
> Thanks,
> Saloni
>
> -----Original Message-----
> From: Nickle Wang <nicklew at nvidia.com>
> Sent: Wednesday, June 28, 2023 3:30 PM
> To: Kasbekar, Saloni <saloni.kasbekar at intel.com>; devel at edk2.groups.io
> Cc: Maciej Rabeda <maciej.rabeda at linux.intel.com>; Siyuan Fu
> <siyuan.fu at intel.com>; Abner Chang <abner.chang at amd.com>; Igor
> Kulchytskyy <igork at ami.com>; Nick Ramirez <nramirez at nvidia.com>
> Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> binding start issue.
>
> Hi Saloni,
>
> Thanks for your review.
>
> When uninstall fails, per UEFI specification, the protocol will be
> installed again and will be visible to UEFI drivers.
>
> Page 190, UEFI spec. 2.10:
> "If any errors are generated while the protocol interfaces are being
> uninstalled, then the protocols uninstalled prior to the error will be
> reinstalled with the boot service
> EFI_BOOT_SERVICES.InstallProtocolInterface() and the status code
> EFI_INVALID_PARAMETER is returned."
>
> In this case, if we do FreePool while driver still can locate
> gEfiHttpServiceBindingProtocolGuid. Driver will access to the memory
> that is released to system. Memory issue may happen.
>
> Regards,
> Nickle
>
> > -----Original Message-----
> > From: Kasbekar, Saloni <saloni.kasbekar at intel.com>
> > Sent: Thursday, June 29, 2023 3:07 AM
> > To: devel at edk2.groups.io; Nickle Wang <nicklew at nvidia.com>
> > Cc: Maciej Rabeda <maciej.rabeda at linux.intel.com>; Siyuan Fu
> > <siyuan.fu at intel.com>; Abner Chang <abner.chang at amd.com>; Igor
> > Kulchytskyy <igork at ami.com>; Nick Ramirez <nramirez at nvidia.com>
> > Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> > binding start issue.
> >
> > External email: Use caution opening links or attachments
> >
> >
> > Hi Nickle,
> >
> > We would want to do the FreePool even if the Uninstall fails (like
> > in the case where we failed to install the multiple protocol
> > interfaces and then went to ON_ERROR). Do you think it's better if
> > we change it to -
> >
> > if (HttpService != NULL) {
> > HttpCleanService (HttpService, UsingIpv6);
> > Status = gBS->UninstallMultipleProtocolInterfaces (
> > &ControllerHandle,
> > &gEfiHttpServiceBindingProtocolGuid,
> > &HttpService->ServiceBinding,
> > NULL
> > );
> > if ((HttpService->Tcp4ChildHandle == NULL) &&
> > (HttpService->Tcp6ChildHandle == NULL)) {
> > FreePool (HttpService);
> > }
> > }
> >
> > Thanks,
> > Saloni
> >
> > -----Original Message-----
> > From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> > Nickle Wang via groups.io
> > Sent: Tuesday, June 27, 2023 5:56 PM
> > To: devel at edk2.groups.io; Nickle Wang <nicklew at nvidia.com>
> > Cc: Maciej Rabeda <maciej.rabeda at linux.intel.com>; Siyuan Fu
> > <siyuan.fu at intel.com>; Abner Chang <abner.chang at amd.com>; Igor
> > Kulchytskyy <igork at ami.com>; Nick Ramirez <nramirez at nvidia.com>
> > Subject: Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> > binding start issue.
> >
> > May I know if someone can help to review this patch?
> >
> > Thanks,
> > Nickle
> >
> > > -----Original Message-----
> > > From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> > > Nickle Wang via groups.io
> > > Sent: Friday, February 10, 2023 8:34 PM
> > > To: devel at edk2.groups.io
> > > Cc: Maciej Rabeda <maciej.rabeda at linux.intel.com>; Siyuan Fu
> > > <siyuan.fu at intel.com>; Abner Chang <abner.chang at amd.com>; Igor
> > > Kulchytskyy <igork at ami.com>; Nick Ramirez <nramirez at nvidia.com>
> > > Subject: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> > > binding start issue.
> > >
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > When failure happens in HttpDxeStart, the error handling code
> > > release the memory buffer but it does not uninstall HTTP service
> > > bindnig protocol. As the result, application can still locate this
> > > protocol and invoke service binding fucntions in released memory pool.
> > >
> > > Signed-off-by: Nickle Wang <nicklew at nvidia.com>
> > > Cc: Maciej Rabeda <maciej.rabeda at linux.intel.com>
> > > Cc: Siyuan Fu <siyuan.fu at intel.com>
> > > Cc: Abner Chang <abner.chang at amd.com>
> > > Cc: Igor Kulchytskyy <igork at ami.com>
> > > Cc: Nick Ramirez <nramirez at nvidia.com>
> > > ---
> > > NetworkPkg/HttpDxe/HttpDriver.c | 13 +++++++++++--
> > > 1 file changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/NetworkPkg/HttpDxe/HttpDriver.c
> > > b/NetworkPkg/HttpDxe/HttpDriver.c index 5d918d3c4d..f6d1263cad
> > > 100644
> > > --- a/NetworkPkg/HttpDxe/HttpDriver.c
> > > +++ b/NetworkPkg/HttpDxe/HttpDriver.c
> > > @@ -3,6 +3,7 @@
> > >
> > > Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
> > > (C) Copyright 2016 Hewlett Packard Enterprise Development
> > > LP<BR>
> > > + Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> > >
> > > SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > @@ -464,8 +465,16 @@ ON_ERROR:
> > >
> > > if (HttpService != NULL) {
> > > HttpCleanService (HttpService, UsingIpv6);
> > > - if ((HttpService->Tcp4ChildHandle == NULL) && (HttpService-
> > > >Tcp6ChildHandle == NULL)) {
> > > - FreePool (HttpService);
> > > + Status = gBS->UninstallMultipleProtocolInterfaces (
> > > + &ControllerHandle,
> > > + &gEfiHttpServiceBindingProtocolGuid,
> > > + &HttpService->ServiceBinding,
> > > + NULL
> > > + );
> > > + if (!EFI_ERROR (Status)) {
> > > + if ((HttpService->Tcp4ChildHandle == NULL) && (HttpService-
> > > >Tcp6ChildHandle == NULL)) {
> > > + FreePool (HttpService);
> > > + }
> > > }
> > > }
> > >
> > > --
> > > 2.39.1.windows.1
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106913): https://edk2.groups.io/g/devel/message/106913
Mute This Topic: https://groups.io/mt/99821789/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list