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

enable oom_score_adj test #251

Merged
merged 1 commit into from
Aug 31, 2021
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: 1 addition & 1 deletion integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ test_cases=(
"process/process.t"
"process_capabilities/process_capabilities.t"
"process_capabilities_fail/process_capabilities_fail.t"
# "process_oom_score_adj/process_oom_score_adj.t"
"process_oom_score_adj/process_oom_score_adj.t"
"process_rlimits/process_rlimits.t"
"process_rlimits_fail/process_rlimits_fail.t"
# "process_user/process_user.t"
Expand Down
32 changes: 29 additions & 3 deletions src/container/builder_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
use anyhow::{Context, Result};
use cgroups;
use oci_spec::Spec;
use std::{fs, os::unix::prelude::RawFd, path::PathBuf};
use std::{fs, io::Write, os::unix::prelude::RawFd, path::PathBuf};

use super::{Container, ContainerStatus};

Expand Down Expand Up @@ -49,11 +49,10 @@ impl<'a> ContainerBuilderImpl<'a> {
}

fn run_container(&mut self) -> Result<()> {
prctl::set_dumpable(false).unwrap();

let linux = self.spec.linux.as_ref().context("no linux in spec")?;
let cgroups_path = utils::get_cgroup_path(&linux.cgroups_path, &self.container_id);
let cmanager = cgroups::common::create_cgroup_manager(&cgroups_path, self.use_systemd)?;
let process = self.spec.process.as_ref().context("No process in spec")?;

if self.init {
if let Some(hooks) = self.spec.hooks.as_ref() {
Expand All @@ -71,6 +70,33 @@ impl<'a> ContainerBuilderImpl<'a> {
// namespace.
let notify_socket: NotifyListener = NotifyListener::new(&self.notify_path)?;

// If Out-of-memory score adjustment is set in specification. set the score
// value for the current process check
// https://dev.to/rrampage/surviving-the-linux-oom-killer-2ki9 for some more
// information.
//
// This has to be done before !dumpable because /proc/self/oom_score_adj
// is not writeable unless you're an privileged user (if !dumpable is
// set). All children inherit their parent's oom_score_adj value on
// fork(2) so this will always be propagated properly.
if let Some(oom_score_adj) = process.oom_score_adj {
log::debug!("Set OOM score to {}", oom_score_adj);
let mut f = fs::File::create("/proc/self/oom_score_adj")?;
f.write_all(oom_score_adj.to_string().as_bytes())?;
}

// Make the process non-dumpable, to avoid various race conditions that
// could cause processes in namespaces we're joining to access host
// resources (or potentially execute code).
//
// However, if the number of namespaces we are joining is 0, we are not
// going to be switching to a different security context. Thus setting
// ourselves to be non-dumpable only breaks things (like rootless
// containers), which is the recommendation from the kernel folks.
if linux.namespaces.is_some() {
prctl::set_dumpable(false).unwrap();
}

// This init_args will be passed to the container init process,
// therefore we will have to move all the variable by value. Since self
// is a shared reference, we have to clone these variables here.
Expand Down
13 changes: 1 addition & 12 deletions src/process/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::{
env,
os::unix::{io::AsRawFd, prelude::RawFd},
};
use std::{fs, io::Write, path::Path, path::PathBuf};
use std::{fs, path::Path, path::PathBuf};

use crate::{
capabilities,
Expand Down Expand Up @@ -175,17 +175,6 @@ pub fn container_intermidiate(
let linux = spec.linux.as_ref().context("no linux in spec")?;
let namespaces = Namespaces::from(linux.namespaces.as_ref());

// if Out-of-memory score adjustment is set in specification. set the score
// value for the current process check
// https://dev.to/rrampage/surviving-the-linux-oom-killer-2ki9 for some more
// information
if let Some(ref resource) = linux.resources {
if let Some(oom_score_adj) = resource.oom_score_adj {
let mut f = fs::File::create("/proc/self/oom_score_adj")?;
f.write_all(oom_score_adj.to_string().as_bytes())?;
}
}

// if new user is specified in specification, this will be true and new
// namespace will be created, check
// https://man7.org/linux/man-pages/man7/user_namespaces.7.html for more
Expand Down