[edk2-devel] [PATCH 26/35] SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls

Laszlo Ersek posted 35 patches 6 years, 4 months ago
[edk2-devel] [PATCH 26/35] SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls
Posted by Laszlo Ersek 6 years, 4 months ago
Unlike the InstallMultipleProtocolInterfaces() boot service, which takes
an (EFI_HANDLE*) as first parameter, the
UninstallMultipleProtocolInterfaces() boot service takes an EFI_HANDLE as
first parameter.

These are actual bugs. They must have remained hidden until now because
they are all in Unload() functions, which are probably exercised
infrequently. Fix the UninstallMultipleProtocolInterfaces() calls.

Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jian Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    build-tested only

 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c                                  | 2 +-
 SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c                                 | 2 +-
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
index 54155a338100..9052eced757d 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
@@ -443,7 +443,7 @@ Tcg2ConfigDriverUnload (
   ASSERT (PrivateData->Signature == TCG2_CONFIG_PRIVATE_DATA_SIGNATURE);
 
   gBS->UninstallMultipleProtocolInterfaces (
-         &ImageHandle,
+         ImageHandle,
          &gEfiCallerIdGuid,
          PrivateData,
          NULL
diff --git a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c b/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
index 341879e4c4ba..fb06624fdb8f 100644
--- a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
+++ b/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
@@ -138,7 +138,7 @@ TcgConfigDriverUnload (
   ASSERT (PrivateData->Signature == TCG_CONFIG_PRIVATE_DATA_SIGNATURE);
 
   gBS->UninstallMultipleProtocolInterfaces (
-         &ImageHandle,
+         ImageHandle,
          &gEfiCallerIdGuid,
          PrivateData,
          NULL
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c
index 798ef9cfbc01..6c0294151e6c 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c
@@ -115,7 +115,7 @@ SecureBootConfigDriverUnload (
   ASSERT (PrivateData->Signature == SECUREBOOT_CONFIG_PRIVATE_DATA_SIGNATURE);
 
   gBS->UninstallMultipleProtocolInterfaces (
-         &ImageHandle,
+         ImageHandle,
          &gEfiCallerIdGuid,
          PrivateData,
          NULL
-- 
2.19.1.3.g30247aa5d201



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47413): https://edk2.groups.io/g/devel/message/47413
Mute This Topic: https://groups.io/mt/34180228/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 26/35] SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls
Posted by Philippe Mathieu-Daudé 6 years, 4 months ago
On 9/17/19 9:49 PM, Laszlo Ersek wrote:
> Unlike the InstallMultipleProtocolInterfaces() boot service, which takes
> an (EFI_HANDLE*) as first parameter, the
> UninstallMultipleProtocolInterfaces() boot service takes an EFI_HANDLE as
> first parameter.
> 
> These are actual bugs. They must have remained hidden until now because
> they are all in Unload() functions, which are probably exercised
> infrequently. Fix the UninstallMultipleProtocolInterfaces() calls.
> 
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Jian Wang <jian.j.wang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     build-tested only
> 
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c                                  | 2 +-
>  SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c                                 | 2 +-
>  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> index 54155a338100..9052eced757d 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> @@ -443,7 +443,7 @@ Tcg2ConfigDriverUnload (
>    ASSERT (PrivateData->Signature == TCG2_CONFIG_PRIVATE_DATA_SIGNATURE);
>  
>    gBS->UninstallMultipleProtocolInterfaces (
> -         &ImageHandle,
> +         ImageHandle,
>           &gEfiCallerIdGuid,
>           PrivateData,
>           NULL
> diff --git a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c b/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
> index 341879e4c4ba..fb06624fdb8f 100644
> --- a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
> +++ b/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
> @@ -138,7 +138,7 @@ TcgConfigDriverUnload (
>    ASSERT (PrivateData->Signature == TCG_CONFIG_PRIVATE_DATA_SIGNATURE);
>  
>    gBS->UninstallMultipleProtocolInterfaces (
> -         &ImageHandle,
> +         ImageHandle,
>           &gEfiCallerIdGuid,
>           PrivateData,
>           NULL
> diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c
> index 798ef9cfbc01..6c0294151e6c 100644
> --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c
> +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c
> @@ -115,7 +115,7 @@ SecureBootConfigDriverUnload (
>    ASSERT (PrivateData->Signature == SECUREBOOT_CONFIG_PRIVATE_DATA_SIGNATURE);
>  
>    gBS->UninstallMultipleProtocolInterfaces (
> -         &ImageHandle,
> +         ImageHandle,
>           &gEfiCallerIdGuid,
>           PrivateData,
>           NULL
> 

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47844): https://edk2.groups.io/g/devel/message/47844
Mute This Topic: https://groups.io/mt/34180228/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 26/35] SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls
Posted by Laszlo Ersek 6 years, 4 months ago
Chao, Jian, Jiewen,

can you please review this patch?

Thanks,
Laszlo

On 09/17/19 21:49, Laszlo Ersek wrote:
> Unlike the InstallMultipleProtocolInterfaces() boot service, which takes
> an (EFI_HANDLE*) as first parameter, the
> UninstallMultipleProtocolInterfaces() boot service takes an EFI_HANDLE as
> first parameter.
> 
> These are actual bugs. They must have remained hidden until now because
> they are all in Unload() functions, which are probably exercised
> infrequently. Fix the UninstallMultipleProtocolInterfaces() calls.
> 
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Jian Wang <jian.j.wang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     build-tested only
> 
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c                                  | 2 +-
>  SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c                                 | 2 +-
>  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> index 54155a338100..9052eced757d 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> @@ -443,7 +443,7 @@ Tcg2ConfigDriverUnload (
>    ASSERT (PrivateData->Signature == TCG2_CONFIG_PRIVATE_DATA_SIGNATURE);
>  
>    gBS->UninstallMultipleProtocolInterfaces (
> -         &ImageHandle,
> +         ImageHandle,
>           &gEfiCallerIdGuid,
>           PrivateData,
>           NULL
> diff --git a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c b/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
> index 341879e4c4ba..fb06624fdb8f 100644
> --- a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
> +++ b/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
> @@ -138,7 +138,7 @@ TcgConfigDriverUnload (
>    ASSERT (PrivateData->Signature == TCG_CONFIG_PRIVATE_DATA_SIGNATURE);
>  
>    gBS->UninstallMultipleProtocolInterfaces (
> -         &ImageHandle,
> +         ImageHandle,
>           &gEfiCallerIdGuid,
>           PrivateData,
>           NULL
> diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c
> index 798ef9cfbc01..6c0294151e6c 100644
> --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c
> +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c
> @@ -115,7 +115,7 @@ SecureBootConfigDriverUnload (
>    ASSERT (PrivateData->Signature == SECUREBOOT_CONFIG_PRIVATE_DATA_SIGNATURE);
>  
>    gBS->UninstallMultipleProtocolInterfaces (
> -         &ImageHandle,
> +         ImageHandle,
>           &gEfiCallerIdGuid,
>           PrivateData,
>           NULL
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#48104): https://edk2.groups.io/g/devel/message/48104
Mute This Topic: https://groups.io/mt/34180228/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 26/35] SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls
Posted by Laszlo Ersek 6 years, 4 months ago
Pinging SecurityPkg maintainers again, for reviewing this patch.

Thanks
Laszlo

On 09/26/19 14:45, Laszlo Ersek wrote:
> Chao, Jian, Jiewen,
> 
> can you please review this patch?
> 
> Thanks,
> Laszlo
> 
> On 09/17/19 21:49, Laszlo Ersek wrote:
>> Unlike the InstallMultipleProtocolInterfaces() boot service, which takes
>> an (EFI_HANDLE*) as first parameter, the
>> UninstallMultipleProtocolInterfaces() boot service takes an EFI_HANDLE as
>> first parameter.
>>
>> These are actual bugs. They must have remained hidden until now because
>> they are all in Unload() functions, which are probably exercised
>> infrequently. Fix the UninstallMultipleProtocolInterfaces() calls.
>>
>> Cc: Chao Zhang <chao.b.zhang@intel.com>
>> Cc: Jian Wang <jian.j.wang@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>>     build-tested only
>>
>>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c                                  | 2 +-
>>  SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c                                 | 2 +-
>>  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c | 2 +-
>>  3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
>> index 54155a338100..9052eced757d 100644
>> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
>> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
>> @@ -443,7 +443,7 @@ Tcg2ConfigDriverUnload (
>>    ASSERT (PrivateData->Signature == TCG2_CONFIG_PRIVATE_DATA_SIGNATURE);
>>  
>>    gBS->UninstallMultipleProtocolInterfaces (
>> -         &ImageHandle,
>> +         ImageHandle,
>>           &gEfiCallerIdGuid,
>>           PrivateData,
>>           NULL
>> diff --git a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c b/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
>> index 341879e4c4ba..fb06624fdb8f 100644
>> --- a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
>> +++ b/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
>> @@ -138,7 +138,7 @@ TcgConfigDriverUnload (
>>    ASSERT (PrivateData->Signature == TCG_CONFIG_PRIVATE_DATA_SIGNATURE);
>>  
>>    gBS->UninstallMultipleProtocolInterfaces (
>> -         &ImageHandle,
>> +         ImageHandle,
>>           &gEfiCallerIdGuid,
>>           PrivateData,
>>           NULL
>> diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c
>> index 798ef9cfbc01..6c0294151e6c 100644
>> --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c
>> +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDriver.c
>> @@ -115,7 +115,7 @@ SecureBootConfigDriverUnload (
>>    ASSERT (PrivateData->Signature == SECUREBOOT_CONFIG_PRIVATE_DATA_SIGNATURE);
>>  
>>    gBS->UninstallMultipleProtocolInterfaces (
>> -         &ImageHandle,
>> +         ImageHandle,
>>           &gEfiCallerIdGuid,
>>           PrivateData,
>>           NULL
>>
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#48431): https://edk2.groups.io/g/devel/message/48431
Mute This Topic: https://groups.io/mt/34180228/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 26/35] SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls
Posted by Yao, Jiewen 6 years, 4 months ago
Good catch!

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Thursday, October 3, 2019 7:07 PM
> To: Zhang, Chao B <chao.b.zhang@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: edk2-devel-groups-io <devel@edk2.groups.io>
> Subject: Re: [edk2-devel] [PATCH 26/35] SecurityPkg: fix
> UninstallMultipleProtocolInterfaces() calls
> 
> Pinging SecurityPkg maintainers again, for reviewing this patch.
> 
> Thanks
> Laszlo
> 
> On 09/26/19 14:45, Laszlo Ersek wrote:
> > Chao, Jian, Jiewen,
> >
> > can you please review this patch?
> >
> > Thanks,
> > Laszlo
> >
> > On 09/17/19 21:49, Laszlo Ersek wrote:
> >> Unlike the InstallMultipleProtocolInterfaces() boot service, which takes
> >> an (EFI_HANDLE*) as first parameter, the
> >> UninstallMultipleProtocolInterfaces() boot service takes an EFI_HANDLE as
> >> first parameter.
> >>
> >> These are actual bugs. They must have remained hidden until now because
> >> they are all in Unload() functions, which are probably exercised
> >> infrequently. Fix the UninstallMultipleProtocolInterfaces() calls.
> >>
> >> Cc: Chao Zhang <chao.b.zhang@intel.com>
> >> Cc: Jian Wang <jian.j.wang@intel.com>
> >> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> >> ---
> >>
> >> Notes:
> >>     build-tested only
> >>
> >>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c                                  | 2 +-
> >>  SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c                                 | 2 +-
> >>
> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDri
> ver.c | 2 +-
> >>  3 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> >> index 54155a338100..9052eced757d 100644
> >> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> >> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> >> @@ -443,7 +443,7 @@ Tcg2ConfigDriverUnload (
> >>    ASSERT (PrivateData->Signature ==
> TCG2_CONFIG_PRIVATE_DATA_SIGNATURE);
> >>
> >>    gBS->UninstallMultipleProtocolInterfaces (
> >> -         &ImageHandle,
> >> +         ImageHandle,
> >>           &gEfiCallerIdGuid,
> >>           PrivateData,
> >>           NULL
> >> diff --git a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
> b/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
> >> index 341879e4c4ba..fb06624fdb8f 100644
> >> --- a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
> >> +++ b/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
> >> @@ -138,7 +138,7 @@ TcgConfigDriverUnload (
> >>    ASSERT (PrivateData->Signature ==
> TCG_CONFIG_PRIVATE_DATA_SIGNATURE);
> >>
> >>    gBS->UninstallMultipleProtocolInterfaces (
> >> -         &ImageHandle,
> >> +         ImageHandle,
> >>           &gEfiCallerIdGuid,
> >>           PrivateData,
> >>           NULL
> >> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> Driver.c
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> Driver.c
> >> index 798ef9cfbc01..6c0294151e6c 100644
> >> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> Driver.c
> >> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig
> Driver.c
> >> @@ -115,7 +115,7 @@ SecureBootConfigDriverUnload (
> >>    ASSERT (PrivateData->Signature ==
> SECUREBOOT_CONFIG_PRIVATE_DATA_SIGNATURE);
> >>
> >>    gBS->UninstallMultipleProtocolInterfaces (
> >> -         &ImageHandle,
> >> +         ImageHandle,
> >>           &gEfiCallerIdGuid,
> >>           PrivateData,
> >>           NULL
> >>
> >
> >
> > 
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#48465): https://edk2.groups.io/g/devel/message/48465
Mute This Topic: https://groups.io/mt/34180228/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 26/35] SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls
Posted by Zhang, Chao B 6 years, 4 months ago
Hi Laszlo:
   Sorry for late response. The fix is good to me. I am also interested in how you find this issue, can you share it?
 
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: 2019年10月3日 19:07
To: Zhang, Chao B <chao.b.zhang@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>
Subject: Re: [edk2-devel] [PATCH 26/35] SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls

Pinging SecurityPkg maintainers again, for reviewing this patch.

Thanks
Laszlo

On 09/26/19 14:45, Laszlo Ersek wrote:
> Chao, Jian, Jiewen,
> 
> can you please review this patch?
> 
> Thanks,
> Laszlo
> 
> On 09/17/19 21:49, Laszlo Ersek wrote:
>> Unlike the InstallMultipleProtocolInterfaces() boot service, which 
>> takes an (EFI_HANDLE*) as first parameter, the
>> UninstallMultipleProtocolInterfaces() boot service takes an 
>> EFI_HANDLE as first parameter.
>>
>> These are actual bugs. They must have remained hidden until now 
>> because they are all in Unload() functions, which are probably 
>> exercised infrequently. Fix the UninstallMultipleProtocolInterfaces() calls.
>>
>> Cc: Chao Zhang <chao.b.zhang@intel.com>
>> Cc: Jian Wang <jian.j.wang@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>>     build-tested only
>>
>>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c                                  | 2 +-
>>  SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c                                 | 2 +-
>>  
>> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfi
>> gDriver.c | 2 +-
>>  3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c 
>> b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
>> index 54155a338100..9052eced757d 100644
>> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
>> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
>> @@ -443,7 +443,7 @@ Tcg2ConfigDriverUnload (
>>    ASSERT (PrivateData->Signature == 
>> TCG2_CONFIG_PRIVATE_DATA_SIGNATURE);
>>  
>>    gBS->UninstallMultipleProtocolInterfaces (
>> -         &ImageHandle,
>> +         ImageHandle,
>>           &gEfiCallerIdGuid,
>>           PrivateData,
>>           NULL
>> diff --git a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c 
>> b/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
>> index 341879e4c4ba..fb06624fdb8f 100644
>> --- a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
>> +++ b/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
>> @@ -138,7 +138,7 @@ TcgConfigDriverUnload (
>>    ASSERT (PrivateData->Signature == 
>> TCG_CONFIG_PRIVATE_DATA_SIGNATURE);
>>  
>>    gBS->UninstallMultipleProtocolInterfaces (
>> -         &ImageHandle,
>> +         ImageHandle,
>>           &gEfiCallerIdGuid,
>>           PrivateData,
>>           NULL
>> diff --git 
>> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
>> figDriver.c 
>> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
>> figDriver.c index 798ef9cfbc01..6c0294151e6c 100644
>> --- 
>> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
>> figDriver.c
>> +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBoo
>> +++ tConfigDriver.c
>> @@ -115,7 +115,7 @@ SecureBootConfigDriverUnload (
>>    ASSERT (PrivateData->Signature == 
>> SECUREBOOT_CONFIG_PRIVATE_DATA_SIGNATURE);
>>  
>>    gBS->UninstallMultipleProtocolInterfaces (
>> -         &ImageHandle,
>> +         ImageHandle,
>>           &gEfiCallerIdGuid,
>>           PrivateData,
>>           NULL
>>
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#48473): https://edk2.groups.io/g/devel/message/48473
Mute This Topic: https://groups.io/mt/34180228/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 26/35] SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls
Posted by Laszlo Ersek 6 years, 4 months ago
On 10/04/19 15:14, Zhang, Chao B wrote:
> Hi Laszlo:
>    Sorry for late response. The fix is good to me.

Thanks!

Can you give Reviewed-by or Acked-by?

> I am also interested in how you find this issue, can you share it?

Sure, please see the explanation in patches #00 and #01 (I CC'd you on
them originally):

* http://mid.mail-archive.com/20190917194935.24322-1-lersek@redhat.com
  https://edk2.groups.io/g/devel/message/47387

* http://mid.mail-archive.com/20190917194935.24322-2-lersek@redhat.com
  https://edk2.groups.io/g/devel/message/47388

Thanks,
Laszlo

>
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com] 
> Sent: 2019年10月3日 19:07
> To: Zhang, Chao B <chao.b.zhang@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: edk2-devel-groups-io <devel@edk2.groups.io>
> Subject: Re: [edk2-devel] [PATCH 26/35] SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls
> 
> Pinging SecurityPkg maintainers again, for reviewing this patch.
> 
> Thanks
> Laszlo
> 
> On 09/26/19 14:45, Laszlo Ersek wrote:
>> Chao, Jian, Jiewen,
>>
>> can you please review this patch?
>>
>> Thanks,
>> Laszlo
>>
>> On 09/17/19 21:49, Laszlo Ersek wrote:
>>> Unlike the InstallMultipleProtocolInterfaces() boot service, which 
>>> takes an (EFI_HANDLE*) as first parameter, the
>>> UninstallMultipleProtocolInterfaces() boot service takes an 
>>> EFI_HANDLE as first parameter.
>>>
>>> These are actual bugs. They must have remained hidden until now 
>>> because they are all in Unload() functions, which are probably 
>>> exercised infrequently. Fix the UninstallMultipleProtocolInterfaces() calls.
>>>
>>> Cc: Chao Zhang <chao.b.zhang@intel.com>
>>> Cc: Jian Wang <jian.j.wang@intel.com>
>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>> ---
>>>
>>> Notes:
>>>     build-tested only
>>>
>>>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c                                  | 2 +-
>>>  SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c                                 | 2 +-
>>>
>>> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfi
>>> gDriver.c | 2 +-
>>>  3 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c 
>>> b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
>>> index 54155a338100..9052eced757d 100644
>>> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
>>> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
>>> @@ -443,7 +443,7 @@ Tcg2ConfigDriverUnload (
>>>    ASSERT (PrivateData->Signature == 
>>> TCG2_CONFIG_PRIVATE_DATA_SIGNATURE);
>>>
>>>    gBS->UninstallMultipleProtocolInterfaces (
>>> -         &ImageHandle,
>>> +         ImageHandle,
>>>           &gEfiCallerIdGuid,
>>>           PrivateData,
>>>           NULL
>>> diff --git a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c 
>>> b/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
>>> index 341879e4c4ba..fb06624fdb8f 100644
>>> --- a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
>>> +++ b/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
>>> @@ -138,7 +138,7 @@ TcgConfigDriverUnload (
>>>    ASSERT (PrivateData->Signature == 
>>> TCG_CONFIG_PRIVATE_DATA_SIGNATURE);
>>>
>>>    gBS->UninstallMultipleProtocolInterfaces (
>>> -         &ImageHandle,
>>> +         ImageHandle,
>>>           &gEfiCallerIdGuid,
>>>           PrivateData,
>>>           NULL
>>> diff --git 
>>> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
>>> figDriver.c 
>>> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
>>> figDriver.c index 798ef9cfbc01..6c0294151e6c 100644
>>> --- 
>>> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon
>>> figDriver.c
>>> +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBoo
>>> +++ tConfigDriver.c
>>> @@ -115,7 +115,7 @@ SecureBootConfigDriverUnload (
>>>    ASSERT (PrivateData->Signature == 
>>> SECUREBOOT_CONFIG_PRIVATE_DATA_SIGNATURE);
>>>
>>>    gBS->UninstallMultipleProtocolInterfaces (
>>> -         &ImageHandle,
>>> +         ImageHandle,
>>>           &gEfiCallerIdGuid,
>>>           PrivateData,
>>>           NULL
>>>
>>
>>
>> 
>>
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#48485): https://edk2.groups.io/g/devel/message/48485
Mute This Topic: https://groups.io/mt/34180228/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-