rustjail: Change mknod_dev() and bind_dev() to take relative device path

Both these functions take the absolute path from LinuxDevice and drop the
leading '/' to make a relative path.  They do that with a simple
&dev.path[1..].  That can be technically incorrect in some edge cases such
as a path with redundant /s like "//dev//sda".

To handle cases like that, have the explicit relative path passed into
these functions.  For now we calculate it in the same buggy way, but we'll
fix that shortly.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This commit is contained in:
David Gibson 2021-10-13 16:43:20 +11:00
parent 2680c0bfee
commit d6b62c029e

View File

@ -829,17 +829,19 @@ fn default_symlinks() -> Result<()> {
Ok(()) Ok(())
} }
fn create_devices(devices: &[LinuxDevice], bind: bool) -> Result<()> { fn create_devices(devices: &[LinuxDevice], bind: bool) -> Result<()> {
let op: fn(&LinuxDevice) -> Result<()> = if bind { bind_dev } else { mknod_dev }; let op: fn(&LinuxDevice, &Path) -> Result<()> = if bind { bind_dev } else { mknod_dev };
let old = stat::umask(Mode::from_bits_truncate(0o000)); let old = stat::umask(Mode::from_bits_truncate(0o000));
for dev in DEFAULT_DEVICES.iter() { for dev in DEFAULT_DEVICES.iter() {
op(dev).context(format!("Creating container device {:?}", dev))?; let path = Path::new(&dev.path[1..]);
op(dev, path).context(format!("Creating container device {:?}", dev))?;
} }
for dev in devices { for dev in devices {
if !dev.path.starts_with("/dev") || dev.path.contains("..") { if !dev.path.starts_with("/dev") || dev.path.contains("..") {
let msg = format!("{} is not a valid device path", dev.path); let msg = format!("{} is not a valid device path", dev.path);
bail!(anyhow!(msg)); bail!(anyhow!(msg));
} }
op(dev).context(format!("Creating container device {:?}", dev))?; let path = Path::new(&dev.path[1..]);
op(dev, path).context(format!("Creating container device {:?}", dev))?;
} }
stat::umask(old); stat::umask(old);
Ok(()) Ok(())
@ -861,21 +863,21 @@ lazy_static! {
}; };
} }
fn mknod_dev(dev: &LinuxDevice) -> Result<()> { fn mknod_dev(dev: &LinuxDevice, relpath: &Path) -> Result<()> {
let f = match LINUXDEVICETYPE.get(dev.r#type.as_str()) { let f = match LINUXDEVICETYPE.get(dev.r#type.as_str()) {
Some(v) => v, Some(v) => v,
None => return Err(anyhow!("invalid spec".to_string())), None => return Err(anyhow!("invalid spec".to_string())),
}; };
stat::mknod( stat::mknod(
&dev.path[1..], relpath,
*f, *f,
Mode::from_bits_truncate(dev.file_mode.unwrap_or(0)), Mode::from_bits_truncate(dev.file_mode.unwrap_or(0)),
nix::sys::stat::makedev(dev.major as u64, dev.minor as u64), nix::sys::stat::makedev(dev.major as u64, dev.minor as u64),
)?; )?;
unistd::chown( unistd::chown(
&dev.path[1..], relpath,
Some(Uid::from_raw(dev.uid.unwrap_or(0) as uid_t)), Some(Uid::from_raw(dev.uid.unwrap_or(0) as uid_t)),
Some(Gid::from_raw(dev.gid.unwrap_or(0) as uid_t)), Some(Gid::from_raw(dev.gid.unwrap_or(0) as uid_t)),
)?; )?;
@ -883,9 +885,9 @@ fn mknod_dev(dev: &LinuxDevice) -> Result<()> {
Ok(()) Ok(())
} }
fn bind_dev(dev: &LinuxDevice) -> Result<()> { fn bind_dev(dev: &LinuxDevice, relpath: &Path) -> Result<()> {
let fd = fcntl::open( let fd = fcntl::open(
&dev.path[1..], relpath,
OFlag::O_RDWR | OFlag::O_CREAT, OFlag::O_RDWR | OFlag::O_CREAT,
Mode::from_bits_truncate(0o644), Mode::from_bits_truncate(0o644),
)?; )?;
@ -894,7 +896,7 @@ fn bind_dev(dev: &LinuxDevice) -> Result<()> {
mount( mount(
Some(&*dev.path), Some(&*dev.path),
&dev.path[1..], relpath,
None::<&str>, None::<&str>,
MsFlags::MS_BIND, MsFlags::MS_BIND,
None::<&str>, None::<&str>,
@ -1258,11 +1260,12 @@ mod tests {
uid: Some(unistd::getuid().as_raw()), uid: Some(unistd::getuid().as_raw()),
gid: Some(unistd::getgid().as_raw()), gid: Some(unistd::getgid().as_raw()),
}; };
let path = Path::new("fifo");
let ret = mknod_dev(&dev); let ret = mknod_dev(&dev, path);
assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); assert!(ret.is_ok(), "Should pass. Got: {:?}", ret);
let ret = stat::stat("fifo"); let ret = stat::stat(path);
assert!(ret.is_ok(), "Should pass. Got: {:?}", ret); assert!(ret.is_ok(), "Should pass. Got: {:?}", ret);
} }
#[test] #[test]