Skip to content

Commit

Permalink
blockdev.rs: --save-partindex will now check for MBR disk
Browse files Browse the repository at this point in the history
If running `install` with the `----save-partindex` argument on a MBR drive
the listed partitions would be lost durring the install. To prevent the loss
of data the install proccess now checks to see if the drive is MBR, and if it
is it will bail fixes coreos#816
  • Loading branch information
prestist committed Sep 13, 2022
1 parent eabfdd4 commit c84b0af
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 10 deletions.
1 change: 1 addition & 0 deletions docs/release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Minor changes:
- Add release notes to documentation
- iso: Detect incomplete ISO files
- Warn if console kargs could have used `--console`/`--dest-console` instead
- install: bail when `--save-partindex` is specified on a MBR disk

Internal changes:

Expand Down
36 changes: 26 additions & 10 deletions src/blockdev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,10 @@ impl SavedPartitions {
let gpt = match GPT::find_from(disk) {
Ok(gpt) => gpt,
Err(gptman::Error::InvalidSignature) => {
if disk_has_mbr(disk).context("checking if disk has an MBR")? {
bail!("saving partitions from an MBR disk is not yet supported");
}

// no GPT on this disk, so no partitions to save
return Ok(Self {
sector_size,
Expand Down Expand Up @@ -1103,6 +1107,15 @@ pub fn get_gpt_size(file: &mut (impl Read + Seek)) -> Result<u64> {
Ok(gpt.header.first_usable_lba * gpt.sector_size)
}

fn disk_has_mbr(file: &mut (impl Read + Seek)) -> Result<bool> {
let mut sig = [0u8; 2];
file.seek(SeekFrom::Start(510))
.context("seeking to MBR signature")?;
file.read_exact(&mut sig)
.context("reading MBR signature")?;
Ok(sig == [0x55, 0xaa])
}

pub fn udev_settle() -> Result<()> {
// "udevadm settle" silently no-ops if the udev socket is missing, and
// then lsblk can't find partition labels. Catch this early.
Expand Down Expand Up @@ -1469,7 +1482,7 @@ mod tests {
let saved = SavedPartitions::new_from_file(&mut base, 512, filter).unwrap();
let mut disk = make_unformatted_disk();
saved.overwrite(&mut disk).unwrap();
assert!(disk_has_mbr(&mut disk), "test {}", testnum);
assert!(disk_has_mbr(&mut disk).unwrap(), "test {}", testnum);
let result = GPT::find_from(&mut disk).unwrap();
assert_eq!(
get_gpt_size(&mut disk).unwrap(),
Expand All @@ -1481,7 +1494,7 @@ mod tests {
let mut disk = make_disk(512, &merge_base_parts);
saved.merge(&mut image, &mut disk).unwrap();
assert!(
disk_has_mbr(&mut disk) == !expected_blank.is_empty(),
disk_has_mbr(&mut disk).unwrap() == !expected_blank.is_empty(),
"test {}",
testnum
);
Expand Down Expand Up @@ -1515,7 +1528,7 @@ mod tests {
let saved =
SavedPartitions::new_from_file(&mut disk, *sector_size as u64, &vec![]).unwrap();
saved.overwrite(&mut disk).unwrap();
assert!(disk_has_mbr(&mut disk), "{}", *sector_size);
assert!(disk_has_mbr(&mut disk).unwrap(), "{}", *sector_size);
disk.seek(SeekFrom::Start(0)).unwrap();
let mut buf = vec![0u8; *sector_size + 1];
disk.read_exact(&mut buf).unwrap();
Expand Down Expand Up @@ -1549,6 +1562,16 @@ mod tests {
err
);

// test trying to save partitions from a MBR disk
let mut disk = make_unformatted_disk();
gptman::GPT::write_protective_mbr_into(&mut disk, 512).unwrap();
assert_eq!(
SavedPartitions::new(&mut disk, 512, &vec![label("*i*")])
.unwrap_err()
.to_string(),
"saving partitions from an MBR disk is not yet supported"
);

// test sector size mismatch
let saved = SavedPartitions::new_from_file(&mut base, 512, &vec![label("*i*")]).unwrap();
let mut image_4096 = make_disk(4096, &image_parts);
Expand Down Expand Up @@ -1679,13 +1702,6 @@ mod tests {
guid
}

fn disk_has_mbr(disk: &mut File) -> bool {
let mut sig = [0u8; 2];
disk.seek(SeekFrom::Start(510)).unwrap();
disk.read_exact(&mut sig).unwrap();
sig == [0x55, 0xaa]
}

fn assert_partitions_eq(expected: &Vec<(u32, GPTPartitionEntry)>, found: &GPT, message: &str) {
assert_eq!(
expected
Expand Down

0 comments on commit c84b0af

Please sign in to comment.