public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] Pkcs7VerifyDxe: Don't allow Pkcs7Verify to install protocols twice.
@ 2016-09-29 15:59 Peter Jones
  2016-09-29 16:33 ` Kinney, Michael D
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Jones @ 2016-09-29 15:59 UTC (permalink / raw)
  To: edk2-devel; +Cc: Peter Jones

This patch makes Pkcs7VerifyDxe check that it has not already been
installed before installing its protocols.  This prevents the case where
loading it as an external driver (either manually, through Driver####
variables, etc.) will refuse to add a second provider of the API.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Peter Jones <pjones@redhat.com>
---
 SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
index 07fdf55..3389af4 100644
--- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
+++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
@@ -1030,8 +1030,16 @@ Pkcs7VerifyDriverEntry (
   IN EFI_SYSTEM_TABLE    *SystemTable
   )
 {
-  EFI_STATUS    Status;
-  EFI_HANDLE    Handle;
+  EFI_STATUS                Status;
+  EFI_HANDLE                Handle;
+  EFI_PKCS7_VERIFY_PROTOCOL Useless;
+
+  //
+  // Avoid loading a second copy if this is built as an external module.
+  //
+  Status = gBS->LocateProtocol (&gEfiPkcs7VerifyProtocolGuid, NULL, (VOID **) &Useless);
+  if (Status != EFI_NOT_FOUND)
+	  return EFI_SUCCESS;
 
   //
   // Install UEFI Pkcs7 Verification Protocol
-- 
2.10.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Pkcs7VerifyDxe: Don't allow Pkcs7Verify to install protocols twice.
  2016-09-29 15:59 [PATCH] Pkcs7VerifyDxe: Don't allow Pkcs7Verify to install protocols twice Peter Jones
@ 2016-09-29 16:33 ` Kinney, Michael D
  2016-09-29 17:45   ` Peter Jones
  0 siblings, 1 reply; 8+ messages in thread
From: Kinney, Michael D @ 2016-09-29 16:33 UTC (permalink / raw)
  To: Peter Jones, edk2-devel@ml01.01.org, Kinney, Michael D

Hi Peter,

If the protocol is already installed, you want to return an error
so the PE/COFF image for the driver will be unloaded.  Following the
example from Section 7.8 of UEFI Driver Writer's Guide an error code 
of EFI_ABORTED can be used for this case.

  Status = gBS->LocateProtocol (&gEfiPkcs7VerifyProtocolGuid, NULL, (VOID **)&Useless);
  if (!EFI_ERROR (Status)) {
    return EFI_ABORTED;
  }

Best regards,

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Peter Jones
> Sent: Thursday, September 29, 2016 8:59 AM
> To: edk2-devel@ml01.01.org
> Cc: Peter Jones <pjones@redhat.com>
> Subject: [edk2] [PATCH] Pkcs7VerifyDxe: Don't allow Pkcs7Verify to install protocols
> twice.
> 
> This patch makes Pkcs7VerifyDxe check that it has not already been
> installed before installing its protocols.  This prevents the case where
> loading it as an external driver (either manually, through Driver####
> variables, etc.) will refuse to add a second provider of the API.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Peter Jones <pjones@redhat.com>
> ---
>  SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> index 07fdf55..3389af4 100644
> --- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> +++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> @@ -1030,8 +1030,16 @@ Pkcs7VerifyDriverEntry (
>    IN EFI_SYSTEM_TABLE    *SystemTable
>    )
>  {
> -  EFI_STATUS    Status;
> -  EFI_HANDLE    Handle;
> +  EFI_STATUS                Status;
> +  EFI_HANDLE                Handle;
> +  EFI_PKCS7_VERIFY_PROTOCOL Useless;
> +
> +  //
> +  // Avoid loading a second copy if this is built as an external module.
> +  //
> +  Status = gBS->LocateProtocol (&gEfiPkcs7VerifyProtocolGuid, NULL, (VOID **)
> &Useless);
> +  if (Status != EFI_NOT_FOUND)
> +	  return EFI_SUCCESS;> 
>    //
>    // Install UEFI Pkcs7 Verification Protocol
> --
> 2.10.0
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] Pkcs7VerifyDxe: Don't allow Pkcs7Verify to install protocols twice.
  2016-09-29 16:33 ` Kinney, Michael D
@ 2016-09-29 17:45   ` Peter Jones
  2016-09-29 18:38     ` Kinney, Michael D
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Jones @ 2016-09-29 17:45 UTC (permalink / raw)
  To: edk2-devel; +Cc: Peter Jones

This patch makes Pkcs7VerifyDxe check that it has not already been
installed before installing its protocols.  This prevents the case where
loading it as an external driver (either manually, through Driver####
variables, etc.) will refuse to add a second provider of the API.

v2 - return EFI_ABORTED as per Michael Kinney's feedback.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Peter Jones <pjones@redhat.com>
---
 SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
index 07fdf55..7927c58 100644
--- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
+++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
@@ -1030,8 +1030,16 @@ Pkcs7VerifyDriverEntry (
   IN EFI_SYSTEM_TABLE    *SystemTable
   )
 {
-  EFI_STATUS    Status;
-  EFI_HANDLE    Handle;
+  EFI_STATUS                Status;
+  EFI_HANDLE                Handle;
+  EFI_PKCS7_VERIFY_PROTOCOL Useless;
+
+  //
+  // Avoid loading a second copy if this is built as an external module.
+  //
+  Status = gBS->LocateProtocol (&gEfiPkcs7VerifyProtocolGuid, NULL, (VOID **) &Useless);
+  if (Status != EFI_NOT_FOUND)
+	  return EFI_ABORTED;
 
   //
   // Install UEFI Pkcs7 Verification Protocol
-- 
2.10.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Pkcs7VerifyDxe: Don't allow Pkcs7Verify to install protocols twice.
  2016-09-29 17:45   ` Peter Jones
@ 2016-09-29 18:38     ` Kinney, Michael D
  2016-09-29 18:39       ` Peter Jones
  2016-09-29 18:45       ` Peter Jones
  0 siblings, 2 replies; 8+ messages in thread
From: Kinney, Michael D @ 2016-09-29 18:38 UTC (permalink / raw)
  To: Peter Jones, edk2-devel@ml01.01.org, Kinney, Michael D

Peter,

Please use this form in your patch.  The UEFI Spec does allow other error codes than
those listed in the API to be returned.  Using !EFI_ERROR (Status) is safer.  EDK II
Coding Style also requires {} for if statements.

  Status = gBS->LocateProtocol (&gEfiPkcs7VerifyProtocolGuid, NULL, (VOID **)&Useless);
  if (!EFI_ERROR (Status)) {
    return EFI_ABORTED;
  }

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Peter Jones
> Sent: Thursday, September 29, 2016 10:45 AM
> To: edk2-devel@ml01.01.org
> Cc: Peter Jones <pjones@redhat.com>
> Subject: [edk2] [PATCH] Pkcs7VerifyDxe: Don't allow Pkcs7Verify to install protocols
> twice.
> 
> This patch makes Pkcs7VerifyDxe check that it has not already been
> installed before installing its protocols.  This prevents the case where
> loading it as an external driver (either manually, through Driver####
> variables, etc.) will refuse to add a second provider of the API.
> 
> v2 - return EFI_ABORTED as per Michael Kinney's feedback.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Peter Jones <pjones@redhat.com>
> ---
>  SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> index 07fdf55..7927c58 100644
> --- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> +++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> @@ -1030,8 +1030,16 @@ Pkcs7VerifyDriverEntry (
>    IN EFI_SYSTEM_TABLE    *SystemTable
>    )
>  {
> -  EFI_STATUS    Status;
> -  EFI_HANDLE    Handle;
> +  EFI_STATUS                Status;
> +  EFI_HANDLE                Handle;
> +  EFI_PKCS7_VERIFY_PROTOCOL Useless;
> +
> +  //
> +  // Avoid loading a second copy if this is built as an external module.
> +  //
> +  Status = gBS->LocateProtocol (&gEfiPkcs7VerifyProtocolGuid, NULL, (VOID **)
> &Useless);
> +  if (Status != EFI_NOT_FOUND)
> +	  return EFI_ABORTED;
> 
>    //
>    // Install UEFI Pkcs7 Verification Protocol
> --
> 2.10.0
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Pkcs7VerifyDxe: Don't allow Pkcs7Verify to install protocols twice.
  2016-09-29 18:38     ` Kinney, Michael D
@ 2016-09-29 18:39       ` Peter Jones
  2016-09-29 18:45       ` Peter Jones
  1 sibling, 0 replies; 8+ messages in thread
From: Peter Jones @ 2016-09-29 18:39 UTC (permalink / raw)
  To: Kinney, Michael D; +Cc: edk2-devel@ml01.01.org

On Thu, Sep 29, 2016 at 06:38:17PM +0000, Kinney, Michael D wrote:
> Peter,
> 
> Please use this form in your patch.  The UEFI Spec does allow other error codes than
> those listed in the API to be returned.  Using !EFI_ERROR (Status) is safer.  EDK II
> Coding Style also requires {} for if statements.

Apologies; obviously you were also saying this in your previous reply,
which I read too quickly during a meeting.  Update to follow.

Thanks.
-- 
  Peter


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] Pkcs7VerifyDxe: Don't allow Pkcs7Verify to install protocols twice.
  2016-09-29 18:38     ` Kinney, Michael D
  2016-09-29 18:39       ` Peter Jones
@ 2016-09-29 18:45       ` Peter Jones
  2016-09-29 18:48         ` Kinney, Michael D
  1 sibling, 1 reply; 8+ messages in thread
From: Peter Jones @ 2016-09-29 18:45 UTC (permalink / raw)
  To: edk2-devel; +Cc: Peter Jones

This patch makes Pkcs7VerifyDxe check that it has not already been
installed before installing its protocols.  This prevents the case where
loading it as an external driver (either manually, through Driver####
variables, etc.) will refuse to add a second provider of the API.

v2 - return EFI_ABORTED as per Michael Kinney's feedback.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Peter Jones <pjones@redhat.com>
---
 SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
index 07fdf55..e5ec1b4 100644
--- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
+++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
@@ -1030,8 +1030,17 @@ Pkcs7VerifyDriverEntry (
   IN EFI_SYSTEM_TABLE    *SystemTable
   )
 {
-  EFI_STATUS    Status;
-  EFI_HANDLE    Handle;
+  EFI_STATUS                Status;
+  EFI_HANDLE                Handle;
+  EFI_PKCS7_VERIFY_PROTOCOL Useless;
+
+  //
+  // Avoid loading a second copy if this is built as an external module.
+  //
+  Status = gBS->LocateProtocol (&gEfiPkcs7VerifyProtocolGuid, NULL, (VOID **) &Useless);
+  if (!EFI_ERROR (Status)) {
+    return EFI_ABORTED;
+  }
 
   //
   // Install UEFI Pkcs7 Verification Protocol
-- 
2.10.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Pkcs7VerifyDxe: Don't allow Pkcs7Verify to install protocols twice.
  2016-09-29 18:45       ` Peter Jones
@ 2016-09-29 18:48         ` Kinney, Michael D
  2016-09-30  1:06           ` Long, Qin
  0 siblings, 1 reply; 8+ messages in thread
From: Kinney, Michael D @ 2016-09-29 18:48 UTC (permalink / raw)
  To: Peter Jones, edk2-devel@ml01.01.org, Zhang, Chao B,
	Kinney, Michael D

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

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Peter Jones
> Sent: Thursday, September 29, 2016 11:46 AM
> To: edk2-devel@ml01.01.org
> Cc: Peter Jones <pjones@redhat.com>
> Subject: [edk2] [PATCH] Pkcs7VerifyDxe: Don't allow Pkcs7Verify to install protocols
> twice.
> 
> This patch makes Pkcs7VerifyDxe check that it has not already been
> installed before installing its protocols.  This prevents the case where
> loading it as an external driver (either manually, through Driver####
> variables, etc.) will refuse to add a second provider of the API.
> 
> v2 - return EFI_ABORTED as per Michael Kinney's feedback.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Peter Jones <pjones@redhat.com>
> ---
>  SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> index 07fdf55..e5ec1b4 100644
> --- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> +++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> @@ -1030,8 +1030,17 @@ Pkcs7VerifyDriverEntry (
>    IN EFI_SYSTEM_TABLE    *SystemTable
>    )
>  {
> -  EFI_STATUS    Status;
> -  EFI_HANDLE    Handle;
> +  EFI_STATUS                Status;
> +  EFI_HANDLE                Handle;
> +  EFI_PKCS7_VERIFY_PROTOCOL Useless;
> +
> +  //
> +  // Avoid loading a second copy if this is built as an external module.
> +  //
> +  Status = gBS->LocateProtocol (&gEfiPkcs7VerifyProtocolGuid, NULL, (VOID **)
> &Useless);
> +  if (!EFI_ERROR (Status)) {
> +    return EFI_ABORTED;
> +  }
> 
>    //
>    // Install UEFI Pkcs7 Verification Protocol
> --
> 2.10.0
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Pkcs7VerifyDxe: Don't allow Pkcs7Verify to install protocols twice.
  2016-09-29 18:48         ` Kinney, Michael D
@ 2016-09-30  1:06           ` Long, Qin
  0 siblings, 0 replies; 8+ messages in thread
From: Long, Qin @ 2016-09-30  1:06 UTC (permalink / raw)
  To: Kinney, Michael D, Peter Jones, edk2-devel@ml01.01.org,
	Zhang, Chao B, Kinney, Michael D

The update looks good to me. 

Reviewed-by: Qin Long <qin.long@intel.com>



> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Kinney, Michael D
> Sent: Friday, September 30, 2016 2:48 AM
> To: Peter Jones <pjones@redhat.com>; edk2-devel@ml01.01.org; Zhang,
> Chao B <chao.b.zhang@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: Re: [edk2] [PATCH] Pkcs7VerifyDxe: Don't allow Pkcs7Verify to
> install protocols twice.
> 
> Reviewed-by: Michael Kinney <michael.d.kinney@intel.com>
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Peter Jones
> > Sent: Thursday, September 29, 2016 11:46 AM
> > To: edk2-devel@ml01.01.org
> > Cc: Peter Jones <pjones@redhat.com>
> > Subject: [edk2] [PATCH] Pkcs7VerifyDxe: Don't allow Pkcs7Verify to
> > install protocols twice.
> >
> > This patch makes Pkcs7VerifyDxe check that it has not already been
> > installed before installing its protocols.  This prevents the case
> > where loading it as an external driver (either manually, through
> > Driver#### variables, etc.) will refuse to add a second provider of the API.
> >
> > v2 - return EFI_ABORTED as per Michael Kinney's feedback.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Peter Jones <pjones@redhat.com>
> > ---
> >  SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c | 13
> > +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> > b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> > index 07fdf55..e5ec1b4 100644
> > --- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> > +++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
> > @@ -1030,8 +1030,17 @@ Pkcs7VerifyDriverEntry (
> >    IN EFI_SYSTEM_TABLE    *SystemTable
> >    )
> >  {
> > -  EFI_STATUS    Status;
> > -  EFI_HANDLE    Handle;
> > +  EFI_STATUS                Status;
> > +  EFI_HANDLE                Handle;
> > +  EFI_PKCS7_VERIFY_PROTOCOL Useless;
> > +
> > +  //
> > +  // Avoid loading a second copy if this is built as an external module.
> > +  //
> > +  Status = gBS->LocateProtocol (&gEfiPkcs7VerifyProtocolGuid, NULL,
> > + (VOID **)
> > &Useless);
> > +  if (!EFI_ERROR (Status)) {
> > +    return EFI_ABORTED;
> > +  }
> >
> >    //
> >    // Install UEFI Pkcs7 Verification Protocol
> > --
> > 2.10.0
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-09-30  1:06 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-29 15:59 [PATCH] Pkcs7VerifyDxe: Don't allow Pkcs7Verify to install protocols twice Peter Jones
2016-09-29 16:33 ` Kinney, Michael D
2016-09-29 17:45   ` Peter Jones
2016-09-29 18:38     ` Kinney, Michael D
2016-09-29 18:39       ` Peter Jones
2016-09-29 18:45       ` Peter Jones
2016-09-29 18:48         ` Kinney, Michael D
2016-09-30  1:06           ` Long, Qin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox