Description
In ScopedProtocol::drop
, we currently panic if close_protocol
returns something other than Status::SUCCESS
.
I've observed that on some firmware, this can cause a problem if the same protocol is opened twice in non-exclusive mode via OpenProtocolAttributes::GetProtocol
. In general you shouldn't open the same protocol twice, but with a read-only protocol like DevicePath
it should be fine to do so. However, this can lead to the following situation:
fn do_something(h1: Handle, h2: Handle2) {
{
let dp1 = boot::open_protocol::<DevicePath>(OpenProtocolParams { handle: h1, agent: boot::image_handle(), controller: None }).unwrap();
let dp2 = boot::open_protocol::<DevicePath>(OpenProtocolParams { handle: h2, agent: boot::image_handle(), controller: None }).unwrap();
// Do stuff with dp1/dp2...
}
// Now dp1 and dp2 are dropped.
// If dp1 happens to be the same as dp2, the first drop will succeed,
// but the second will fail (at least on some firmware), with `Status::NOT_FOUND`.
}
I ran into this with real code, where I didn't expect dp1 == dp2. Fortunately I noticed the issue in testing; it would have been bad if the code started panicking in production.
Although my code is fixed, I don't think we should panic here, as it's not a particularly harmful error if close_protocol fails in this case. Some potential changes:
- Remove the panic, or change it to a verbose log (debug! or trace! perhaps)
- Keep the panic, but don't call close_protocol if
GetProtocol
is used to open the protocol. The spec says "The caller is also not required to close the protocol interface with close_protocol" when GET_PROTOCOL is used. - Keep the panic, but allow either SUCCESS or NOT_FOUND when
GetProtocol
is used to open the protocol.