Skip to content

ScopedProtocol::drop can panic when used with OpenProtocolAttributes::GetProtocol #1622

Open
@nicholasbishop

Description

@nicholasbishop

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions