From 8af1ea2f44100b1987f3ac8731e0b0849e284886 Mon Sep 17 00:00:00 2001 From: James Forcier Date: Mon, 31 Jul 2017 14:19:02 -0700 Subject: [PATCH 1/9] Cargo.toml: specify lib path explicitly This silences a warning from cargo. --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index e670e12..bcf5476 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,6 +6,7 @@ authors = [ "james.forcier@coreos.com" ] [lib] name = "picker" crate-type = ["dylib"] +path = "src/picker.rs" [dependencies] uefi = { git = "https://github.com/csssuf/rust-uefi" } From 0b7205762c7a5b0ae2ca47ec9029c3ff5c50a897 Mon Sep 17 00:00:00 2001 From: James Forcier Date: Thu, 3 Aug 2017 18:13:31 -0700 Subject: [PATCH 2/9] util/gpt: support reading GPT headers --- src/util/gpt.rs | 159 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/mod.rs | 2 + 2 files changed, 161 insertions(+) create mode 100644 src/util/gpt.rs diff --git a/src/util/gpt.rs b/src/util/gpt.rs new file mode 100644 index 0000000..e5ecba9 --- /dev/null +++ b/src/util/gpt.rs @@ -0,0 +1,159 @@ +// Copyright 2017 CoreOS, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied +// See the License for the specific language governing permissions and +// limitations under the License. + +extern crate uefi; + +use uefi::{Guid, Handle, Status, GET_PROTOCOL}; +use uefi::protocol::{get_current_image_handle, BlockIOProtocol, DevicePathProtocol}; + +// "EFI PART", u64 constant given in UEFI spec +const HEADER_SIGNATURE: u64 = 0x5452_4150_2049_4645; + +#[repr(C, packed)] +pub struct GptHeader { + signature: u64, + revision: u32, + header_size: u32, + header_crc32: u32, + _reserved: u32, + my_lba: u64, + alternate_lba: u64, + first_usable_lba: u64, + last_usable_lba: u64, + disk_guid: Guid, + partition_entry_lba: u64, + num_partition_entries: u32, + sizeof_partition_entry: u32, + partition_entry_array_crc32: u32, +} + +impl Drop for GptHeader { + fn drop(&mut self) { + uefi::get_system_table().boot_services().free_pool(self); + } +} + +impl GptHeader { + fn validate(&self, my_lba: u64, block_io: &'static BlockIOProtocol) -> Result<(), Status> { + let bs = uefi::get_system_table().boot_services(); + + if self.signature != HEADER_SIGNATURE { + return Err(Status::InvalidParameter); + } + + if self.my_lba != my_lba { + // FIXME(csssuf): is there a better error to use here? spec doesn't say + return Err(Status::VolumeCorrupted); + } + + block_io + .read_blocks(self.my_lba, 1) + .and_then(|header_copy_block| { + let this_header = + unsafe { &mut *(header_copy_block.as_mut_ptr() as *mut GptHeader) }; + + this_header.header_crc32 = 0; + + bs.calculate_crc32_sized(this_header, self.header_size as usize) + .and_then(|crc32| { + if crc32 != self.header_crc32 { + return Err(Status::CrcError); + } + + Ok(()) + }) + }) + .and_then(|_| { + let partition_entry_table_size = + (self.num_partition_entries * self.sizeof_partition_entry) as usize; + + block_io + .read_bytes(self.partition_entry_lba, partition_entry_table_size) + .and_then(|entry_table| { + bs.calculate_crc32_sized(entry_table.as_ptr(), partition_entry_table_size) + .and_then(|crc32| { + if crc32 != self.partition_entry_array_crc32 { + return Err(Status::CrcError); + } + + Ok(()) + }) + }) + }) + } +} + +pub struct GptDisk<'a> { + block_device: &'static BlockIOProtocol, + primary_header: &'a GptHeader, + alternate_header: &'a GptHeader, +} + +impl<'a> GptDisk<'a> { + /// Read the GPT header from a given device, and perform all necessary validation on it. + pub fn read_from(device: &DevicePathProtocol) -> Result { + let bs = uefi::get_system_table().boot_services(); + + bs.locate_device_path::(device) + .and_then(|(handle, usable_device)| { + bs.open_protocol::( + handle, + get_current_image_handle(), + Handle::default(), + GET_PROTOCOL, + ).and_then(|protocol| { + // Read 1 block starting at block 1. + protocol.read_blocks(1, 1).and_then(|block| { + let mut out = GptDisk { + block_device: protocol, + primary_header: unsafe { &*(block.as_ptr() as *const GptHeader) }, + alternate_header: unsafe { &*(block.as_ptr() as *const GptHeader) }, + }; + + unsafe { + out.validate().map(|_| out).map_err(|e| { + bs.free_pool(block.as_ptr()); + e + }) + } + }) + }) + }) + } + + /// Validate an instance of a `GptHeader`. + /// The UEFI spec gives the steps required to validate a GPT header as follows: + /// >* Check the Signature + /// >* Check the Header CRC + /// >* Check that the MyLBA entry points to the LBA that contains the GUID Partition Table + /// >* Check the CRC of the GUID Partition Entry Array + /// + /// >If the GPT is the primary table, stored at LBA 1: + /// + /// >* Check the AlternateLBA to see if it is a valid GPT + unsafe fn validate(&mut self) -> Result<(), Status> { + // The primary header is always read from LBA 1, so we call `validate` with that value. + self.primary_header + .validate(1, self.block_device) + .and_then(|_| { + self.block_device + .read_blocks(self.primary_header.alternate_lba, 1) + .and_then(|block| { + self.alternate_header = &*(block.as_ptr() as *const GptHeader); + self.alternate_header + .validate(self.primary_header.alternate_lba, self.block_device) + }) + }) + } +} diff --git a/src/util/mod.rs b/src/util/mod.rs index b890520..c9ffe60 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -14,8 +14,10 @@ extern crate uefi; +mod gpt; mod input; +pub use self::gpt::*; pub use self::input::*; use core::fmt::Arguments; From 776d3c80c3bb5cbbfceb059c0a5be5f8d516b96a Mon Sep 17 00:00:00 2001 From: James Forcier Date: Mon, 7 Aug 2017 14:16:19 -0700 Subject: [PATCH 3/9] util/gpt: support reading GPT partitions from disk --- src/util/gpt.rs | 69 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 66 insertions(+), 3 deletions(-) diff --git a/src/util/gpt.rs b/src/util/gpt.rs index e5ecba9..2c1aae4 100644 --- a/src/util/gpt.rs +++ b/src/util/gpt.rs @@ -14,6 +14,8 @@ extern crate uefi; +use core::{mem, slice}; + use uefi::{Guid, Handle, Status, GET_PROTOCOL}; use uefi::protocol::{get_current_image_handle, BlockIOProtocol, DevicePathProtocol}; @@ -94,6 +96,30 @@ impl GptHeader { } } +#[derive(Copy)] +#[repr(C, packed)] +pub struct GptPartitionEntry { + pub(crate) partition_type_guid: Guid, + pub(crate) unique_partition_guid: Guid, + starting_lba: u64, + ending_lba: u64, + pub(crate) attributes: u64, + pub(crate) partition_name: [u16; 36], +} + +impl Clone for GptPartitionEntry { + fn clone(&self) -> GptPartitionEntry { + GptPartitionEntry { + partition_type_guid: self.partition_type_guid, + unique_partition_guid: self.unique_partition_guid, + starting_lba: self.starting_lba, + ending_lba: self.ending_lba, + attributes: self.attributes, + partition_name: self.partition_name, + } + } +} + pub struct GptDisk<'a> { block_device: &'static BlockIOProtocol, primary_header: &'a GptHeader, @@ -105,8 +131,8 @@ impl<'a> GptDisk<'a> { pub fn read_from(device: &DevicePathProtocol) -> Result { let bs = uefi::get_system_table().boot_services(); - bs.locate_device_path::(device) - .and_then(|(handle, usable_device)| { + bs.locate_device_path::(device).and_then( + |(handle, _usable_device)| { bs.open_protocol::( handle, get_current_image_handle(), @@ -129,7 +155,8 @@ impl<'a> GptDisk<'a> { } }) }) - }) + }, + ) } /// Validate an instance of a `GptHeader`. @@ -156,4 +183,40 @@ impl<'a> GptDisk<'a> { }) }) } + + /// Read the partition entry array from this disk and return it. + pub fn read_partitions(&self) -> Result<&[&mut GptPartitionEntry], Status> { + let bs = uefi::get_system_table().boot_services(); + + let num_partitions = self.primary_header.num_partition_entries as usize; + let partition_entry_table_size = (self.primary_header.num_partition_entries * + self.primary_header.sizeof_partition_entry) as + usize; + + self.block_device + .read_bytes( + self.primary_header.partition_entry_lba, + partition_entry_table_size, + ) + .and_then(|partition_entry_table| { + bs.allocate_pool::<&mut GptPartitionEntry>( + num_partitions * mem::size_of::<&mut GptPartitionEntry>(), + ).map(|entries_ptr| { + let entries = + unsafe { slice::from_raw_parts_mut(entries_ptr, num_partitions) }; + for part_number in 0..(self.primary_header.num_partition_entries) { + let offset = + (part_number * self.primary_header.sizeof_partition_entry) as isize; + + unsafe { + let entry_ptr = partition_entry_table.as_ptr().offset(offset); + let entry = &mut *(entry_ptr as *mut GptPartitionEntry); + (*entries)[part_number as usize] = entry; + } + } + + &*entries + }) + }) + } } From 6b1e8636a535d06d9270203c61dac02c0f0758fc Mon Sep 17 00:00:00 2001 From: James Forcier Date: Mon, 7 Aug 2017 14:49:34 -0700 Subject: [PATCH 4/9] util/gptprio: add read-only gptprio implementation --- Cargo.lock | 7 ++++++ Cargo.toml | 1 + src/picker.rs | 2 ++ src/util/gptprio.rs | 53 +++++++++++++++++++++++++++++++++++++++++++++ src/util/mod.rs | 1 + 5 files changed, 64 insertions(+) create mode 100644 src/util/gptprio.rs diff --git a/Cargo.lock b/Cargo.lock index ff58ebe..e365813 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,10 +2,16 @@ name = "picker" version = "0.1.0" dependencies = [ + "bitfield 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)", "rlibc 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "uefi 0.1.2 (git+https://github.com/csssuf/rust-uefi)", ] +[[package]] +name = "bitfield" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "bitflags" version = "0.9.1" @@ -25,6 +31,7 @@ dependencies = [ ] [metadata] +"checksum bitfield 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)" = "eb988c10184429ae27919881dd9ce0298b87557d12c508e664353eafb336580e" "checksum bitflags 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)" = "4efd02e230a02e18f92fc2735f44597385ed02ad8f831e7c1c1156ee5e1ab3a5" "checksum rlibc 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "fc874b127765f014d792f16763a81245ab80500e2ad921ed4ee9e82481ee08fe" "checksum uefi 0.1.2 (git+https://github.com/csssuf/rust-uefi)" = "" diff --git a/Cargo.toml b/Cargo.toml index bcf5476..1f5658d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,3 +11,4 @@ path = "src/picker.rs" [dependencies] uefi = { git = "https://github.com/csssuf/rust-uefi" } rlibc = "1.0" +bitfield = "0.12.0" diff --git a/src/picker.rs b/src/picker.rs index b2a4a96..f882895 100644 --- a/src/picker.rs +++ b/src/picker.rs @@ -15,6 +15,8 @@ #![no_std] #![feature(lang_items)] +#[macro_use] +extern crate bitfield; extern crate rlibc; extern crate uefi; diff --git a/src/util/gptprio.rs b/src/util/gptprio.rs new file mode 100644 index 0000000..d5a84e2 --- /dev/null +++ b/src/util/gptprio.rs @@ -0,0 +1,53 @@ +// Copyright 2017 CoreOS, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied +// See the License for the specific language governing permissions and +// limitations under the License. + +use util::gpt::GptPartitionEntry; + +use uefi::Guid; + +static COREOS_ROOTFS_TYPE: Guid = Guid( + 0x5DFB_F5F4, + 0x2848, + 0x4BAC, + [0xAA, 0x5E, 0x0D, 0x9A, 0x20, 0xB7, 0x45, 0xA6], +); + +bitfield! { + struct GptprioAttributes(u64); + _required, _: 0; + _no_block_io, _: 1; + _legacy_bios_bootable, _: 2; + _unused1, _: 47, 3; + gptprio_priority, _: 51, 48; + gptprio_tries_left, _: 55, 52; + gptprio_successful, _: 56; + _unused2, _: 63, 57; +} + +pub fn next(partitions: &[&mut GptPartitionEntry]) -> Option { + let partition_iter = partitions.into_iter(); + + partition_iter + .filter(|partition| { + partition.partition_type_guid == COREOS_ROOTFS_TYPE + }) + .filter(|partition| { + let attributes = GptprioAttributes(partition.attributes); + attributes.gptprio_successful() || attributes.gptprio_tries_left() > 0 + }) + .max_by_key(|partition| { + GptprioAttributes(partition.attributes).gptprio_priority() + }) + .map(|part| **part) +} diff --git a/src/util/mod.rs b/src/util/mod.rs index c9ffe60..e911703 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -15,6 +15,7 @@ extern crate uefi; mod gpt; +pub mod gptprio; mod input; pub use self::gpt::*; From a05bdca5917c437dbaadc7a5ab52259cffd1df45 Mon Sep 17 00:00:00 2001 From: James Forcier Date: Tue, 8 Aug 2017 14:06:13 -0700 Subject: [PATCH 5/9] picker: choose default boot option via gptprio On boot, determine the partition gptprio specifies, and use it as the default. As part of the boot menu, display the default choice to the user. --- src/boot/mod.rs | 4 ++- src/menu/mod.rs | 14 +++++++- src/picker.rs | 89 +++++++++++++++++++++++++++++++++++++++++-------- 3 files changed, 92 insertions(+), 15 deletions(-) diff --git a/src/boot/mod.rs b/src/boot/mod.rs index 259ec0f..d5bb331 100644 --- a/src/boot/mod.rs +++ b/src/boot/mod.rs @@ -18,10 +18,12 @@ use core::ptr; use uefi::*; -// When gptprio is implemented, boot_data may change to another type. For now it's just a path. +#[derive(Clone, Copy)] pub struct BootOption { pub display: &'static str, pub boot_data: &'static str, + pub default: bool, + pub guid: Guid, } fn str_to_device_path(image: &str) -> Result<&protocol::DevicePathProtocol, Status> { diff --git a/src/menu/mod.rs b/src/menu/mod.rs index fad8bad..c140b4b 100644 --- a/src/menu/mod.rs +++ b/src/menu/mod.rs @@ -35,10 +35,16 @@ where loop { write("Option 1: "); write(option_a.display); + if option_a.default { + write(" (default)"); + } write("\r\n"); write("Option 2: "); write(option_b.display); + if option_b.default { + write(" (default)"); + } write("\r\n"); write("Enter boot choice: "); @@ -50,7 +56,13 @@ where }, Ok(None) => { write("Taking default.\r\n"); - return Ok(None); + if option_a.default { + return Ok(Some(option_a)); + } else if option_b.default { + return Ok(Some(option_b)); + } else { + return Ok(None); + } } Err(e) => { write("Error reading: "); diff --git a/src/picker.rs b/src/picker.rs index f882895..8643149 100644 --- a/src/picker.rs +++ b/src/picker.rs @@ -20,7 +20,8 @@ extern crate bitfield; extern crate rlibc; extern crate uefi; -use uefi::*; +use uefi::{Guid, Handle, SimpleTextOutput, Status}; +use uefi::util::parent_device_path; pub mod boot; pub mod menu; @@ -29,28 +30,90 @@ pub mod uefi_entry; use boot::BootOption; -const BOOTPATH_1: &'static str = "\\efi\\boot\\shim_a.efi"; -const BOOTPATH_2: &'static str = "\\efi\\boot\\shim_b.efi"; +const BOOTPATH_USR_A: &'static str = "\\EFI\\coreos\\shim_a.efi"; +const BOOTPATH_USR_B: &'static str = "\\EFI\\coreos\\shim_b.efi"; + +const PART_UUID_USR_A: Guid = Guid( + 0x7130_C94A, + 0x213A, + 0x4E5A, + [0x8E, 0x26, 0x6C, 0xCE, 0x96, 0x62, 0xF1, 0x32], +); +const PART_UUID_USR_B: Guid = Guid( + 0xE03D_D35C, + 0x7C2D, + 0x4A47, + [0xB3, 0xFE, 0x27, 0xF1, 0x57, 0x80, 0xA5, 0x7C], +); pub fn efi_main(image_handle: Handle) -> Status { let sys_table = uefi::get_system_table(); + let bs = sys_table.boot_services(); let cons = sys_table.console(); cons.write("picker v0.0.1\r\n"); - let option_a = BootOption { - display: "application a", - boot_data: BOOTPATH_1, + let mut option_a = BootOption { + display: "USR-A", + boot_data: BOOTPATH_USR_A, + default: false, + guid: PART_UUID_USR_A, }; - let option_b = BootOption { - display: "application b", - boot_data: BOOTPATH_2, + let mut option_b = BootOption { + display: "USR-B", + boot_data: BOOTPATH_USR_B, + default: false, + guid: PART_UUID_USR_B, }; - match menu::boot_menu(&option_a, &option_b).and_then(|option| { - let result = option.unwrap_or(&option_a); - boot::boot(result, image_handle) - }) { + let this = uefi::protocol::get_current_image(); + let partition = bs.handle_protocol::(this.device_handle) + .and_then(parent_device_path) + .and_then(|parent_path| util::GptDisk::read_from(parent_path)) + .and_then(|disk| disk.read_partitions().map(util::gptprio::next)); + + match partition { + Ok(Some(gptprio_partition)) => { + if option_a.guid == gptprio_partition.unique_partition_guid { + option_a.default = true; + } else if option_b.guid == gptprio_partition.unique_partition_guid { + option_b.default = true; + } else { + cons.write( + "Unknown gptprio partition chosen as next. Defaulting to USR-A.\r\n", + ); + option_a.default = true; + } + } + Ok(None) => { + cons.write( + "No acceptable gptprio partitions found; defaulting to USR-A.\r\n", + ); + option_a.default = true; + } + Err(e) => { + cons.write("error reading from disk: "); + cons.write(e.str()); + return e; + } + } + + let boot_result = menu::boot_menu(&option_a, &option_b) + .and_then(|option| { + match option { + Some(boot_choice) => Ok(boot_choice), + None => { + cons.write( + "No option selected and no default was set. Can't proceed.\r\n", + ); + // FIXME(csssuf) is this the best error to use here? + Err(Status::NoMedia) + } + } + }) + .and_then(|boot_option| boot::boot(boot_option, image_handle)); + + match boot_result { Ok(_) => Status::Success, Err(e) => e, } From d1be63b4f8c718621c35a78886b1c313c5415574 Mon Sep 17 00:00:00 2001 From: James Forcier Date: Thu, 10 Aug 2017 11:55:53 -0700 Subject: [PATCH 6/9] review: rework header validation design --- src/util/gpt.rs | 138 +++++++++++++++++++++++------------------------- 1 file changed, 66 insertions(+), 72 deletions(-) diff --git a/src/util/gpt.rs b/src/util/gpt.rs index 2c1aae4..af5865d 100644 --- a/src/util/gpt.rs +++ b/src/util/gpt.rs @@ -14,7 +14,7 @@ extern crate uefi; -use core::{mem, slice}; +use core::{mem, ptr, slice}; use uefi::{Guid, Handle, Status, GET_PROTOCOL}; use uefi::protocol::{get_current_image_handle, BlockIOProtocol, DevicePathProtocol}; @@ -47,7 +47,7 @@ impl Drop for GptHeader { } impl GptHeader { - fn validate(&self, my_lba: u64, block_io: &'static BlockIOProtocol) -> Result<(), Status> { + fn validate(&mut self, my_lba: u64) -> Result<(), Status> { let bs = uefi::get_system_table().boot_services(); if self.signature != HEADER_SIGNATURE { @@ -59,40 +59,17 @@ impl GptHeader { return Err(Status::VolumeCorrupted); } - block_io - .read_blocks(self.my_lba, 1) - .and_then(|header_copy_block| { - let this_header = - unsafe { &mut *(header_copy_block.as_mut_ptr() as *mut GptHeader) }; + let my_crc32 = self.header_crc32; - this_header.header_crc32 = 0; + self.header_crc32 = 0; + let crc32 = bs.calculate_crc32_sized(self, self.header_size as usize)?; + self.header_crc32 = my_crc32; - bs.calculate_crc32_sized(this_header, self.header_size as usize) - .and_then(|crc32| { - if crc32 != self.header_crc32 { - return Err(Status::CrcError); - } + if crc32 != self.header_crc32 { + return Err(Status::CrcError); + } - Ok(()) - }) - }) - .and_then(|_| { - let partition_entry_table_size = - (self.num_partition_entries * self.sizeof_partition_entry) as usize; - - block_io - .read_bytes(self.partition_entry_lba, partition_entry_table_size) - .and_then(|entry_table| { - bs.calculate_crc32_sized(entry_table.as_ptr(), partition_entry_table_size) - .and_then(|crc32| { - if crc32 != self.partition_entry_array_crc32 { - return Err(Status::CrcError); - } - - Ok(()) - }) - }) - }) + Ok(()) } } @@ -122,8 +99,8 @@ impl Clone for GptPartitionEntry { pub struct GptDisk<'a> { block_device: &'static BlockIOProtocol, - primary_header: &'a GptHeader, - alternate_header: &'a GptHeader, + primary_header: &'a mut GptHeader, + alternate_header: &'a mut GptHeader, } impl<'a> GptDisk<'a> { @@ -131,32 +108,39 @@ impl<'a> GptDisk<'a> { pub fn read_from(device: &DevicePathProtocol) -> Result { let bs = uefi::get_system_table().boot_services(); - bs.locate_device_path::(device).and_then( - |(handle, _usable_device)| { - bs.open_protocol::( - handle, - get_current_image_handle(), - Handle::default(), - GET_PROTOCOL, - ).and_then(|protocol| { - // Read 1 block starting at block 1. - protocol.read_blocks(1, 1).and_then(|block| { - let mut out = GptDisk { - block_device: protocol, - primary_header: unsafe { &*(block.as_ptr() as *const GptHeader) }, - alternate_header: unsafe { &*(block.as_ptr() as *const GptHeader) }, - }; - - unsafe { - out.validate().map(|_| out).map_err(|e| { - bs.free_pool(block.as_ptr()); - e - }) - } - }) - }) - }, - ) + let (handle, _usable_device) = bs.locate_device_path::(device)?; + let protocol = bs.open_protocol::( + handle, + get_current_image_handle(), + Handle::default(), + GET_PROTOCOL, + )?; + + let mut out = GptDisk { + block_device: protocol, + primary_header: unsafe { &mut *(ptr::null_mut()) }, + alternate_header: unsafe { &mut *(ptr::null_mut()) }, + }; + + let primary_block = protocol.read_blocks(1, 1)?; + let primary_header = unsafe { &mut *(primary_block.as_mut_ptr() as *mut GptHeader) }; + + let alternate_block = protocol.read_blocks(primary_header.alternate_lba, 1)?; + let alternate_header = unsafe { &mut *(alternate_block.as_mut_ptr() as *mut GptHeader) }; + + out.primary_header = primary_header; + out.alternate_header = alternate_header; + + match unsafe { out.validate() } { + Ok(_) => Ok(out), + Err(e) => { + bs.free_pool(primary_block.as_ptr()); + mem::forget(out.primary_header); + bs.free_pool(alternate_block.as_ptr()); + mem::forget(out.alternate_header); + Err(e) + } + } } /// Validate an instance of a `GptHeader`. @@ -171,17 +155,20 @@ impl<'a> GptDisk<'a> { /// >* Check the AlternateLBA to see if it is a valid GPT unsafe fn validate(&mut self) -> Result<(), Status> { // The primary header is always read from LBA 1, so we call `validate` with that value. - self.primary_header - .validate(1, self.block_device) - .and_then(|_| { - self.block_device - .read_blocks(self.primary_header.alternate_lba, 1) - .and_then(|block| { - self.alternate_header = &*(block.as_ptr() as *const GptHeader); - self.alternate_header - .validate(self.primary_header.alternate_lba, self.block_device) - }) - }) + self.primary_header.validate(1)?; + self.alternate_header + .validate(self.primary_header.alternate_lba)?; + + let partitions = self.read_partitions_raw()?; + let partition_crc32 = uefi::get_system_table() + .boot_services() + .calculate_crc32_sized(partitions.as_ptr(), partitions.len())?; + + if partition_crc32 != self.primary_header.partition_entry_array_crc32 { + return Err(Status::CrcError); + } + + Ok(()) } /// Read the partition entry array from this disk and return it. @@ -219,4 +206,11 @@ impl<'a> GptDisk<'a> { }) }) } + + fn read_partitions_raw(&self) -> Result<&mut [u8], Status> { + let read_size = (self.primary_header.num_partition_entries * + self.primary_header.sizeof_partition_entry) as usize; + self.block_device + .read_bytes(self.primary_header.partition_entry_lba, read_size) + } } From 05b8bfb938058d4a78ba804fae8c983b23e2148d Mon Sep 17 00:00:00 2001 From: James Forcier Date: Thu, 10 Aug 2017 11:57:21 -0700 Subject: [PATCH 7/9] review: use read_partitions_raw in read_partitions --- src/util/gpt.rs | 45 +++++++++++++++++---------------------------- 1 file changed, 17 insertions(+), 28 deletions(-) diff --git a/src/util/gpt.rs b/src/util/gpt.rs index af5865d..1b97284 100644 --- a/src/util/gpt.rs +++ b/src/util/gpt.rs @@ -176,35 +176,24 @@ impl<'a> GptDisk<'a> { let bs = uefi::get_system_table().boot_services(); let num_partitions = self.primary_header.num_partition_entries as usize; - let partition_entry_table_size = (self.primary_header.num_partition_entries * - self.primary_header.sizeof_partition_entry) as - usize; + let partition_entry_table = self.read_partitions_raw()?; - self.block_device - .read_bytes( - self.primary_header.partition_entry_lba, - partition_entry_table_size, - ) - .and_then(|partition_entry_table| { - bs.allocate_pool::<&mut GptPartitionEntry>( - num_partitions * mem::size_of::<&mut GptPartitionEntry>(), - ).map(|entries_ptr| { - let entries = - unsafe { slice::from_raw_parts_mut(entries_ptr, num_partitions) }; - for part_number in 0..(self.primary_header.num_partition_entries) { - let offset = - (part_number * self.primary_header.sizeof_partition_entry) as isize; - - unsafe { - let entry_ptr = partition_entry_table.as_ptr().offset(offset); - let entry = &mut *(entry_ptr as *mut GptPartitionEntry); - (*entries)[part_number as usize] = entry; - } - } - - &*entries - }) - }) + let entries_ptr = bs.allocate_pool::<&mut GptPartitionEntry>( + num_partitions * mem::size_of::<&mut GptPartitionEntry>(), + )?; + + let entries = unsafe { slice::from_raw_parts_mut(entries_ptr, num_partitions) }; + for part_number in 0..(self.primary_header.num_partition_entries) { + let offset = (part_number * self.primary_header.sizeof_partition_entry) as isize; + + unsafe { + let entry_ptr = partition_entry_table.as_ptr().offset(offset); + let entry = &mut *(entry_ptr as *mut GptPartitionEntry); + (*entries)[part_number as usize] = entry; + } + } + + Ok(&*entries) } fn read_partitions_raw(&self) -> Result<&mut [u8], Status> { From cc53a1209a4d00ead1d8367c102c57ebe6a1f8ee Mon Sep 17 00:00:00 2001 From: James Forcier Date: Thu, 10 Aug 2017 13:10:34 -0700 Subject: [PATCH 8/9] review: properly free GptHeader structs --- src/util/gpt.rs | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/util/gpt.rs b/src/util/gpt.rs index 1b97284..c3be87f 100644 --- a/src/util/gpt.rs +++ b/src/util/gpt.rs @@ -40,12 +40,6 @@ pub struct GptHeader { partition_entry_array_crc32: u32, } -impl Drop for GptHeader { - fn drop(&mut self) { - uefi::get_system_table().boot_services().free_pool(self); - } -} - impl GptHeader { fn validate(&mut self, my_lba: u64) -> Result<(), Status> { let bs = uefi::get_system_table().boot_services(); @@ -103,6 +97,25 @@ pub struct GptDisk<'a> { alternate_header: &'a mut GptHeader, } +impl<'a> Drop for GptDisk<'a> { + fn drop(&mut self) { + let bs = uefi::get_system_table().boot_services(); + if self.block_device.must_align() { + bs.free_pages( + self.primary_header, + self.block_device.required_pages_block(1), + ); + bs.free_pages( + self.alternate_header, + self.block_device.required_pages_block(1), + ); + } else { + bs.free_pool(self.primary_header); + bs.free_pool(self.alternate_header); + } + } +} + impl<'a> GptDisk<'a> { /// Read the GPT header from a given device, and perform all necessary validation on it. pub fn read_from(device: &DevicePathProtocol) -> Result { @@ -131,16 +144,7 @@ impl<'a> GptDisk<'a> { out.primary_header = primary_header; out.alternate_header = alternate_header; - match unsafe { out.validate() } { - Ok(_) => Ok(out), - Err(e) => { - bs.free_pool(primary_block.as_ptr()); - mem::forget(out.primary_header); - bs.free_pool(alternate_block.as_ptr()); - mem::forget(out.alternate_header); - Err(e) - } - } + unsafe { out.validate().map(|_| out) } } /// Validate an instance of a `GptHeader`. From ece55f7fd62b74bbb0d18758e2bd63a3e8d1b96c Mon Sep 17 00:00:00 2001 From: James Forcier Date: Fri, 11 Aug 2017 15:31:52 -0700 Subject: [PATCH 9/9] review: split up gpt reading/validation functions By splitting up the validation stage into validate_headers() and validate_partitions(), and splitting each of those into a reading and validation stage, leaks are avoided. --- src/picker.rs | 2 +- src/util/gpt.rs | 77 ++++++++++++++++++++++++++++--------------------- 2 files changed, 45 insertions(+), 34 deletions(-) diff --git a/src/picker.rs b/src/picker.rs index 8643149..5cdc266 100644 --- a/src/picker.rs +++ b/src/picker.rs @@ -70,7 +70,7 @@ pub fn efi_main(image_handle: Handle) -> Status { let partition = bs.handle_protocol::(this.device_handle) .and_then(parent_device_path) .and_then(|parent_path| util::GptDisk::read_from(parent_path)) - .and_then(|disk| disk.read_partitions().map(util::gptprio::next)); + .map(|disk| util::gptprio::next(disk.partitions)); match partition { Ok(Some(gptprio_partition)) => { diff --git a/src/util/gpt.rs b/src/util/gpt.rs index c3be87f..647e49d 100644 --- a/src/util/gpt.rs +++ b/src/util/gpt.rs @@ -14,7 +14,7 @@ extern crate uefi; -use core::{mem, ptr, slice}; +use core::{mem, slice}; use uefi::{Guid, Handle, Status, GET_PROTOCOL}; use uefi::protocol::{get_current_image_handle, BlockIOProtocol, DevicePathProtocol}; @@ -91,28 +91,29 @@ impl Clone for GptPartitionEntry { } } +type GptPartitionTable<'a> = &'a [&'a mut GptPartitionEntry]; + pub struct GptDisk<'a> { block_device: &'static BlockIOProtocol, primary_header: &'a mut GptHeader, alternate_header: &'a mut GptHeader, + raw_partitions: &'a mut [u8], + pub partitions: GptPartitionTable<'a>, } impl<'a> Drop for GptDisk<'a> { fn drop(&mut self) { let bs = uefi::get_system_table().boot_services(); - if self.block_device.must_align() { - bs.free_pages( - self.primary_header, - self.block_device.required_pages_block(1), - ); - bs.free_pages( - self.alternate_header, - self.block_device.required_pages_block(1), - ); - } else { - bs.free_pool(self.primary_header); - bs.free_pool(self.alternate_header); - } + + self.block_device.free_read(self.raw_partitions); + bs.free_pages( + self.primary_header, + self.block_device.required_pages_block(1), + ); + bs.free_pages( + self.alternate_header, + self.block_device.required_pages_block(1), + ); } } @@ -129,22 +130,27 @@ impl<'a> GptDisk<'a> { GET_PROTOCOL, )?; - let mut out = GptDisk { - block_device: protocol, - primary_header: unsafe { &mut *(ptr::null_mut()) }, - alternate_header: unsafe { &mut *(ptr::null_mut()) }, - }; - let primary_block = protocol.read_blocks(1, 1)?; let primary_header = unsafe { &mut *(primary_block.as_mut_ptr() as *mut GptHeader) }; let alternate_block = protocol.read_blocks(primary_header.alternate_lba, 1)?; let alternate_header = unsafe { &mut *(alternate_block.as_mut_ptr() as *mut GptHeader) }; - out.primary_header = primary_header; - out.alternate_header = alternate_header; + let mut out = GptDisk { + block_device: protocol, + primary_header: primary_header, + alternate_header: alternate_header, + raw_partitions: &mut [], + partitions: &[], + }; + + unsafe { out.validate_headers()? }; + + out.read_partitions()?; - unsafe { out.validate().map(|_| out) } + unsafe { out.validate_partitions()? }; + + Ok(out) } /// Validate an instance of a `GptHeader`. @@ -157,16 +163,19 @@ impl<'a> GptDisk<'a> { /// >If the GPT is the primary table, stored at LBA 1: /// /// >* Check the AlternateLBA to see if it is a valid GPT - unsafe fn validate(&mut self) -> Result<(), Status> { + unsafe fn validate_headers(&mut self) -> Result<(), Status> { // The primary header is always read from LBA 1, so we call `validate` with that value. self.primary_header.validate(1)?; self.alternate_header .validate(self.primary_header.alternate_lba)?; - let partitions = self.read_partitions_raw()?; + Ok(()) + } + + unsafe fn validate_partitions(&mut self) -> Result<(), Status> { let partition_crc32 = uefi::get_system_table() .boot_services() - .calculate_crc32_sized(partitions.as_ptr(), partitions.len())?; + .calculate_crc32_sized(self.raw_partitions.as_ptr(), self.raw_partitions.len())?; if partition_crc32 != self.primary_header.partition_entry_array_crc32 { return Err(Status::CrcError); @@ -176,11 +185,11 @@ impl<'a> GptDisk<'a> { } /// Read the partition entry array from this disk and return it. - pub fn read_partitions(&self) -> Result<&[&mut GptPartitionEntry], Status> { + fn read_partitions(&mut self) -> Result<(), Status> { let bs = uefi::get_system_table().boot_services(); let num_partitions = self.primary_header.num_partition_entries as usize; - let partition_entry_table = self.read_partitions_raw()?; + self.read_raw_partitions()?; let entries_ptr = bs.allocate_pool::<&mut GptPartitionEntry>( num_partitions * mem::size_of::<&mut GptPartitionEntry>(), @@ -191,19 +200,21 @@ impl<'a> GptDisk<'a> { let offset = (part_number * self.primary_header.sizeof_partition_entry) as isize; unsafe { - let entry_ptr = partition_entry_table.as_ptr().offset(offset); + let entry_ptr = self.raw_partitions.as_ptr().offset(offset); let entry = &mut *(entry_ptr as *mut GptPartitionEntry); (*entries)[part_number as usize] = entry; } } - Ok(&*entries) + self.partitions = entries; + Ok(()) } - fn read_partitions_raw(&self) -> Result<&mut [u8], Status> { + fn read_raw_partitions(&mut self) -> Result<(), Status> { let read_size = (self.primary_header.num_partition_entries * self.primary_header.sizeof_partition_entry) as usize; - self.block_device - .read_bytes(self.primary_header.partition_entry_lba, read_size) + self.raw_partitions = self.block_device + .read_bytes(self.primary_header.partition_entry_lba, read_size)?; + Ok(()) } }