Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle ro /boot #118

Merged
merged 2 commits into from
Dec 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/bootupd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::component::{Component, ValidationResult};
use crate::coreos;
use crate::efi;
use crate::model::{ComponentStatus, ComponentUpdatable, ContentMetadata, SavedState, Status};
use crate::util;
use crate::{component, ipc};
use anyhow::{anyhow, Context, Result};
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -93,6 +94,10 @@ pub(crate) enum ComponentUpdateResult {
},
}

fn ensure_writable_boot() -> Result<()> {
util::ensure_writable_mount("/boot")
}

/// daemon implementation of component update
pub(crate) fn update(name: &str) -> Result<ComponentUpdateResult> {
let mut state = SavedState::load_from_disk("/")?.unwrap_or_default();
Expand All @@ -109,6 +114,8 @@ pub(crate) fn update(name: &str) -> Result<ComponentUpdateResult> {
_ => return Ok(ComponentUpdateResult::AtLatestVersion),
};

ensure_writable_boot()?;

let mut pending_container = state.pending.take().unwrap_or_default();
let interrupted = pending_container.get(component.name()).cloned();
pending_container.insert(component.name().into(), update.clone());
Expand Down Expand Up @@ -140,6 +147,9 @@ pub(crate) fn adopt_and_update(name: &str) -> Result<ContentMetadata> {
if state.installed.get(name).is_some() {
anyhow::bail!("Component {} is already installed", name);
};

ensure_writable_boot()?;

let update = if let Some(update) = component.query_update(&sysroot)? {
update
} else {
Expand Down
6 changes: 6 additions & 0 deletions src/efi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ impl Component for EFI {
let updatef = filetree::FileTree::new_from_dir(&updated).context("reading update dir")?;
// For adoption, we should only touch files that we know about.
let diff = updatef.relative_diff_to(&esp)?;
ensure_writable_efi()?;
log::trace!("applying adoption diff: {}", &diff);
filetree::apply_diff(&updated, &esp, &diff, None).context("applying filesystem changes")?;
Ok(InstalledContent {
Expand Down Expand Up @@ -153,6 +154,7 @@ impl Component for EFI {
let destdir = openat::Dir::open(&Path::new("/").join(MOUNT_PATH).join("EFI"))
.context("opening EFI dir")?;
validate_esp(&destdir)?;
ensure_writable_efi()?;
log::trace!("applying diff: {}", &diff);
filetree::apply_diff(&updated, &destdir, &diff, None)
.context("applying filesystem changes")?;
Expand Down Expand Up @@ -280,3 +282,7 @@ fn validate_esp(dir: &openat::Dir) -> Result<()> {
};
Ok(())
}

fn ensure_writable_efi() -> Result<()> {
util::ensure_writable_mount(&Path::new("/").join(MOUNT_PATH))
}
18 changes: 18 additions & 0 deletions src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::collections::HashSet;
use anyhow::{bail, Result};
use openat_ext::OpenatDirExt;

use std::path::Path;
use std::process::Command;

pub(crate) trait CommandRunExt {
Expand Down Expand Up @@ -65,3 +66,20 @@ pub(crate) fn filenames(dir: &openat::Dir) -> Result<HashSet<String>> {
}
Ok(ret)
}

pub(crate) fn ensure_writable_mount<P: AsRef<Path>>(p: P) -> Result<()> {
use nix::sys::statvfs;
let p = p.as_ref();
let stat = statvfs::statvfs(p)?;
if !stat.flags().contains(statvfs::FsFlags::ST_RDONLY) {
return Ok(());
}
let status = std::process::Command::new("mount")
.args(&["-o", "remount,rw"])
.arg(p)
.status()?;
if !status.success() {
anyhow::bail!("Failed to remount {:?} writable", p);
}
Ok(())
}
7 changes: 6 additions & 1 deletion tests/kola/test-bootupd
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ function cleanup () {
fi
}

bootefidir=/boot/efi/EFI
bootmount=/boot
bootefimount=${bootmount}/efi
bootefidir=${bootefimount}/EFI
bootupdir=/usr/lib/bootupd/updates
efiupdir=${bootupdir}/EFI
ostbaseefi=/usr/lib/ostree-boot/efi/EFI
Expand Down Expand Up @@ -115,6 +117,9 @@ bootupctl update | tee out.txt
assert_file_has_content_literal out.txt 'No update available for any component'
assert_not_file_has_content_literal out.txt 'Updated EFI'

# Ensure /boot and /boot/efi can be written to
mount -o remount,rw ${bootmount}
mount -o remount,rw ${bootefimount}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is OK for now, but in the future we can clean this up by having the test do something like re-exec itself in a new mount namespace so we're not affecting the main one.

echo "some additions" >> ${bootefidir}/${efisubdir}/shimx64.efi
if bootupctl validate 2>err.txt; then
fatal "unexpectedly passed validation"
Expand Down