Skip to content

Commit

Permalink
Defer unlock prompt until update is accepted
Browse files Browse the repository at this point in the history
Move unlocking from EC validation to the main app update logic. This
allows users to accept updating before unlocking, or cancel without
unlocking.

This does not resolve the issue of returning to firmware-update when
canceling/failing the unlock prompt, as the boot override still exists.

Signed-off-by: Tim Crawford <tcrawford@system76.com>
  • Loading branch information
crawfxrd committed Jun 28, 2023
1 parent d7d8fca commit 2bfb2f9
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 17 deletions.
18 changes: 1 addition & 17 deletions src/app/ec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ unsafe fn flash_legacy(firmware_data: &[u8]) -> core::result::Result<(), ectool:
Ok(())
}

unsafe fn security_unlock() -> core::result::Result<(), ectool::Error> {
pub unsafe fn security_unlock() -> core::result::Result<(), ectool::Error> {
let access = AccessLpcDirect::new(UefiTimeout::new(100_000))?;
let mut ec = ectool::Ec::new(access)?;

Expand Down Expand Up @@ -690,22 +690,6 @@ impl Component for EcComponent {
}

fn validate(&self) -> Result<bool> {
// If EC tag does not exist
if find(ECTAG).is_err() {
match &self.ec {
// Make sure EC is unlocked if running System76 EC
EcKind::System76(_, _) => match unsafe { security_unlock() } {
Ok(()) => (),
Err(err) => {
println!("{} Failed to unlock: {:?}", self.name(), err);
return Err(Error::DeviceError);
}
},
// Assume EC is unlocked if not running System76 EC
_ => (),
}
}

let data = load(self.path())?;
Ok(self.validate_data(data))
}
Expand Down
20 changes: 20 additions & 0 deletions src/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,26 @@ fn inner() -> Result<()> {

if c == '\n' || c == '\r' {
success = true;

{
let ec_kind = unsafe { EcKind::new(true) };
// If EC tag does not exist, unlock the firmware
if find(ECTAG).is_err() {
match ec_kind {
// Make sure EC is unlocked if running System76 EC
EcKind::System76(_, _) => match unsafe { ec::security_unlock() } {
Ok(()) => (),
Err(err) => {
println!("Failed to unlock firmware: {:?}", err);
return Err(Error::DeviceError);
}
},
// Assume EC is unlocked if not running System76 EC
_ => (),
}

Check warning on line 288 in src/app/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`

warning: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` --> src/app/mod.rs:277:21 | 277 | / match ec_kind { 278 | | // Make sure EC is unlocked if running System76 EC 279 | | EcKind::System76(_, _) => match unsafe { ec::security_unlock() } { 280 | | Ok(()) => (), ... | 287 | | _ => (), 288 | | } | |_____________________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_match help: try this | 277 ~ if let EcKind::System76(_, _) = ec_kind { match unsafe { ec::security_unlock() } { 278 + Ok(()) => (), 279 + Err(err) => { 280 + println!("Failed to unlock firmware: {:?}", err); 281 + return Err(Error::DeviceError); 282 + } 283 + } } |
}
}

for (component, validation) in components.iter().zip(validations.iter()) {
if *validation == ValidateKind::Found {
// Only shutdown if components are flashed
Expand Down

0 comments on commit 2bfb2f9

Please sign in to comment.