Skip to content

Commit

Permalink
Fix inconsistent state after failed prepackaged header installation (#…
Browse files Browse the repository at this point in the history
…1663)

Summary: Pixie's installation process requires downloading a Linux
header bundle and applying patches. Issue #1056 identified that a failed
installation process could lead to an inconsistent state, resulting in
the error:

```bash
Failed to install packaged Linux headers to /lib/modules/5.15.0-58-generic, error: Internal: Refusing to clobber existing directory: /usr/src/linux-headers-5.15.90-pl.
```

The problem was that the installation process did not clean up after a
failed attempt.

This PR modifies the extraction process for prepackaged Linux headers by
introducing a staging directory. Extraction is now performed in this
staging directory first, which helps ensure that the check for existing
directories does not raise an error from a previous failed run. After
successfully extracting and applying patches, the contents are moved
from the staging directory to the expected directory.

Additional context: The function for extraction `Minitar::Extract` was
modified to accept an additional parameter, `prefix_to_strip`. This
helps remove any specified prefix from the pathnames of the untarred
files to prevent unnecessary nesting in the staging directory.

Relevant Issues: Fixes #1092

Type of change: /kind bug

Test Plan: Removed the kernel config files on a node, then installed
Pixie and verified that the initial error which previously led to
inconsistent state is raised:
```
Failed to install packaged Linux headers to /lib/modules/5.15.0-58-generic, error: Not Found : No kernel config found. Searched: /proc/config,/proc/config.gz,/host/boot/config-5.15.0-58-generic,/host/lib/modules/5.15.0-58-generic/config,/proc/1/root/media/sr0/boot/config-virt,/proc/1/root/media/vda/boot/config-virt.
```
After this, I added the config files back and redeployed, verifying that
things work as intended i.e. that the config files are found and that
there are no leftover files from a failed installation in the expected
directory.

---------

Signed-off-by: Benjamin Kilimnik <[email protected]>
  • Loading branch information
benkilimnik authored Aug 12, 2023
1 parent 2b198fb commit 1a95c5f
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 28 deletions.
16 changes: 14 additions & 2 deletions src/common/minitar/minitar.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ Status CopyData(struct archive* ar, struct archive* aw) {
}
} // namespace

Status Minitar::Extract(std::string_view dest_dir, int flags) {
Status Minitar::Extract(std::string_view dest_dir, std::string_view prefix_to_strip, int flags) {
int r;

// Declare the deferred closes up-front,
Expand All @@ -97,11 +97,23 @@ Status Minitar::Extract(std::string_view dest_dir, int flags) {
}
PX_RETURN_IF_NOT_ARCHIVE_OK(r, a);

// Get the original entry pathname.
std::string_view pathname(archive_entry_pathname(entry));

// Check if the pathname starts with the prefix to strip, and if so, remove it.
if (!prefix_to_strip.empty() && absl::StartsWith(pathname, prefix_to_strip)) {
pathname.remove_prefix(prefix_to_strip.length());
}

// if a destination directory is provided...
if (!dest_dir.empty()) {
std::string dest_path = absl::StrCat(dest_dir, "/", archive_entry_pathname(entry));
// Concatenate the destination directory with the entry's pathname.
std::string dest_path = absl::StrCat(dest_dir, "/", pathname);
// Set the entry's pathname to the newly constructed path.
archive_entry_set_pathname(entry, dest_path.c_str());
}

// Write the header for the current entry.
r = archive_write_header(ext, entry);
PX_RETURN_IF_NOT_ARCHIVE_OK(r, a);
PX_RETURN_IF_ERROR(CopyData(a, ext));
Expand Down
3 changes: 2 additions & 1 deletion src/common/minitar/minitar.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ class Minitar {
* See libarchive for flag defintitions and other possible flags.
* @return error if the tarball could not be extracted.
*/
Status Extract(std::string_view dest_dir = {}, int flags = kDefaultFlags);
Status Extract(std::string_view dest_dir = {}, std::string_view prefix_to_strip = {},
int flags = kDefaultFlags);

private:
static constexpr int kDefaultFlags = ARCHIVE_EXTRACT_TIME;
Expand Down
62 changes: 37 additions & 25 deletions src/stirling/utils/linux_headers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -498,31 +498,23 @@ Status LinkHostLinuxHeaders(const std::filesystem::path& lib_modules_dir) {
return Status::OK();
}

Status ExtractPackagedHeaders(PackagedLinuxHeadersSpec* headers_package) {
std::string expected_directory =
absl::Substitute("/usr/src/linux-headers-$0.$1.$2-pl", headers_package->version.version,
headers_package->version.major_rev, headers_package->version.minor_rev);

// This is a loose check that we don't clobber what we *think* should the output directory.
// If someone built a tar.gz with an incorrect directory structure, this check wouldn't save us.
if (fs::Exists(expected_directory)) {
return error::Internal("Refusing to clobber existing directory: $0.", expected_directory);
}

// Extract the files.
::px::tools::Minitar minitar(headers_package->path.string());
PX_RETURN_IF_ERROR(minitar.Extract("/"));

// Check that the expected path was created.
if (!fs::Exists(expected_directory)) {
Status ExtractPackagedHeaders(const PackagedLinuxHeadersSpec& headers_package,
const std::string& staging_directory,
const std::string& expected_directory) {
std::filesystem::create_directories(staging_directory);
// Instantiate a minitar object with the path to the tarball.
::px::tools::Minitar minitar(headers_package.path.string());
// Extract the files from the tarball, stripping the leading prefix
// "usr/src/linux-headers-$0.$1.$2-pl" to avoid unnecessary nesting in the staging directory.
std::string_view expected_directory_view = expected_directory;
std::string_view prefix_to_strip = expected_directory_view.substr(1);
PX_RETURN_IF_ERROR(minitar.Extract(staging_directory, prefix_to_strip));
// Check that the staging path was created.
if (!fs::Exists(staging_directory)) {
return error::Internal(
"Package extraction did not result in the expected headers directory: $0.",
expected_directory);
}

// Update the path to the extracted copy.
headers_package->path = expected_directory;

return Status::OK();
}

Expand Down Expand Up @@ -627,10 +619,30 @@ Status InstallPackagedLinuxHeaders(const std::filesystem::path& lib_modules_dir)
PX_ASSIGN_OR_RETURN(PackagedLinuxHeadersSpec packaged_headers,
FindClosestPackagedLinuxHeaders(kPackagedHeadersRoot, kernel_version));
LOG(INFO) << absl::Substitute("Using packaged header: $0", packaged_headers.path.string());
PX_RETURN_IF_ERROR(ExtractPackagedHeaders(&packaged_headers));
PX_RETURN_IF_ERROR(ModifyKernelVersion(packaged_headers.path, kernel_version.code()));
PX_RETURN_IF_ERROR(ApplyConfigPatches(packaged_headers.path));
PX_RETURN_IF_ERROR(fs::CreateSymlinkIfNotExists(packaged_headers.path, lib_modules_build_dir));
const std::string version =
absl::Substitute("$0.$1.$2-pl", packaged_headers.version.version,
packaged_headers.version.major_rev, packaged_headers.version.minor_rev);
const std::string staging_directory = absl::StrCat("/usr/src/staging/linux-headers-", version);
const std::string expected_directory = absl::StrCat("/usr/src/linux-headers-", version);
// Verify that the target directory doesn't already exist.
// If someone built a tar.gz with an incorrect directory structure, this check wouldn't save us.
if (fs::Exists(expected_directory)) {
return error::Internal(
"Not attempting to install packaged headers because the target directory already exists: "
"$0",
expected_directory);
}
// Extract the packaged headers to a staging directory, stripping the expected target directory
// prefix.
PX_RETURN_IF_ERROR(
ExtractPackagedHeaders(packaged_headers, staging_directory, expected_directory));
// Modify version.h to the specific kernel version in the staged headers.
PX_RETURN_IF_ERROR(ModifyKernelVersion(staging_directory, kernel_version.code()));
// Find valid kernel config and patch the staged headers to match.
PX_RETURN_IF_ERROR(ApplyConfigPatches(staging_directory));
// Move the staged headers to the expected, target directory.
std::filesystem::rename(staging_directory, expected_directory);
PX_RETURN_IF_ERROR(fs::CreateSymlinkIfNotExists(expected_directory, lib_modules_build_dir));
LOG(INFO) << absl::Substitute("Successfully installed packaged copy of headers at $0",
lib_modules_build_dir.string());
g_packaged_headers_installed = true;
Expand Down

0 comments on commit 1a95c5f

Please sign in to comment.