Skip to content

Commit

Permalink
Use ManuallyDrop instead of stop function for cgroup lifecycle.
Browse files Browse the repository at this point in the history
  • Loading branch information
nbdd0121 committed Apr 18, 2024
1 parent 09cbf0f commit 6552032
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 24 deletions.
23 changes: 6 additions & 17 deletions src/cgroup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ pub trait DeviceAccessController {
minor: u32,
access: Access,
) -> Result<()>;

/// Stop performing access control. This may allow all accesses, so should only be used when
/// the cgroup is shutdown.
fn stop(self: Box<Self>) -> Result<()>;
}

pub struct DeviceAccessControllerV1 {
Expand Down Expand Up @@ -105,10 +101,6 @@ impl DeviceAccessController for DeviceAccessControllerV1 {

Ok(())
}

fn stop(self: Box<Self>) -> Result<()> {
Ok(())
}
}

#[repr(C)] // This is read as POD by the BPF program.
Expand Down Expand Up @@ -179,6 +171,12 @@ impl DeviceAccessControllerV2 {
}
}

impl Drop for DeviceAccessControllerV2 {
fn drop(&mut self) {
let _ = std::fs::remove_file(&self.pin);
}
}

impl DeviceAccessController for DeviceAccessControllerV2 {
fn set_permission(
&mut self,
Expand All @@ -199,11 +197,6 @@ impl DeviceAccessController for DeviceAccessControllerV2 {
}
Ok(())
}

fn stop(self: Box<Self>) -> Result<()> {
CgroupDevice::from_pin(&self.pin)?.unpin()?;
Ok(())
}
}

pub struct DeviceAccessControllerDummy;
Expand All @@ -218,8 +211,4 @@ impl DeviceAccessController for DeviceAccessControllerDummy {
) -> Result<()> {
bail!("neither cgroup v1 and cgroup v2 works");
}

fn stop(self: Box<Self>) -> Result<()> {
Ok(())
}
}
16 changes: 9 additions & 7 deletions src/docker/container.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::mem::ManuallyDrop;
use std::pin::pin;
use std::sync::{Arc, Mutex};
use std::time::Duration;
Expand All @@ -22,7 +23,7 @@ pub struct Container {
id: String,
user: String,
remove_event: Shared<BoxFuture<'static, Option<EventMessage>>>,
cgroup_device_filter: Arc<Mutex<Option<Box<dyn DeviceAccessController + Send>>>>,
cgroup_device_filter: Arc<Mutex<Option<ManuallyDrop<Box<dyn DeviceAccessController + Send>>>>>,
}

impl Container {
Expand Down Expand Up @@ -61,6 +62,10 @@ impl Container {
},
};

// Dropping the device filter will cause the container to have arbitrary device access.
// So keep it alive until we're sure that the container is stopped.
let cgroup_device_filter = ManuallyDrop::new(cgroup_device_filter);

Ok(Self {
docker: docker.clone(),
id,
Expand Down Expand Up @@ -109,12 +114,9 @@ impl Container {
}

// Stop the cgroup device filter. Only do so once we're sure that the container is removed.
self.cgroup_device_filter
.lock()
.unwrap()
.take()
.unwrap()
.stop()?;
drop(ManuallyDrop::into_inner(
self.cgroup_device_filter.lock().unwrap().take().unwrap(),
));

Ok(())
}
Expand Down

0 comments on commit 6552032

Please sign in to comment.