More protocol safety improvements #478
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
nicholasbishop force-pushed the bishop-global-image-7 branch 3 times, most recently from d584e18 to b22e119 Compare 
Describe details of what it does and what the entry point function must look like. Note that since `uefi-macros` does not depend on `uefi` (except as a dev-dependency), short links to `uefi` types cannot be used. Full links to docs.rs are used instead.
nicholasbishop force-pushed the bishop-global-image-7 branch from b22e119 to ce94db4 Compare 
GabrielMajeri approved these changes Sep 1, 2022
| No worries at all, thanks for reviewing :) |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
This is a change I've been noodling on for a bit related to #359. The basic issue is that in order to use protocols fully safely, with no chance of either the handle or protocol being modified or removed while in use, we need to ensure that protocols are opened in exclusive mode with the correct agent handle set. We've made some good steps towards that by deprecating and marking unsafe things like
handle_protocol, but there are still unsafe ways to useopen_protocol.To resolve this (to the extent possible given the design of UEFI), I've added a new
open_protocol_exclusivefunction. This function is fully safe; it ensures that the correct agent is set through the use of a new global image handle set by the#[entry]macro, and always opens the protocol in exclusive mode. This is a much nicer interface to use thanopen_protocol, since you only have to provide theHandleyou want to open.There are still some cases where the full power of
open_protocolis needed; uefi-test-runner has some examples of this. For those cases though the caller needs to be aware that they are responsible for ensuring that the handle and protocol are valid until the protocol is no longer in use, so I've markedopen_protocolas unsafe. Since most uses can be replaced withopen_protocol_exclusive, this is hopefully not too much of a burden.Having a global image handle and marking
open_protocolas unsafe are fairly significant changes to the API (although notably we haven't done a release yet that includes #434, so users ofopen_protocolwill already have to make some code adjustments when they next upgrade), but I think it will provide a nice boost in both safety and ease of use.