Re: Potentially missing CloseProtocol() call


Laszlo Ersek
 

On 02/09/21 13:33, Michael Brown wrote:
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.
Right, my apologies. I'm not sure why I wrote CloseProtocol(). Perhaps a
typo, or maybe I lost track of where I was in the PDF with "evince".


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.
That seems to be the case, yes.

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

Laszlo

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