MdeModulePkg/Core/Dxe/Hand/Locate.c | 64 +++++++++++++++++++++++------ 1 file changed, 51 insertions(+), 13 deletions(-)
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3666
Currently, CoreLocateHandleBuffer() follows three steps:
1) get the size of protocol database firstly
2) allocate the buffer based on the size
3) get the protocol database into the buffer
There is no lock protection for the whole three steps. If a new protocol
added in step 2) by other task, e.g. (event timer handle USB device
hotplug). The size of protocol database may be increased and cannot fit
into the previous buffer in step 3). The protocol database cannot be
returned successfully, EFI_BUFFER_TOO_SMALL error will be returned.
This patch adds the lock to protect the whole three steps.
It can make sure the correct protocol database be returned.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Dandan Bi <dandan.bi@intel.com>
Signed-off-by: Hua Ma <hua.ma@intel.com>
---
MdeModulePkg/Core/Dxe/Hand/Locate.c | 64 +++++++++++++++++++++++------
1 file changed, 51 insertions(+), 13 deletions(-)
diff --git a/MdeModulePkg/Core/Dxe/Hand/Locate.c b/MdeModulePkg/Core/Dxe/Hand/Locate.c
index be17f4cbc3..4987c046c6 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Locate.c
+++ b/MdeModulePkg/Core/Dxe/Hand/Locate.c
@@ -86,7 +86,8 @@ CoreGetNextLocateByProtocol (
/**
- Locates the requested handle(s) and returns them in Buffer.
+ Internal function for locating the requested handle(s) and returns them in Buffer.
+ The caller should already have acquired the ProtocolLock.
@param SearchType The type of search to perform to locate the
handles
@@ -104,8 +105,7 @@ CoreGetNextLocateByProtocol (
**/
EFI_STATUS
-EFIAPI
-CoreLocateHandle (
+InternalCoreLocateHandle (
IN EFI_LOCATE_SEARCH_TYPE SearchType,
IN EFI_GUID *Protocol OPTIONAL,
IN VOID *SearchKey OPTIONAL,
@@ -143,11 +143,6 @@ CoreLocateHandle (
ResultBuffer = (IHANDLE **) Buffer;
Status = EFI_SUCCESS;
- //
- // Lock the protocol database
- //
- CoreAcquireProtocolLock ();
-
//
// Get the search function based on type
//
@@ -190,7 +185,6 @@ CoreLocateHandle (
}
if (EFI_ERROR(Status)) {
- CoreReleaseProtocolLock ();
return Status;
}
@@ -247,10 +241,47 @@ CoreLocateHandle (
}
}
- CoreReleaseProtocolLock ();
return Status;
}
+/**
+ Locates the requested handle(s) and returns them in Buffer.
+
+ @param SearchType The type of search to perform to locate the
+ handles
+ @param Protocol The protocol to search for
+ @param SearchKey Dependant on SearchType
+ @param BufferSize On input the size of Buffer. On output the
+ size of data returned.
+ @param Buffer The buffer to return the results in
+
+ @retval EFI_BUFFER_TOO_SMALL Buffer too small, required buffer size is
+ returned in BufferSize.
+ @retval EFI_INVALID_PARAMETER Invalid parameter
+ @retval EFI_SUCCESS Successfully found the requested handle(s) and
+ returns them in Buffer.
+
+**/
+EFI_STATUS
+EFIAPI
+CoreLocateHandle (
+ IN EFI_LOCATE_SEARCH_TYPE SearchType,
+ IN EFI_GUID *Protocol OPTIONAL,
+ IN VOID *SearchKey OPTIONAL,
+ IN OUT UINTN *BufferSize,
+ OUT EFI_HANDLE *Buffer
+ )
+{
+ EFI_STATUS Status;
+
+ //
+ // Lock the protocol database
+ //
+ CoreAcquireProtocolLock ();
+ Status = InternalCoreLocateHandle(SearchType, Protocol, SearchKey, BufferSize, Buffer);
+ CoreReleaseProtocolLock ();
+ return Status;
+}
/**
@@ -610,7 +641,6 @@ Done:
return Status;
}
-
/**
Function returns an array of handles that support the requested protocol
in a buffer allocated from pool. This is a version of CoreLocateHandle()
@@ -657,7 +687,12 @@ CoreLocateHandleBuffer (
BufferSize = 0;
*NumberHandles = 0;
*Buffer = NULL;
- Status = CoreLocateHandle (
+
+ //
+ // Lock the protocol database
+ //
+ CoreAcquireProtocolLock();
+ Status = InternalCoreLocateHandle (
SearchType,
Protocol,
SearchKey,
@@ -674,15 +709,17 @@ CoreLocateHandleBuffer (
if (Status != EFI_INVALID_PARAMETER) {
Status = EFI_NOT_FOUND;
}
+ CoreReleaseProtocolLock ();
return Status;
}
*Buffer = AllocatePool (BufferSize);
if (*Buffer == NULL) {
+ CoreReleaseProtocolLock ();
return EFI_OUT_OF_RESOURCES;
}
- Status = CoreLocateHandle (
+ Status = InternalCoreLocateHandle (
SearchType,
Protocol,
SearchKey,
@@ -695,6 +732,7 @@ CoreLocateHandleBuffer (
*NumberHandles = 0;
}
+ CoreReleaseProtocolLock ();
return Status;
}
--
2.32.0.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81270): https://edk2.groups.io/g/devel/message/81270
Mute This Topic: https://groups.io/mt/85943365/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Reviewed-by: Dandan Bi <dandan.bi@intel.com>
Thanks,
Dandan
> -----Original Message-----
> From: Ma, Hua <hua.ma@intel.com>
> Sent: Wednesday, September 29, 2021 1:49 PM
> To: devel@edk2.groups.io
> Cc: Ma, Hua <hua.ma@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Liming Gao <gaoliming@byosoft.com.cn>; Bi, Dandan <dandan.bi@intel.com>
> Subject: [PATCH] MdeModulePkg/Core/Dxe: Add lock protection in
> CoreLocateHandleBuffer()
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3666
>
> Currently, CoreLocateHandleBuffer() follows three steps:
> 1) get the size of protocol database firstly
> 2) allocate the buffer based on the size
> 3) get the protocol database into the buffer There is no lock protection for
> the whole three steps. If a new protocol added in step 2) by other task, e.g.
> (event timer handle USB device hotplug). The size of protocol database may
> be increased and cannot fit into the previous buffer in step 3). The protocol
> database cannot be returned successfully, EFI_BUFFER_TOO_SMALL error
> will be returned.
>
> This patch adds the lock to protect the whole three steps.
> It can make sure the correct protocol database be returned.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Signed-off-by: Hua Ma <hua.ma@intel.com>
> ---
> MdeModulePkg/Core/Dxe/Hand/Locate.c | 64
> +++++++++++++++++++++++------
> 1 file changed, 51 insertions(+), 13 deletions(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/Hand/Locate.c
> b/MdeModulePkg/Core/Dxe/Hand/Locate.c
> index be17f4cbc3..4987c046c6 100644
> --- a/MdeModulePkg/Core/Dxe/Hand/Locate.c
> +++ b/MdeModulePkg/Core/Dxe/Hand/Locate.c
> @@ -86,7 +86,8 @@ CoreGetNextLocateByProtocol (
>
>
> /**
> - Locates the requested handle(s) and returns them in Buffer.
> + Internal function for locating the requested handle(s) and returns them in
> Buffer.
> + The caller should already have acquired the ProtocolLock.
>
> @param SearchType The type of search to perform to locate the
> handles @@ -104,8 +105,7 @@
> CoreGetNextLocateByProtocol (
>
> **/
> EFI_STATUS
> -EFIAPI
> -CoreLocateHandle (
> +InternalCoreLocateHandle (
> IN EFI_LOCATE_SEARCH_TYPE SearchType,
> IN EFI_GUID *Protocol OPTIONAL,
> IN VOID *SearchKey OPTIONAL,
> @@ -143,11 +143,6 @@ CoreLocateHandle (
> ResultBuffer = (IHANDLE **) Buffer;
> Status = EFI_SUCCESS;
>
> - //
> - // Lock the protocol database
> - //
> - CoreAcquireProtocolLock ();
> -
> //
> // Get the search function based on type
> //
> @@ -190,7 +185,6 @@ CoreLocateHandle (
> }
>
> if (EFI_ERROR(Status)) {
> - CoreReleaseProtocolLock ();
> return Status;
> }
>
> @@ -247,10 +241,47 @@ CoreLocateHandle (
> }
> }
>
> - CoreReleaseProtocolLock ();
> return Status;
> }
>
> +/**
> + Locates the requested handle(s) and returns them in Buffer.
> +
> + @param SearchType The type of search to perform to locate the
> + handles
> + @param Protocol The protocol to search for
> + @param SearchKey Dependant on SearchType
> + @param BufferSize On input the size of Buffer. On output the
> + size of data returned.
> + @param Buffer The buffer to return the results in
> +
> + @retval EFI_BUFFER_TOO_SMALL Buffer too small, required buffer size is
> + returned in BufferSize.
> + @retval EFI_INVALID_PARAMETER Invalid parameter
> + @retval EFI_SUCCESS Successfully found the requested handle(s)
> and
> + returns them in Buffer.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +CoreLocateHandle (
> + IN EFI_LOCATE_SEARCH_TYPE SearchType,
> + IN EFI_GUID *Protocol OPTIONAL,
> + IN VOID *SearchKey OPTIONAL,
> + IN OUT UINTN *BufferSize,
> + OUT EFI_HANDLE *Buffer
> + )
> +{
> + EFI_STATUS Status;
> +
> + //
> + // Lock the protocol database
> + //
> + CoreAcquireProtocolLock ();
> + Status = InternalCoreLocateHandle(SearchType, Protocol, SearchKey,
> +BufferSize, Buffer);
> + CoreReleaseProtocolLock ();
> + return Status;
> +}
>
>
> /**
> @@ -610,7 +641,6 @@ Done:
> return Status;
> }
>
> -
> /**
> Function returns an array of handles that support the requested protocol
> in a buffer allocated from pool. This is a version of CoreLocateHandle() @@
> -657,7 +687,12 @@ CoreLocateHandleBuffer (
> BufferSize = 0;
> *NumberHandles = 0;
> *Buffer = NULL;
> - Status = CoreLocateHandle (
> +
> + //
> + // Lock the protocol database
> + //
> + CoreAcquireProtocolLock();
> + Status = InternalCoreLocateHandle (
> SearchType,
> Protocol,
> SearchKey,
> @@ -674,15 +709,17 @@ CoreLocateHandleBuffer (
> if (Status != EFI_INVALID_PARAMETER) {
> Status = EFI_NOT_FOUND;
> }
> + CoreReleaseProtocolLock ();
> return Status;
> }
>
> *Buffer = AllocatePool (BufferSize);
> if (*Buffer == NULL) {
> + CoreReleaseProtocolLock ();
> return EFI_OUT_OF_RESOURCES;
> }
>
> - Status = CoreLocateHandle (
> + Status = InternalCoreLocateHandle (
> SearchType,
> Protocol,
> SearchKey,
> @@ -695,6 +732,7 @@ CoreLocateHandleBuffer (
> *NumberHandles = 0;
> }
>
> + CoreReleaseProtocolLock ();
> return Status;
> }
>
> --
> 2.32.0.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81618): https://edk2.groups.io/g/devel/message/81618
Mute This Topic: https://groups.io/mt/85943365/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> -----邮件原件-----
> 发件人: Bi, Dandan <dandan.bi@intel.com>
> 发送时间: 2021年10月8日 9:40
> 收件人: Ma, Hua <hua.ma@intel.com>; devel@edk2.groups.io
> 抄送: Wang, Jian J <jian.j.wang@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>
> 主题: RE: [PATCH] MdeModulePkg/Core/Dxe: Add lock protection in
> CoreLocateHandleBuffer()
>
> Reviewed-by: Dandan Bi <dandan.bi@intel.com>
>
>
>
> Thanks,
> Dandan
>
> > -----Original Message-----
> > From: Ma, Hua <hua.ma@intel.com>
> > Sent: Wednesday, September 29, 2021 1:49 PM
> > To: devel@edk2.groups.io
> > Cc: Ma, Hua <hua.ma@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> > Liming Gao <gaoliming@byosoft.com.cn>; Bi, Dandan
> <dandan.bi@intel.com>
> > Subject: [PATCH] MdeModulePkg/Core/Dxe: Add lock protection in
> > CoreLocateHandleBuffer()
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3666
> >
> > Currently, CoreLocateHandleBuffer() follows three steps:
> > 1) get the size of protocol database firstly
> > 2) allocate the buffer based on the size
> > 3) get the protocol database into the buffer There is no lock protection
for
> > the whole three steps. If a new protocol added in step 2) by other task,
e.g.
> > (event timer handle USB device hotplug). The size of protocol database
may
> > be increased and cannot fit into the previous buffer in step 3). The
protocol
> > database cannot be returned successfully, EFI_BUFFER_TOO_SMALL error
> > will be returned.
> >
> > This patch adds the lock to protect the whole three steps.
> > It can make sure the correct protocol database be returned.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Dandan Bi <dandan.bi@intel.com>
> > Signed-off-by: Hua Ma <hua.ma@intel.com>
> > ---
> > MdeModulePkg/Core/Dxe/Hand/Locate.c | 64
> > +++++++++++++++++++++++------
> > 1 file changed, 51 insertions(+), 13 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/Dxe/Hand/Locate.c
> > b/MdeModulePkg/Core/Dxe/Hand/Locate.c
> > index be17f4cbc3..4987c046c6 100644
> > --- a/MdeModulePkg/Core/Dxe/Hand/Locate.c
> > +++ b/MdeModulePkg/Core/Dxe/Hand/Locate.c
> > @@ -86,7 +86,8 @@ CoreGetNextLocateByProtocol (
> >
> >
> > /**
> > - Locates the requested handle(s) and returns them in Buffer.
> > + Internal function for locating the requested handle(s) and returns
them
> in
> > Buffer.
> > + The caller should already have acquired the ProtocolLock.
> >
> > @param SearchType The type of search to perform to
> locate the
> > handles @@ -104,8 +105,7 @@
> > CoreGetNextLocateByProtocol (
> >
> > **/
> > EFI_STATUS
> > -EFIAPI
> > -CoreLocateHandle (
> > +InternalCoreLocateHandle (
> > IN EFI_LOCATE_SEARCH_TYPE SearchType,
> > IN EFI_GUID *Protocol OPTIONAL,
> > IN VOID *SearchKey OPTIONAL,
> > @@ -143,11 +143,6 @@ CoreLocateHandle (
> > ResultBuffer = (IHANDLE **) Buffer;
> > Status = EFI_SUCCESS;
> >
> > - //
> > - // Lock the protocol database
> > - //
> > - CoreAcquireProtocolLock ();
> > -
> > //
> > // Get the search function based on type
> > //
> > @@ -190,7 +185,6 @@ CoreLocateHandle (
> > }
> >
> > if (EFI_ERROR(Status)) {
> > - CoreReleaseProtocolLock ();
> > return Status;
> > }
> >
> > @@ -247,10 +241,47 @@ CoreLocateHandle (
> > }
> > }
> >
> > - CoreReleaseProtocolLock ();
> > return Status;
> > }
> >
> > +/**
> > + Locates the requested handle(s) and returns them in Buffer.
> > +
> > + @param SearchType The type of search to perform to
> locate the
> > + handles
> > + @param Protocol The protocol to search for
> > + @param SearchKey Dependant on SearchType
> > + @param BufferSize On input the size of Buffer. On
> output the
> > + size of data returned.
> > + @param Buffer The buffer to return the results in
> > +
> > + @retval EFI_BUFFER_TOO_SMALL Buffer too small, required buffer
> size is
> > + returned in BufferSize.
> > + @retval EFI_INVALID_PARAMETER Invalid parameter
> > + @retval EFI_SUCCESS Successfully found the requested
> handle(s)
> > and
> > + returns them in Buffer.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +CoreLocateHandle (
> > + IN EFI_LOCATE_SEARCH_TYPE SearchType,
> > + IN EFI_GUID *Protocol OPTIONAL,
> > + IN VOID *SearchKey OPTIONAL,
> > + IN OUT UINTN *BufferSize,
> > + OUT EFI_HANDLE *Buffer
> > + )
> > +{
> > + EFI_STATUS Status;
> > +
> > + //
> > + // Lock the protocol database
> > + //
> > + CoreAcquireProtocolLock ();
> > + Status = InternalCoreLocateHandle(SearchType, Protocol, SearchKey,
> > +BufferSize, Buffer);
> > + CoreReleaseProtocolLock ();
> > + return Status;
> > +}
> >
> >
> > /**
> > @@ -610,7 +641,6 @@ Done:
> > return Status;
> > }
> >
> > -
> > /**
> > Function returns an array of handles that support the requested
> protocol
> > in a buffer allocated from pool. This is a version of
CoreLocateHandle()
> @@
> > -657,7 +687,12 @@ CoreLocateHandleBuffer (
> > BufferSize = 0;
> > *NumberHandles = 0;
> > *Buffer = NULL;
> > - Status = CoreLocateHandle (
> > +
> > + //
> > + // Lock the protocol database
> > + //
> > + CoreAcquireProtocolLock();
> > + Status = InternalCoreLocateHandle (
> > SearchType,
> > Protocol,
> > SearchKey,
> > @@ -674,15 +709,17 @@ CoreLocateHandleBuffer (
> > if (Status != EFI_INVALID_PARAMETER) {
> > Status = EFI_NOT_FOUND;
> > }
> > + CoreReleaseProtocolLock ();
> > return Status;
> > }
> >
> > *Buffer = AllocatePool (BufferSize);
> > if (*Buffer == NULL) {
> > + CoreReleaseProtocolLock ();
> > return EFI_OUT_OF_RESOURCES;
> > }
> >
> > - Status = CoreLocateHandle (
> > + Status = InternalCoreLocateHandle (
> > SearchType,
> > Protocol,
> > SearchKey,
> > @@ -695,6 +732,7 @@ CoreLocateHandleBuffer (
> > *NumberHandles = 0;
> > }
> >
> > + CoreReleaseProtocolLock ();
> > return Status;
> > }
> >
> > --
> > 2.32.0.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81636): https://edk2.groups.io/g/devel/message/81636
Mute This Topic: https://groups.io/mt/86162062/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.