Potentially missing CloseProtocol() call
There are a number of places that do not call CloseProtocol() while it
appears to be required, in EDK2. Can someone confirm if (some of) those are
indeed errors, or there are actually cases where skipping CloseProtocol()
after OpenProtocol() is appropriate?
Here are the places I would expect a CloseProtocol() call but apparently
- GetDriverName() / GetDriverImageName()
- *ProtocolDumpInformation(). DevicePathProtocolDumpInformationEx() does
call it. So lack of call seems like an error to me.
I am new to UEFI and trying to learn basics like how to use OpenProtocol().
I have not observed or encountered any concrete issue.
On 02/06/21 19:56, Satoshi Tanda wrote:
There are a number of places that do not call CloseProtocol() while itgBS->OpenProtocol() calls that pass the EFI_OPEN_PROTOCOL_GET_PROTOCOL
argument for the "Attributes" parameter *need not* be mirrored with
Please refer to the UEFI spec on the OpenProtocol() boot service,
[...] The caller is also not required to close the protocol
interface with EFI_BOOT_SERVICES.CloseProtocol().
So you *can* call CloseProtocol(), but you don't have to.
On 08/02/2021 14:22, Laszlo Ersek wrote:
gBS->OpenProtocol() calls that pass the EFI_OPEN_PROTOCOL_GET_PROTOCOLThis reminds me of a very longstanding question I've had around OpenProtocol(): is there any defined way for something that is an ordinary consumer (rather than a driver or child controller) to obtain a long-lived pointer to a protocol interface?
For example: ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/MainTextEditor.c seems to open EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL using just HandleProtocol() and then continues to use the pointer for the lifetime of the application. But, as far as I can tell, there's nothing that guarantees that this pointer remains valid?
Hi Laszlo,toggle quoted messageShow quoted text
I was almost exclusively checking edk2 headers but should have checked with
the specs. Thank you for clarifying it for me.
On Mon, Feb 8, 2021 at 7:28 AM Michael Brown <mcb30@...> wrote:
On 08/02/2021 14:22, Laszlo Ersek wrote:gBS->OpenProtocol() calls that pass the EFI_OPEN_PROTOCOL_GET_PROTOCOLThis reminds me of a very longstanding question I've had around
On 02/08/21 16:28, Michael Brown wrote:
On 08/02/2021 14:22, Laszlo Ersek wrote:The CloseProtocol() specification has parts as follows (relevant passagegBS->OpenProtocol() calls that pass the EFI_OPEN_PROTOCOL_GET_PROTOCOLThis reminds me of a very longstanding question I've had around
is the last one below):
The caller is responsible for ensuring that there are no references
to a protocol interface that has been removed. In some cases,
outstanding reference information is not available in the protocol,
so the protocol, once added, cannot be removed. Examples include
Console I/O, Block I/O, Disk I/O, and (in general) handles to device
EFI 1.10 Extension
The extension to this service directly addresses the limitations
described in the section above. There may be some drivers that are
currently consuming the protocol interface that needs to be
uninstalled, so it may be dangerous to just blindly remove a
protocol interface from the system. Since the usage of protocol
interfaces is now being tracked for components that use the
EFI_BOOT_SERVICES.CloseProtocol() boot services, a safe version of
this function can be implemented.
Lastly, all of the agents that have the protocol interface open with
an attribute of EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL,
EFI_OPEN_PROTOCOL_GET_PROTOCOL, or EFI_OPEN_PROTOCOL_TEST_PROTOCOL
are closed. If there are any agents remaining that still have the
protocol interface open, the protocol interface is not removed from
the handle and EFI_ACCESS_DENIED is returned. In addition, all of
the drivers that were disconnected with the boot service
DisconnectController() earlier, are reconnected with the boot
service EFI_BOOT_SERVICES.ConnectController(). If there are no
agents remaining that are consuming the protocol interface, then the
protocol interface is removed from the handle as described above.
(1) I don't really understand what the last quoted passage means by
"closing an agent".
(2) I've never tested this, nor attempted to verify it from the edk2
source, myself. It does suggest however that GET_PROTOCOL opens are
tracked too, and thus, *not* closing a GET_PROTOCOL open is technically
still a leak.
On 09/02/2021 11:54, Laszlo Ersek wrote:
On 02/08/21 16:28, Michael Brown wrote:Thank you! I see that text in the documentation for the deprecated call UninstallProtocolInterface(), which explains why I hadn't previously noticed it.This reminds me of a very longstanding question I've had aroundThe CloseProtocol() specification has parts as follows (relevant passage
My comments:Based on what the EDK2 implementation actually does, I'm pretty sure this means that any opens recorded with attributes of BY_HANDLE_PROTOCOL, GET_PROTOCOL, or TEST_PROTOCOL are simply discarded:
The code that performed the BY_HANDLE_PROTOCOL, GET_PROTOCOL, or TEST_PROTOCOL open is not notified in any way.
(2) I've never tested this, nor attempted to verify it from the edk2They are tracked, but as far as I can tell only for informative purposes (e.g. debugging using the EFI shell "openinfo" command, or with iPXE's DBG_EFI_OPENERS() call).
So: technically a leak, but a leak that is arguably useful from the perspective of someone (e.g. quite often me) trying to debug what code has tried to do something with a particular handle. And a leak that will get automatically cleaned up when the protocol is uninstalled.
But this still leaves my original problem: there is no way for code to ever know that a protocol it opened with GET_PROTOCOL has ceased to be valid, and nothing that stops said code from dereferencing the potentially invalid pointer.
As far as I can tell, the only way to be notified when a protocol pointer ceases to be valid is to open BY_DRIVER, and we can't open some protocols (such as EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL) with BY_DRIVER attributes because there will be multiple openers of that protocol.
On 02/09/21 13:33, Michael Brown wrote:
On 09/02/2021 11:54, Laszlo Ersek wrote:Right, my apologies. I'm not sure why I wrote CloseProtocol(). Perhaps aOn 02/08/21 16:28, Michael Brown wrote:Thank you! I see that text in the documentation for the deprecated callThis reminds me of a very longstanding question I've had aroundThe CloseProtocol() specification has parts as follows (relevant passage
typo, or maybe I lost track of where I was in the PDF with "evince".
That seems to be the case, yes.My comments:Based on what the EDK2 implementation actually does, I'm pretty sure
As far as I can tell, the only way to be notified when a protocolAgreed.