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


Laszlo Ersek
 

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/SecureBootConfigDr=
iver.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 =3D=3D TCG2_CONFIG_PRIVATE_DATA_SIGNATU=
RE);
=20
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 =3D=3D TCG_CONFIG_PRIVATE_DATA_SIGNATUR=
E);
=20
gBS->UninstallMultipleProtocolInterfaces (
- &ImageHandle,
+ ImageHandle,
&gEfiCallerIdGuid,
PrivateData,
NULL
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/Secure=
BootConfigDriver.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDx=
e/SecureBootConfigDriver.c
index 798ef9cfbc01..6c0294151e6c 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon=
figDriver.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon=
figDriver.c
@@ -115,7 +115,7 @@ SecureBootConfigDriverUnload (
ASSERT (PrivateData->Signature =3D=3D SECUREBOOT_CONFIG_PRIVATE_DATA_S=
IGNATURE);
=20
gBS->UninstallMultipleProtocolInterfaces (
- &ImageHandle,
+ ImageHandle,
&gEfiCallerIdGuid,
PrivateData,
NULL
--=20
2.19.1.3.g30247aa5d201


Philippe Mathieu-Daudé
 

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>


Laszlo Ersek
 

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


Laszlo Ersek
 

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



Yao, Jiewen
 

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



Zhang, Chao B
 

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



Laszlo Ersek
 

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



Zhang, Chao B
 

Reviewed-by : Chao Zhang <chao.b.zhang@intel.com>

-----Original Message-----
From: Zhang, Chao B
Sent: 2019年10月4日 21:14
To: edk2-devel-groups-io <devel@edk2.groups.io>; 'lersek@redhat.com' <lersek@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
Subject: RE: [edk2-devel] [PATCH 26/35] SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls

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



Laszlo Ersek
 

On 10/05/19 16:28, Zhang, Chao B wrote:
Reviewed-by : Chao Zhang <chao.b.zhang@intel.com>
Thanks!
Laszlo


-----Original Message-----
From: Zhang, Chao B
Sent: 2019年10月4日 21:14
To: edk2-devel-groups-io <devel@edk2.groups.io>; 'lersek@redhat.com' <lersek@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
Subject: RE: [edk2-devel] [PATCH 26/35] SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls

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