Skip to content
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
2 changes: 2 additions & 0 deletions docs/treefile.md
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,8 @@ version of `rpm-ostree`.
- `var`: `/opt` and `/usr/local` are symlinks to subdirectories in `/var`
and are purely machine-local state.
- `root`: These are plain directories; only use this with composefs enabled!
* `no-initramfs`: boolean, optional: If `true` don't run dracut to generate an
initramfs. Defaults to `false`.

### Kickstarts

Expand Down
23 changes: 23 additions & 0 deletions rpmostree-cxxrs.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -1876,6 +1876,7 @@ struct Treefile final : public ::rust::Opaque
bool may_require_local_assembly () const noexcept;
bool has_any_packages () const noexcept;
bool merge_treefile (::rust::Str treefile);
bool get_no_initramfs () const noexcept;
~Treefile () = delete;

private:
Expand Down Expand Up @@ -2835,6 +2836,9 @@ extern "C"
::rust::repr::PtrLen
rpmostreecxx$cxxbridge1$Treefile$merge_treefile (::rpmostreecxx::Treefile &self,
::rust::Str treefile, bool *return$) noexcept;

bool
rpmostreecxx$cxxbridge1$Treefile$get_no_initramfs (::rpmostreecxx::Treefile const &self) noexcept;
::std::size_t rpmostreecxx$cxxbridge1$RepoPackage$operator$sizeof () noexcept;
::std::size_t rpmostreecxx$cxxbridge1$RepoPackage$operator$alignof () noexcept;

Expand Down Expand Up @@ -2885,6 +2889,9 @@ extern "C"
rpmostreecxx$cxxbridge1$transaction_apply_live (::rpmostreecxx::OstreeSysroot const &sysroot,
::rpmostreecxx::GVariant const &target) noexcept;

::rust::repr::PtrLen
rpmostreecxx$cxxbridge1$normalize_etc_shadow (::std::int32_t rootfs_dfd) noexcept;

::rust::repr::PtrLen rpmostreecxx$cxxbridge1$prepare_rpm_layering (
::std::int32_t rootfs, ::std::int32_t merge_passwd_dir, bool *return$) noexcept;

Expand Down Expand Up @@ -5625,6 +5632,12 @@ Treefile::merge_treefile (::rust::Str treefile)
return ::std::move (return$.value);
}

bool
Treefile::get_no_initramfs () const noexcept
{
return rpmostreecxx$cxxbridge1$Treefile$get_no_initramfs (*this);
}

::std::size_t
RepoPackage::layout::size () noexcept
{
Expand Down Expand Up @@ -5770,6 +5783,16 @@ transaction_apply_live (::rpmostreecxx::OstreeSysroot const &sysroot,
}
}

void
normalize_etc_shadow (::std::int32_t rootfs_dfd)
{
::rust::repr::PtrLen error$ = rpmostreecxx$cxxbridge1$normalize_etc_shadow (rootfs_dfd);
if (error$.ptr)
{
throw ::rust::impl<::rust::Error>::error (error$);
}
}

bool
prepare_rpm_layering (::std::int32_t rootfs, ::std::int32_t merge_passwd_dir)
{
Expand Down
3 changes: 3 additions & 0 deletions rpmostree-cxxrs.h
Original file line number Diff line number Diff line change
Expand Up @@ -1653,6 +1653,7 @@ struct Treefile final : public ::rust::Opaque
bool may_require_local_assembly () const noexcept;
bool has_any_packages () const noexcept;
bool merge_treefile (::rust::Str treefile);
bool get_no_initramfs () const noexcept;
~Treefile () = delete;

private:
Expand Down Expand Up @@ -2083,6 +2084,8 @@ void applylive_sync_ref (::rpmostreecxx::OstreeSysroot const &sysroot);
void transaction_apply_live (::rpmostreecxx::OstreeSysroot const &sysroot,
::rpmostreecxx::GVariant const &target);

void normalize_etc_shadow (::std::int32_t rootfs_dfd);

bool prepare_rpm_layering (::std::int32_t rootfs, ::std::int32_t merge_passwd_dir);

void complete_rpm_layering (::std::int32_t rootfs);
Expand Down
7 changes: 7 additions & 0 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,7 @@ pub mod ffi {
fn may_require_local_assembly(&self) -> bool;
fn has_any_packages(&self) -> bool;
fn merge_treefile(&mut self, treefile: &str) -> Result<bool>;
fn get_no_initramfs(&self) -> bool;
}

// treefile.rs (split out from above to make &self nice to use)
Expand Down Expand Up @@ -738,6 +739,11 @@ pub mod ffi {
fn transaction_apply_live(sysroot: &OstreeSysroot, target: &GVariant) -> Result<()>;
}

// normalization.rs
extern "Rust" {
fn normalize_etc_shadow(rootfs_dfd: i32) -> Result<()>;
}

// passwd.rs
extern "Rust" {
fn prepare_rpm_layering(rootfs: i32, merge_passwd_dir: i32) -> Result<bool>;
Expand Down Expand Up @@ -1018,6 +1024,7 @@ mod live;
pub(crate) use self::live::*;
mod nameservice;
mod normalization;
use normalization::*;
mod origin;
mod ostree_prepareroot;
pub(crate) use self::origin::*;
Expand Down
5 changes: 4 additions & 1 deletion rust/src/normalization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

use crate::bwrap::Bubblewrap;
use crate::cxxrsutil::*;
use crate::ffiutil;
use crate::nameservice::shadow::parse_shadow_content;
use anyhow::{anyhow, Context, Result};
use cap_std::fs::OpenOptionsExt;
Expand All @@ -30,7 +32,8 @@ pub(crate) fn source_date_epoch_raw() -> Option<String> {
}

#[context("Rewriting /etc/shadow to remove lstchg field")]
pub(crate) fn normalize_etc_shadow(rootfs: &Dir) -> Result<()> {
pub(crate) fn normalize_etc_shadow(rootfs_dfd: i32) -> CxxResult<()> {
let rootfs = unsafe { ffiutil::ffi_dirfd(rootfs_dfd)? };
// Read in existing entries.
let mut openopts = OpenOptions::new();
openopts.create(true).read(true).write(true).mode(0o400);
Expand Down
2 changes: 1 addition & 1 deletion rust/src/passwd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub fn complete_rpm_layering(rootfs_dfd: i32) -> CxxResult<()> {
// /etc/shadow ends up with a timestamp in it thanks to the `lstchg`
// field. This can be made empty safely, especially for accounts that
// are locked.
normalization::normalize_etc_shadow(&rootfs)?;
normalization::normalize_etc_shadow(rootfs_dfd)?;

Ok(())
}
Expand Down
9 changes: 8 additions & 1 deletion rust/src/treefile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,8 @@ fn treefile_merge(dest: &mut TreeComposeConfig, src: &mut TreeComposeConfig) {
check_passwd,
check_groups,
postprocess_script,
rpmdb_normalize
rpmdb_normalize,
no_initramfs
);
merge_hashsets!(ignore_removed_groups, ignore_removed_users);
merge_maps!(add_commit_metadata, variables, metadata, repovars);
Expand Down Expand Up @@ -1856,6 +1857,10 @@ impl Treefile {
derived
}

pub(crate) fn get_no_initramfs(&self) -> bool {
self.parsed.base.no_initramfs.unwrap_or_default()
}

pub(crate) fn set_initramfs_regenerate(&mut self, enabled: bool, args: Vec<String>) {
if !enabled {
// implicitly leaves empty if already empty
Expand Down Expand Up @@ -2609,6 +2614,8 @@ pub(crate) struct BaseComposeConfigFields {
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) initramfs_args: Option<Vec<String>>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) no_initramfs: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) readonly_executables: Option<bool>,

// Tree layout options
Expand Down
13 changes: 10 additions & 3 deletions src/libpriv/rpmostree-kernel.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -398,10 +398,17 @@ rpmostree_finalize_kernel (int rootfs_dfd, const char *bootdir, const char *kver
}
else
{
/* we're not replacing the initramfs; use built-in one */
if (!_rpmostree_util_update_checksum_from_file (boot_checksum, rootfs_dfd,
initramfs_modules_path, cancellable, error))
/* We're not replacing the initramfs; use built-in one if it exists */
if (!glnx_fstatat_allow_noent (rootfs_dfd, initramfs_modules_path, NULL, AT_SYMLINK_NOFOLLOW,
error))
return FALSE;
if (errno == 0)
Comment on lines +402 to +405

Choose a reason for hiding this comment

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

high

The check if (errno == 0) is not guaranteed to be safe. The glnx_fstatat_allow_noent function does not reset errno to 0 on success. If a previous, unrelated syscall failed, errno could hold a non-zero value, leading this check to incorrectly assume the file does not exist when it does. To prevent this potential bug, you should explicitly set errno = 0; before calling glnx_fstatat_allow_noent.

      errno = 0;
      if (!glnx_fstatat_allow_noent (rootfs_dfd, initramfs_modules_path, NULL, AT_SYMLINK_NOFOLLOW,
                                     error))
        return FALSE;
      if (errno == 0)

Copy link
Member Author

Choose a reason for hiding this comment

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

There are other uses in this file that don't seem to reset errno. Maybe we should be doing that everywhere?

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 a good observation by Gemini, but since it only sees the code in this repo it doesn't know that actually that API does always set errno = 0: https://gitlab.gnome.org/GNOME/libglnx/-/blob/de29c5f7d9df8d57b4f5caa9920f5d4caa7a8cfc/glnx-fdio.h#L347

{
/* It exists, update the checksum */
if (!_rpmostree_util_update_checksum_from_file (
boot_checksum, rootfs_dfd, initramfs_modules_path, cancellable, error))
return FALSE;
}
}

const char *boot_checksum_str = g_checksum_get_string (boot_checksum);
Expand Down
84 changes: 51 additions & 33 deletions src/libpriv/rpmostree-postprocess.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,48 @@ rename_if_exists (int src_dfd, const char *from, int dest_dfd, const char *to, G
return TRUE;
}

static gboolean
run_dracut (int rootfs_dfd, rpmostreecxx::Treefile &treefile, GLnxTmpfile *initramfs_tmpf,
const char *kver, GCancellable *cancellable, GError **error)
{
/* Run dracut with our chosen arguments (commonly at least --no-hostonly) */
g_autoptr (GPtrArray) dracut_argv = g_ptr_array_new ();
rust::Vec<rust::String> initramfs_args = treefile.get_initramfs_args ();
if (!initramfs_args.empty ())
{
for (auto &arg : initramfs_args)
g_ptr_array_add (dracut_argv, (void *)arg.c_str ());
}
else
{
/* Default to this for treecomposes */
g_ptr_array_add (dracut_argv, (char *)"--no-hostonly");
}
g_ptr_array_add (dracut_argv, NULL);

/* We use a tmpdir under the target root since dracut currently tries to copy
* xattrs, including e.g. user.ostreemeta, which can't be copied to tmpfs.
*/
{
g_auto (GLnxTmpDir) dracut_host_tmpd = {
0,
};
if (!glnx_mkdtempat (rootfs_dfd, "rpmostree-dracut.XXXXXX", 0700, &dracut_host_tmpd, error))
return FALSE;
if (!rpmostree_run_dracut (rootfs_dfd, (const char *const *)dracut_argv->pdata, kver, NULL,
FALSE, &dracut_host_tmpd, initramfs_tmpf, cancellable, error))
return FALSE;
/* No reason to have the initramfs not be world-readable since
* it's server-side generated and shouldn't contain any secrets.
* https://github.com/coreos/coreos-assembler/pull/372#issuecomment-467620937
*/
if (!glnx_fchmod (initramfs_tmpf->fd, 0644, error))
return FALSE;
}

return TRUE;
}

/* Handle the kernel/initramfs, which can be in at least 2 different places:
* - /boot (CentOS, Fedora treecompose before we suppressed kernel.spec's %posttrans)
* - /usr/lib/modules (Fedora treecompose without kernel.spec's %posttrans)
Expand Down Expand Up @@ -195,43 +237,19 @@ process_kernel_and_initramfs (int rootfs_dfd, rpmostreecxx::Treefile &treefile,
(void)unlinkat (rootfs_dfd, "usr/etc/machine-id", 0);
}

/* Run dracut with our chosen arguments (commonly at least --no-hostonly) */
g_autoptr (GPtrArray) dracut_argv = g_ptr_array_new ();
rust::Vec<rust::String> initramfs_args = treefile.get_initramfs_args ();
if (!initramfs_args.empty ())
{
for (auto &arg : initramfs_args)
g_ptr_array_add (dracut_argv, (void *)arg.c_str ());
}
else
{
/* Default to this for treecomposes */
g_ptr_array_add (dracut_argv, (char *)"--no-hostonly");
}
g_ptr_array_add (dracut_argv, NULL);

g_auto (GLnxTmpfile) initramfs_tmpf = {
0,
};
/* We use a tmpdir under the target root since dracut currently tries to copy
* xattrs, including e.g. user.ostreemeta, which can't be copied to tmpfs.
if (!treefile.get_no_initramfs ())
{
if (!run_dracut (rootfs_dfd, treefile, &initramfs_tmpf, kver, cancellable, error))
return FALSE;
}

/* Always normalize etc shadow. If we called run_dracut this may
* have already been done but it's OK since it's idempotent.
*/
{
g_auto (GLnxTmpDir) dracut_host_tmpd = {
0,
};
if (!glnx_mkdtempat (rootfs_dfd, "rpmostree-dracut.XXXXXX", 0700, &dracut_host_tmpd, error))
return FALSE;
if (!rpmostree_run_dracut (rootfs_dfd, (const char *const *)dracut_argv->pdata, kver, NULL,
FALSE, &dracut_host_tmpd, &initramfs_tmpf, cancellable, error))
return FALSE;
/* No reason to have the initramfs not be world-readable since
* it's server-side generated and shouldn't contain any secrets.
* https://github.com/coreos/coreos-assembler/pull/372#issuecomment-467620937
*/
if (!glnx_fchmod (initramfs_tmpf.fd, 0644, error))
return FALSE;
}
ROSCXX_TRY (normalize_etc_shadow (rootfs_dfd), error);

/* We always tell rpmostree_finalize_kernel() to skip /boot, since we'll do a
* full hardlink pass if needed after that for the kernel + bootloader data.
Expand Down
15 changes: 15 additions & 0 deletions tests/compose/test-no-initramfs.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/bin/bash
set -xeuo pipefail

dn=$(cd "$(dirname "$0")" && pwd)
# shellcheck source=libcomposetest.sh
. "${dn}/libcomposetest.sh"

treefile_set "no-initramfs" 'True'
runcompose
echo "ok compose"

commit=$(jq -r '.["ostree-commit"]' < compose.json)
ostree --repo=${repo:?} ls -R ${commit} usr/lib/modules > ls.txt
assert_not_file_has_content ls.txt '/usr/lib/modules/.*/initramfs.img$'
echo "ok no initramfs"