Protocol safety improvements #460
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-proto-safety
branch
from
8920d4b to
1d109e1
Compare
This is a convenience method to get any arbitrary handle that supports a particular `Protocol`.
Rather than opening the Output protocol, which isn't important for this test, just write through the context pointer and assert that the expected data was written.
This function is already marked deprecated, mark it unsafe as well and update the documentation to describe why.
This method has the same problems as `handle_protocol`; it does not mark the handle and protocol as in use. Calls to `locate_protocol` can be replaced by calling `get_handle_for_protocol` and `open_protocol`. rust-osdev#359
GabrielMajeri
force-pushed
the
bishop-proto-safety
branch
from
1d109e1 to
c7d0064
Compare
|
The PR looks great! The new |
e820
pushed a commit
to e820/uefi-rs
that referenced
this pull request
Jul 30, 2022
* Add BootServices::get_handle_for_protocol This is a convenience method to get any arbitrary handle that supports a particular `Protocol`. * Use open_protocol in shim-lock test * Use open_protocol in multiprocessor test * Use open_protocol in device path test * Use open_protocol in pointer test * Use open_protocol in graphics test * Use open_protocol in file system test * Use open_protocol in the serial device test * Simplify event callback with context test Rather than opening the Output protocol, which isn't important for this test, just write through the context pointer and assert that the expected data was written. * Mark handle_protocol as unsafe This function is already marked deprecated, mark it unsafe as well and update the documentation to describe why. * Deprecate BootServices::locate_protocol and mark it unsafe This method has the same problems as `handle_protocol`; it does not mark the handle and protocol as in use. Calls to `locate_protocol` can be replaced by calling `get_handle_for_protocol` and `open_protocol`. rust-osdev#359
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Overview:
BootServices::get_handle_for_protocol. This is a convenience method for finding one arbitrary handle that supports a protocol.locate_protocolasunsafeand deprecated, as it has the same safety problems ashandle_protocol.handle_protocolasunsafe(it's already deprecated).open_protocolinstead oflocate_protocol.Using
open_protocolis now the only undeprecated way of opening a protocol. There's still some more safety work to do here -- if the protocol isn't opened in exclusive mode, or if theagentparameter isn't set correctly, UB could result. But pushing all users towardsopen_protocolis a good first step.This is a partial fix for #359