Re: Potentially missing CloseProtocol() call


Michael Brown
 

On 09/02/2021 11:54, Laszlo Ersek wrote:
On 02/08/21 16:28, Michael Brown wrote:
This 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?
The CloseProtocol() specification has parts as follows (relevant passage
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
protocols.
<snip>
Thank you! I see that text in the documentation for the deprecated call UninstallProtocolInterface(), which explains why I hadn't previously noticed it.

My comments:
(1) I don't really understand what the last quoted passage means by
"closing an agent".
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:

https://github.com/tianocore/edk2/blob/stable/202011/MdeModulePkg/Core/Dxe/Hand/Handle.c#L659-L674

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 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.
They 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.

Michael

Join discuss@edk2.groups.io to automatically receive all group messages.