Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.

[Nickle Wang]

May I know if someone can help to review this patch?

Thanks,
Nickle

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nickle Wang
> via groups.io
> Sent: Friday, February 10, 2023 8:34 PM
> To: devel@edk2.groups.io
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy
> <igork@ami.com>; Nick Ramirez <nramirez@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@nvidia.com>
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Nick Ramirez <nramirez@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
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.

[Saloni Kasbekar]

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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nickle Wang via groups.io
Sent: Tuesday, June 27, 2023 5:56 PM
To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nickle 
> Wang via groups.io
> Sent: Friday, February 10, 2023 8:34 PM
> To: devel@edk2.groups.io
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu 
> <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor 
> Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@nvidia.com>
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Nick Ramirez <nramirez@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
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.

[Nickle Wang]

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@intel.com>
> Sent: Thursday, June 29, 2023 3:07 AM
> To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy
> <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nickle Wang
> via groups.io
> Sent: Tuesday, June 27, 2023 5:56 PM
> To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy
> <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nickle
> > Wang via groups.io
> > Sent: Friday, February 10, 2023 8:34 PM
> > To: devel@edk2.groups.io
> > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@nvidia.com>
> > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Igor Kulchytskyy <igork@ami.com>
> > Cc: Nick Ramirez <nramirez@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
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.

[Saloni Kasbekar]

Hi Nickle,

That makes sense. Thanks for the clarification.

Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>

Thanks,
Saloni

-----Original Message-----
From: Nickle Wang <nicklew@nvidia.com> 
Sent: Wednesday, June 28, 2023 3:30 PM
To: Kasbekar, Saloni <saloni.kasbekar@intel.com>; devel@edk2.groups.io
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@intel.com>
> Sent: Thursday, June 29, 2023 3:07 AM
> To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu 
> <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor 
> Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nickle 
> Wang via groups.io
> Sent: Tuesday, June 27, 2023 5:56 PM
> To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu 
> <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor 
> Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
> > Nickle Wang via groups.io
> > Sent: Friday, February 10, 2023 8:34 PM
> > To: devel@edk2.groups.io
> > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu 
> > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor 
> > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@nvidia.com>
> > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Igor Kulchytskyy <igork@ami.com>
> > Cc: Nick Ramirez <nramirez@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
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.

[Nickle Wang]

Hi Saloni,

Could you please help to merge this fix since there is no objection during past weeks?

Thanks,
Nickle

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Saloni
> Kasbekar via groups.io
> Sent: Friday, June 30, 2023 6:28 AM
> To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy
> <igork@ami.com>; Nick Ramirez <nramirez@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@intel.com>
> 
> Thanks,
> Saloni
> 
> -----Original Message-----
> From: Nickle Wang <nicklew@nvidia.com>
> Sent: Wednesday, June 28, 2023 3:30 PM
> To: Kasbekar, Saloni <saloni.kasbekar@intel.com>; devel@edk2.groups.io
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy
> <igork@ami.com>; Nick Ramirez <nramirez@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@intel.com>
> > Sent: Thursday, June 29, 2023 3:07 AM
> > To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nickle
> > Wang via groups.io
> > Sent: Tuesday, June 27, 2023 5:56 PM
> > To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > Nickle Wang via groups.io
> > > Sent: Friday, February 10, 2023 8:34 PM
> > > To: devel@edk2.groups.io
> > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@nvidia.com>
> > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> > > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > > Cc: Abner Chang <abner.chang@amd.com>
> > > Cc: Igor Kulchytskyy <igork@ami.com>
> > > Cc: Nick Ramirez <nramirez@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
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.

[Saloni Kasbekar]

Mike,

Would you be able to help us merge the patch?

Thanks,
Saloni

-----Original Message-----
From: Nickle Wang <nicklew@nvidia.com> 
Sent: Wednesday, July 12, 2023 11:54 PM
To: devel@edk2.groups.io; Kasbekar, Saloni <saloni.kasbekar@intel.com>; Clark-williams, Zachary <zachary.clark-williams@intel.com>
Cc: Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Saloni 
> Kasbekar via groups.io
> Sent: Friday, June 30, 2023 6:28 AM
> To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu 
> <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor 
> Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@intel.com>
> 
> Thanks,
> Saloni
> 
> -----Original Message-----
> From: Nickle Wang <nicklew@nvidia.com>
> Sent: Wednesday, June 28, 2023 3:30 PM
> To: Kasbekar, Saloni <saloni.kasbekar@intel.com>; devel@edk2.groups.io
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu 
> <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor 
> Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@intel.com>
> > Sent: Thursday, June 29, 2023 3:07 AM
> > To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu 
> > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor 
> > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
> > Nickle Wang via groups.io
> > Sent: Tuesday, June 27, 2023 5:56 PM
> > To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu 
> > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor 
> > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
> > > Nickle Wang via groups.io
> > > Sent: Friday, February 10, 2023 8:34 PM
> > > To: devel@edk2.groups.io
> > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu 
> > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor 
> > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@nvidia.com>
> > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> > > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > > Cc: Abner Chang <abner.chang@amd.com>
> > > Cc: Igor Kulchytskyy <igork@ami.com>
> > > Cc: Nick Ramirez <nramirez@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
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.

[Michael D Kinney]

Acked-by: Michael D Kinney <michael.d.kinney@intel.com>

> -----Original Message-----
> From: Kasbekar, Saloni <saloni.kasbekar@intel.com>
> Sent: Thursday, July 13, 2023 9:13 AM
> To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io; Clark-
> williams, Zachary <zachary.clark-williams@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy <igork@ami.com>;
> Nick Ramirez <nramirez@nvidia.com>
> Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> binding start issue.
> 
> Mike,
> 
> Would you be able to help us merge the patch?
> 
> Thanks,
> Saloni
> 
> -----Original Message-----
> From: Nickle Wang <nicklew@nvidia.com>
> Sent: Wednesday, July 12, 2023 11:54 PM
> To: devel@edk2.groups.io; Kasbekar, Saloni <saloni.kasbekar@intel.com>;
> Clark-williams, Zachary <zachary.clark-williams@intel.com>
> Cc: Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy <igork@ami.com>;
> Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Saloni
> > Kasbekar via groups.io
> > Sent: Friday, June 30, 2023 6:28 AM
> > To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io
> > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@intel.com>
> >
> > Thanks,
> > Saloni
> >
> > -----Original Message-----
> > From: Nickle Wang <nicklew@nvidia.com>
> > Sent: Wednesday, June 28, 2023 3:30 PM
> > To: Kasbekar, Saloni <saloni.kasbekar@intel.com>; devel@edk2.groups.io
> > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@intel.com>
> > > Sent: Thursday, June 29, 2023 3:07 AM
> > > To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > Nickle Wang via groups.io
> > > Sent: Tuesday, June 27, 2023 5:56 PM
> > > To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > Nickle Wang via groups.io
> > > > Sent: Friday, February 10, 2023 8:34 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@nvidia.com>
> > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> > > > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > > > Cc: Abner Chang <abner.chang@amd.com>
> > > > Cc: Igor Kulchytskyy <igork@ami.com>
> > > > Cc: Nick Ramirez <nramirez@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 (#107076): https://edk2.groups.io/g/devel/message/107076
Mute This Topic: https://groups.io/mt/99821789/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.

[Michael D Kinney]

Merged: https://github.com/tianocore/edk2/pull/4666

Mike

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Wednesday, July 19, 2023 5:27 PM
> To: Kasbekar, Saloni <saloni.kasbekar@intel.com>; Nickle Wang
> <nicklew@nvidia.com>; devel@edk2.groups.io; Clark-williams, Zachary
> <zachary.clark-williams@intel.com>
> Cc: Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy <igork@ami.com>;
> Nick Ramirez <nramirez@nvidia.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> binding start issue.
> 
> Acked-by: Michael D Kinney <michael.d.kinney@intel.com>
> 
> > -----Original Message-----
> > From: Kasbekar, Saloni <saloni.kasbekar@intel.com>
> > Sent: Thursday, July 13, 2023 9:13 AM
> > To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io; Clark-
> > williams, Zachary <zachary.clark-williams@intel.com>; Kinney, Michael
> D
> > <michael.d.kinney@intel.com>
> > Cc: Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy
> <igork@ami.com>;
> > Nick Ramirez <nramirez@nvidia.com>
> > Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> > binding start issue.
> >
> > Mike,
> >
> > Would you be able to help us merge the patch?
> >
> > Thanks,
> > Saloni
> >
> > -----Original Message-----
> > From: Nickle Wang <nicklew@nvidia.com>
> > Sent: Wednesday, July 12, 2023 11:54 PM
> > To: devel@edk2.groups.io; Kasbekar, Saloni
> <saloni.kasbekar@intel.com>;
> > Clark-williams, Zachary <zachary.clark-williams@intel.com>
> > Cc: Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy
> <igork@ami.com>;
> > Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Saloni
> > > Kasbekar via groups.io
> > > Sent: Friday, June 30, 2023 6:28 AM
> > > To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io
> > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@intel.com>
> > >
> > > Thanks,
> > > Saloni
> > >
> > > -----Original Message-----
> > > From: Nickle Wang <nicklew@nvidia.com>
> > > Sent: Wednesday, June 28, 2023 3:30 PM
> > > To: Kasbekar, Saloni <saloni.kasbekar@intel.com>;
> devel@edk2.groups.io
> > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@intel.com>
> > > > Sent: Thursday, June 29, 2023 3:07 AM
> > > > To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > Nickle Wang via groups.io
> > > > Sent: Tuesday, June 27, 2023 5:56 PM
> > > > To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > > Nickle Wang via groups.io
> > > > > Sent: Friday, February 10, 2023 8:34 PM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@nvidia.com>
> > > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> > > > > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > > > > Cc: Abner Chang <abner.chang@amd.com>
> > > > > Cc: Igor Kulchytskyy <igork@ami.com>
> > > > > Cc: Nick Ramirez <nramirez@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 (#107080): https://edk2.groups.io/g/devel/message/107080
Mute This Topic: https://groups.io/mt/99821789/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.

[Ard Biesheuvel]

On Thu, 20 Jul 2023 at 03:41, Michael D Kinney
<michael.d.kinney@intel.com> wrote:
>
> Merged: https://github.com/tianocore/edk2/pull/4666
>


This commit is now merged as

commit 6510dcf6f71adbe282bff0ba2b236f1d074f819f
Author:     devel@edk2.groups.io <devel@edk2.groups.io>
AuthorDate: Fri Feb 10 04:34:03 2023 -0800
Commit:     mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
CommitDate: Thu Jul 20 01:41:02 2023 +0000

which is a bit unfortunate.

Given how pedantic our pre-merge CI is, could we perhaps also add a
check that the author field is sane? I don't think matching it to the
signoff makes sense in all cases so we could just check that it isn't
set to this particular value. (I could contribute code that I did not
author, and my signoff would amount to a statement on my part that
distributing the code as part of EDK2 complies with the code's license
terms and those of EDK2 itself) Alternatively, we might have a PR flag
that allows the maintainer to override this check manually if the
author field does not match the signoff, and match it strictly to the
[first] signoff line in other cases.

As for the committer field: it is quite unfortunate that we do not
keep track of who merged which PR. Could we do something about that?
The mergify bot is not a member of the project or a Tianocore
maintainer so it is not accountable, and its identity should not
obfuscate the identity of the person who decided to merge the PR.





> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Wednesday, July 19, 2023 5:27 PM
> > To: Kasbekar, Saloni <saloni.kasbekar@intel.com>; Nickle Wang
> > <nicklew@nvidia.com>; devel@edk2.groups.io; Clark-williams, Zachary
> > <zachary.clark-williams@intel.com>
> > Cc: Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy <igork@ami.com>;
> > Nick Ramirez <nramirez@nvidia.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> > binding start issue.
> >
> > Acked-by: Michael D Kinney <michael.d.kinney@intel.com>
> >
> > > -----Original Message-----
> > > From: Kasbekar, Saloni <saloni.kasbekar@intel.com>
> > > Sent: Thursday, July 13, 2023 9:13 AM
> > > To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io; Clark-
> > > williams, Zachary <zachary.clark-williams@intel.com>; Kinney, Michael
> > D
> > > <michael.d.kinney@intel.com>
> > > Cc: Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy
> > <igork@ami.com>;
> > > Nick Ramirez <nramirez@nvidia.com>
> > > Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> > > binding start issue.
> > >
> > > Mike,
> > >
> > > Would you be able to help us merge the patch?
> > >
> > > Thanks,
> > > Saloni
> > >
> > > -----Original Message-----
> > > From: Nickle Wang <nicklew@nvidia.com>
> > > Sent: Wednesday, July 12, 2023 11:54 PM
> > > To: devel@edk2.groups.io; Kasbekar, Saloni
> > <saloni.kasbekar@intel.com>;
> > > Clark-williams, Zachary <zachary.clark-williams@intel.com>
> > > Cc: Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy
> > <igork@ami.com>;
> > > Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > Saloni
> > > > Kasbekar via groups.io
> > > > Sent: Friday, June 30, 2023 6:28 AM
> > > > To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io
> > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@intel.com>
> > > >
> > > > Thanks,
> > > > Saloni
> > > >
> > > > -----Original Message-----
> > > > From: Nickle Wang <nicklew@nvidia.com>
> > > > Sent: Wednesday, June 28, 2023 3:30 PM
> > > > To: Kasbekar, Saloni <saloni.kasbekar@intel.com>;
> > devel@edk2.groups.io
> > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@intel.com>
> > > > > Sent: Thursday, June 29, 2023 3:07 AM
> > > > > To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> > > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > > Nickle Wang via groups.io
> > > > > Sent: Tuesday, June 27, 2023 5:56 PM
> > > > > To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> > > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > > > Nickle Wang via groups.io
> > > > > > Sent: Friday, February 10, 2023 8:34 PM
> > > > > > To: devel@edk2.groups.io
> > > > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > > > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > > > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@nvidia.com>
> > > > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> > > > > > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > > > > > Cc: Abner Chang <abner.chang@amd.com>
> > > > > > Cc: Igor Kulchytskyy <igork@ami.com>
> > > > > > Cc: Nick Ramirez <nramirez@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 (#107106): https://edk2.groups.io/g/devel/message/107106
Mute This Topic: https://groups.io/mt/99821789/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.

[Michael D Kinney]

Hi Ard,

This is my mistake.  I usually check the Author field when 
adding Rb/Ab tags because I have seen lots of cases where
the Author field needs updating as well.  I forgot to do this
step in this one.

I agree a CI check against the Author field being a valid
email address and not one of the mailing addresses would be
valuable.  Perhaps in PatchCheck.py?

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> Biesheuvel
> Sent: Thursday, July 20, 2023 9:42 AM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>; Leif Lindholm <quic_llindhol@quicinc.com>;
> Demeter, Miki <miki.demeter@intel.com>
> Cc: Kasbekar, Saloni <saloni.kasbekar@intel.com>; Nickle Wang
> <nicklew@nvidia.com>; Clark-williams, Zachary <zachary.clark-
> williams@intel.com>; Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy
> <igork@ami.com>; Nick Ramirez <nramirez@nvidia.com>
> Subject: Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> binding start issue.
> 
> On Thu, 20 Jul 2023 at 03:41, Michael D Kinney
> <michael.d.kinney@intel.com> wrote:
> >
> > Merged: https://github.com/tianocore/edk2/pull/4666
> >
> 
> 
> This commit is now merged as
> 
> commit 6510dcf6f71adbe282bff0ba2b236f1d074f819f
> Author:     devel@edk2.groups.io <devel@edk2.groups.io>
> AuthorDate: Fri Feb 10 04:34:03 2023 -0800
> Commit:     mergify[bot]
> <37929162+mergify[bot]@users.noreply.github.com>
> CommitDate: Thu Jul 20 01:41:02 2023 +0000
> 
> which is a bit unfortunate.
> 
> Given how pedantic our pre-merge CI is, could we perhaps also add a
> check that the author field is sane? I don't think matching it to the
> signoff makes sense in all cases so we could just check that it isn't
> set to this particular value. (I could contribute code that I did not
> author, and my signoff would amount to a statement on my part that
> distributing the code as part of EDK2 complies with the code's license
> terms and those of EDK2 itself) Alternatively, we might have a PR flag
> that allows the maintainer to override this check manually if the
> author field does not match the signoff, and match it strictly to the
> [first] signoff line in other cases.
> 
> As for the committer field: it is quite unfortunate that we do not
> keep track of who merged which PR. Could we do something about that?
> The mergify bot is not a member of the project or a Tianocore
> maintainer so it is not accountable, and its identity should not
> obfuscate the identity of the person who decided to merge the PR.
> 
> 
> 
> 
> 
> > > -----Original Message-----
> > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Sent: Wednesday, July 19, 2023 5:27 PM
> > > To: Kasbekar, Saloni <saloni.kasbekar@intel.com>; Nickle Wang
> > > <nicklew@nvidia.com>; devel@edk2.groups.io; Clark-williams, Zachary
> > > <zachary.clark-williams@intel.com>
> > > Cc: Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy
> <igork@ami.com>;
> > > Nick Ramirez <nramirez@nvidia.com>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> > > binding start issue.
> > >
> > > Acked-by: Michael D Kinney <michael.d.kinney@intel.com>
> > >
> > > > -----Original Message-----
> > > > From: Kasbekar, Saloni <saloni.kasbekar@intel.com>
> > > > Sent: Thursday, July 13, 2023 9:13 AM
> > > > To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io; Clark-
> > > > williams, Zachary <zachary.clark-williams@intel.com>; Kinney,
> Michael
> > > D
> > > > <michael.d.kinney@intel.com>
> > > > Cc: Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy
> > > <igork@ami.com>;
> > > > Nick Ramirez <nramirez@nvidia.com>
> > > > Subject: RE: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix
> driver
> > > > binding start issue.
> > > >
> > > > Mike,
> > > >
> > > > Would you be able to help us merge the patch?
> > > >
> > > > Thanks,
> > > > Saloni
> > > >
> > > > -----Original Message-----
> > > > From: Nickle Wang <nicklew@nvidia.com>
> > > > Sent: Wednesday, July 12, 2023 11:54 PM
> > > > To: devel@edk2.groups.io; Kasbekar, Saloni
> > > <saloni.kasbekar@intel.com>;
> > > > Clark-williams, Zachary <zachary.clark-williams@intel.com>
> > > > Cc: Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy
> > > <igork@ami.com>;
> > > > Nick Ramirez <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > Saloni
> > > > > Kasbekar via groups.io
> > > > > Sent: Friday, June 30, 2023 6:28 AM
> > > > > To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io
> > > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@intel.com>
> > > > >
> > > > > Thanks,
> > > > > Saloni
> > > > >
> > > > > -----Original Message-----
> > > > > From: Nickle Wang <nicklew@nvidia.com>
> > > > > Sent: Wednesday, June 28, 2023 3:30 PM
> > > > > To: Kasbekar, Saloni <saloni.kasbekar@intel.com>;
> > > devel@edk2.groups.io
> > > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > > > Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@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@intel.com>
> > > > > > Sent: Thursday, June 29, 2023 3:07 AM
> > > > > > To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> > > > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > > > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > > > > Kulchytskyy <igork@ami.com>; Nick Ramirez
> <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > > > Nickle Wang via groups.io
> > > > > > Sent: Tuesday, June 27, 2023 5:56 PM
> > > > > > To: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>
> > > > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > > > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>; Igor
> > > > > > Kulchytskyy <igork@ami.com>; Nick Ramirez
> <nramirez@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@edk2.groups.io <devel@edk2.groups.io> On Behalf
> Of
> > > > > > > Nickle Wang via groups.io
> > > > > > > Sent: Friday, February 10, 2023 8:34 PM
> > > > > > > To: devel@edk2.groups.io
> > > > > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Siyuan Fu
> > > > > > > <siyuan.fu@intel.com>; Abner Chang <abner.chang@amd.com>;
> Igor
> > > > > > > Kulchytskyy <igork@ami.com>; Nick Ramirez
> <nramirez@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@nvidia.com>
> > > > > > > Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> > > > > > > Cc: Siyuan Fu <siyuan.fu@intel.com>
> > > > > > > Cc: Abner Chang <abner.chang@amd.com>
> > > > > > > Cc: Igor Kulchytskyy <igork@ami.com>
> > > > > > > Cc: Nick Ramirez <nramirez@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 (#107107): https://edk2.groups.io/g/devel/message/107107
Mute This Topic: https://groups.io/mt/99821789/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.

[Ard Biesheuvel]

On Thu, 20 Jul 2023 at 19:10, Michael D Kinney
<michael.d.kinney@intel.com> wrote:
>
> Hi Ard,
>
> This is my mistake.  I usually check the Author field when
> adding Rb/Ab tags because I have seen lots of cases where
> the Author field needs updating as well.  I forgot to do this
> step in this one.
>
> I agree a CI check against the Author field being a valid
> email address and not one of the mailing addresses would be
> valuable.  Perhaps in PatchCheck.py?
>

Yeah, and actually, a test seems to exist already but it didn't catch
this particular case:

        if ' via Groups.Io' in name and mo.group(3).endswith('@groups.io'):
            self.error("Email rewritten by lists DMARC / DKIM / SPF: " +
                       email)

I suppose replacing the 'and' with 'or' would be sufficient here? I
don't think @groups.io is ever a valid author email domain.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107108): https://edk2.groups.io/g/devel/message/107108
Mute This Topic: https://groups.io/mt/99821789/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver binding start issue.

[Michael D Kinney]

Hi Ard,

I did a search for invalid tags in commit messages.  Here is what I suggest as 
a change to PatchCheck.py to catch the cases I observe that includes the fix
the to logic you suggested to find email addresses from edk2 groups/io mailing
lists.

=============================
diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index 900226f18fe5..1c432f9d5688 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -54,36 +54,55 @@ class EmailAddressCheck:
             print(prefix, line)
             count += 1

-    email_re1 = re.compile(r'(?:\s*)(.*?)(\s*)<(.+)>\s*$',
+    email_re1 = re.compile(r'(?:\s*)(.*?)(\s*)<(.+)>\s*(.*?)$',
                            re.MULTILINE|re.IGNORECASE)

     def check_email_address(self, email):
         email = email.strip()
         mo = self.email_re1.match(email)
+
+        # Check for too few fields
         if mo is None:
-            self.error("Email format is invalid: " + email.strip())
+            self.error("Email format is invalid: " + email)
+            return
+        if len (mo.groups()) < 3:
+            self.error("Email format is missing fields: " + email)
             return

+        # Check the name
         name = mo.group(1).strip()
-        if name == '':
-            self.error("Name is not provided with email address: " +
-                       email)
-        else:
-            quoted = len(name) > 2 and name[0] == '"' and name[-1] == '"'
-            if name.find(',') >= 0 and not quoted:
-                self.error('Add quotes (") around name with a comma: ' +
-                           name)
+        if ',' in name:
+            self.error('Change order of name and remove comma: ' + name)
+        if '"' in name:
+            self.error('Remove double quotes around name: ' + name)
+        if ' via ' in name:
+            self.error('Remove via from another source from name: ' + name)

-        if mo.group(2) == '':
-            self.error("There should be a space between the name and " +
+        # Check whitespace between name and email address
+        if len(mo.group(2)) != mo.group(2).count(' '):
+            self.error("There should only be spaces between the name and " +
                        "email address: " + email)

-        if mo.group(3).find(' ') >= 0:
-            self.error("The email address cannot contain a space: " +
+        # Check the email address
+        if mo.group(3).count(' ') >= 1:
+            self.error("The email address must not contain any spaces: " +
                        mo.group(3))

-        if ' via Groups.Io' in name and mo.group(3).endswith('@groups.io'):
-            self.error("Email rewritten by lists DMARC / DKIM / SPF: " +
+        if mo.group(3).count('=') >= 1:
+            self.error("The email address must not contain any '=': " +
+                       mo.group(3))
+
+        if mo.group(3).count('@') != 1:
+            self.error("The email address must contain a single @: " +
+                       mo.group(3))
+
+        if '@edk2.groups.io' in mo.group(3):
+            self.error("The email address cannot be from @edk2.groups.io: " +
+                       mo.group(3))
+
+        # Check for too many fields
+        if len(mo.groups()) > 3 and len(mo.group(4)) > 0:
+            self.error("Email format has too many fields: " +
                        email)
=============================

With this change I looked at the last 500 commits to edk2 repo and it finds
the following issues:

 * Remove double quotes around name: "devel@edk2.groups.io"
 * The email address cannot be from @edk2.groups.io: devel@edk2.groups.io
 * Change order of name and remove comma: "Xie, Yuanhao"
 * Remove double quotes around name: "Xie, Yuanhao"
 * Change order of name and remove comma: "Xie, Yuanhao"
 * Remove double quotes around name: "Xie, Yuanhao"
 * Change order of name and remove comma: "Xie, Yuanhao"
 * Remove double quotes around name: "Xie, Yuanhao"
 * Change order of name and remove comma: "Xie, Yuanhao"
 * Remove double quotes around name: "Xie, Yuanhao"
 * Change order of name and remove comma: "Xie, Yuanhao"
 * Remove double quotes around name: "Xie, Yuanhao"
 * Change order of name and remove comma: "Zhang, Hongbin1"
 * Remove double quotes around name: "Zhang, Hongbin1"
 * Remove double quotes around name: "devel@edk2.groups.io"
 * The email address cannot be from @edk2.groups.io: devel@edk2.groups.io
 * Change order of name and remove comma: "Aishwarya, KurugoduMelmatamX"
 * Remove double quotes around name: "Aishwarya, KurugoduMelmatamX"
 * Email format is invalid: Jiewen.yao@intel.com
 * Email format has too many fields: Jiewen Yao <jiewen.yao@intel.com> [jyao1]
 * Email format has too many fields: Yi Li <yi1.li@intel.com> [liyi77]
 * Email format has too many fields: Xiaoyu Lu <xiaoyu1.lu@intel.com> [xiaoyuxlu]
 * Email format has too many fields: Guomin Jiang <guomin.jiang@intel.com> [guominjia]
 * Change order of name and remove comma: "Ni, Ray"
 * Remove double quotes around name: "Ni, Ray"
 * Remove double quotes around name: "Mikolaj Lisik via groups.io"
 * Remove via from another source from name: "Mikolaj Lisik via groups.io"
 * The email address must not contain any '=': lisik=google.com@groups.io
 * Change order of name and remove comma: "Kinney, Michael D"
 * Remove double quotes around name: "Kinney, Michael D"
 * Change order of name and remove comma: "Liu, Zhiguang"
 * Remove double quotes around name: "Liu, Zhiguang"
 * Change order of name and remove comma: "Liu, Zhiguang"
 * Remove double quotes around name: "Liu, Zhiguang"
 * Change order of name and remove comma: "Liu, Zhiguang"
 * Remove double quotes around name: "Liu, Zhiguang"
 * Change order of name and remove comma: "Liu, Zhiguang"
 * Remove double quotes around name: "Liu, Zhiguang"
 * Change order of name and remove comma: "Tan, Ming"
 * Remove double quotes around name: "Tan, Ming"
 * The email address must not contain any spaces: igork @ami.com
 * The email address must not contain any spaces: igork @ami.com
 * The email address must not contain any spaces: igork @ami.com
 * The email address must not contain any spaces: igork @ami.com
 * The email address must not contain any spaces: igork @ami.com
 * Change order of name and remove comma: "Roth, Michael via groups.io"
 * Remove double quotes around name: "Roth, Michael via groups.io"
 * Remove via from another source from name: "Roth, Michael via groups.io"
 * The email address must not contain any '=': Michael.Roth=amd.com@groups.io
 * Change order of name and remove comma: "Roth, Michael via groups.io"
 * Remove double quotes around name: "Roth, Michael via groups.io"
 * Remove via from another source from name: "Roth, Michael via groups.io"
 * The email address must not contain any '=': Michael.Roth=amd.com@groups.io
 * Change order of name and remove comma: "Roth, Michael via groups.io"
 * Remove double quotes around name: "Roth, Michael via groups.io"
 * Remove via from another source from name: "Roth, Michael via groups.io"
 * The email address must not contain any '=': Michael.Roth=amd.com@groups.io
 * Change order of name and remove comma: "Duggapu, Chinni B"
 * Remove double quotes around name: "Duggapu, Chinni B"
 * Remove double quotes around name: "devel@edk2.groups.io"
 * The email address cannot be from @edk2.groups.io: devel@edk2.groups.io
 * Change order of name and remove comma: "Lin, MillerX"
 * Remove double quotes around name: "Lin, MillerX"
 * Change order of name and remove comma: "Lendacky, Thomas via groups.io"
 * Remove double quotes around name: "Lendacky, Thomas via groups.io"
 * Remove via from another source from name: "Lendacky, Thomas via groups.io"
 * The email address must not contain any '=': thomas.lendacky=amd.com@groups.io
 * Change order of name and remove comma: "Lendacky, Thomas via groups.io"
 * Remove double quotes around name: "Lendacky, Thomas via groups.io"
 * Remove via from another source from name: "Lendacky, Thomas via groups.io"
 * The email address must not contain any '=': thomas.lendacky=amd.com@groups.io
 * Change order of name and remove comma: "Albecki, Mateusz"
 * Remove double quotes around name: "Albecki, Mateusz"
 * Change order of name and remove comma: "Xie, Yuanhao"
 * Remove double quotes around name: "Xie, Yuanhao"

Let me know if you think this is too strict.

Mike

> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Thursday, July 20, 2023 10:19 AM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>; Demeter, Miki
> <miki.demeter@intel.com>; Kasbekar, Saloni <saloni.kasbekar@intel.com>;
> Nickle Wang <nicklew@nvidia.com>; Clark-williams, Zachary <zachary.clark-
> williams@intel.com>; Abner Chang <abner.chang@amd.com>; Igor Kulchytskyy
> <igork@ami.com>; Nick Ramirez <nramirez@nvidia.com>
> Subject: Re: [edk2-devel] [PATCH 2/2] NetworkPkg/HttpDxe: fix driver
> binding start issue.
> 
> On Thu, 20 Jul 2023 at 19:10, Michael D Kinney
> <michael.d.kinney@intel.com> wrote:
> >
> > Hi Ard,
> >
> > This is my mistake.  I usually check the Author field when
> > adding Rb/Ab tags because I have seen lots of cases where
> > the Author field needs updating as well.  I forgot to do this
> > step in this one.
> >
> > I agree a CI check against the Author field being a valid
> > email address and not one of the mailing addresses would be
> > valuable.  Perhaps in PatchCheck.py?
> >
> 
> Yeah, and actually, a test seems to exist already but it didn't catch
> this particular case:
> 
>         if ' via Groups.Io' in name and
> mo.group(3).endswith('@groups.io'):
>             self.error("Email rewritten by lists DMARC / DKIM / SPF: " +
>                        email)
> 
> I suppose replacing the 'and' with 'or' would be sufficient here? I
> don't think @groups.io is ever a valid author email domain.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107191): https://edk2.groups.io/g/devel/message/107191
Mute This Topic: https://groups.io/mt/99821789/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-