For qcom/quality improvements - #18
Open
quic-bjorande wants to merge 3 commits into
Open
Conversation
fastboot.efi is commonly used to boot images cross-compiled on X86 machines, but if the user forgets to force the UKI to be built for aarch64, fastboot.efi will determine that it's not a valid PE image and end up failing to load the image without clear indication of what's wrong. Handle this case in a cleaner way to aid the user in determining the cause of such issues. Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
fastboot_respond() has been allocating a new USB transfer buffer for every response and never released it. Allocate one 64-byte IN transfer buffer in main(), pass it through command handlers, and free it at shutdown. This keeps response semantics unchanged while fixing the response-buffer lifetime and leak. Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
Handle malformed command payloads more defensively by truncating at NUL, trimming CR/LF, validating UTF-8, and rejecting invalid download/getvar syntax with explicit FAIL responses. Also avoid panicking if boot fails after acknowledging a boot request: send a single OKAY before StartImage, then only log StartImage failures. This preserves fastboot protocol expectations and avoids a second response after OKAY. Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
| fn handle_boot(usb_device: &ScopedProtocol<EfiUsbDevice>, payload: &[u8]) -> Result { | ||
| let (handle, _initrd) = if is_peimage(payload) { | ||
| (handle_peimage(payload)?, None) | ||
| } else if let Some(machine) = pe_machine(payload) { |
Contributor
There was a problem hiding this comment.
My immediate reaction would be to move this near the:
if machine != PE_ARM64 {
return false;
}check in is_peimage, however that function currently only returns a boolean.. but OTOH it may be useful to have it define a couple of related error types
|
|
||
| fn fastboot_respond( | ||
| usb_device: &ScopedProtocol<EfiUsbDevice>, | ||
| response_buffer: *mut u8, |
Contributor
There was a problem hiding this comment.
Can we do without the boilerplate argument, i.e. perhaps by declaring a global Option<T> and initializing it in main?
| .unwrap_or(data.len()); | ||
| let request = core::str::from_utf8(&data[..request_end]).map_err(|_| ())?; | ||
| let request = request.trim_end_matches(|ch| ch == '\r' || ch == '\n'); | ||
|
|
Contributor
There was a problem hiding this comment.
String::from_utf8().trim_ascii()?
| fastboot_respond( | ||
| usb_device, | ||
| response_buffer, | ||
| &format!("FAILfailed: {:?}", err.status()), |
Contributor
There was a problem hiding this comment.
I'm not quite sure, but I think that this will only print the textual representation of the UEFI error, but give no clues to what function it came from
| .expect("Failed to send response"); | ||
| usb_device | ||
| .send(usb_device::ENDPOINT_OUT, 1024 * 1024, command_buffer) | ||
| .expect("failed to queue command buffer"); |
Contributor
There was a problem hiding this comment.
nit: there are some Faileds and some faileds
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
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.
No description provided.