Skip to content

Commit

Permalink
maps,programs: avoid path UTF-8 assumptions
Browse files Browse the repository at this point in the history
  • Loading branch information
tamird committed Aug 24, 2023
1 parent 572d047 commit 0bba9b1
Show file tree
Hide file tree
Showing 13 changed files with 325 additions and 204 deletions.
12 changes: 0 additions & 12 deletions aya/src/bpf.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::{
borrow::Cow,
collections::{HashMap, HashSet},
ffi::CString,
fs, io,
os::{
fd::{AsFd as _, OwnedFd},
Expand Down Expand Up @@ -608,10 +607,6 @@ impl<'a> BpfLoader<'a> {
ProgramSection::SchedClassifier => {
Program::SchedClassifier(SchedClassifier {
data: ProgramData::new(prog_name, obj, btf_fd, *verifier_log_level),
name: unsafe {
CString::from_vec_unchecked(Vec::from(name.clone()))
.into_boxed_c_str()
},
})
}
ProgramSection::CgroupSkb => Program::CgroupSkb(CgroupSkb {
Expand Down Expand Up @@ -960,13 +955,6 @@ pub enum BpfError {
name: u32,
},

/// Invalid path
#[error("invalid path `{error}`")]
InvalidPath {
/// The error message
error: String,
},

/// Error parsing BPF object
#[error("error parsing BPF object: {0}")]
ParseError(#[from] ParseError),
Expand Down
41 changes: 26 additions & 15 deletions aya/src/maps/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,9 +515,19 @@ impl MapData {
name: &str,
btf_fd: Option<BorrowedFd<'_>>,
) -> Result<Self, MapError> {
use std::os::unix::ffi::OsStrExt as _;

// try to open map in case it's already pinned
let map_path = path.as_ref().join(name);
let path_string = CString::new(map_path.to_str().unwrap()).unwrap();
let path = path.as_ref().join(name);
let path_string = match CString::new(path.as_os_str().as_bytes()) {
Ok(path) => path,
Err(error) => {
return Err(MapError::PinError {
name: Some(name.into()),
error: PinError::InvalidPinPath { path, error },
});
}
};
match bpf_get_object(&path_string).map_err(|(_, io_error)| SyscallError {
call: "BPF_OBJ_GET",
io_error,
Expand All @@ -540,14 +550,16 @@ impl MapData {

/// Loads a map from a pinned path in bpffs.
pub fn from_pin<P: AsRef<Path>>(path: P) -> Result<Self, MapError> {
use std::os::unix::ffi::OsStrExt as _;

let path = path.as_ref();
let path_string =
CString::new(path.as_ref().to_string_lossy().into_owned()).map_err(|e| {
MapError::PinError {
name: None,
error: PinError::InvalidPinPath {
error: e.to_string(),
},
}
CString::new(path.as_os_str().as_bytes()).map_err(|error| MapError::PinError {
name: None,
error: PinError::InvalidPinPath {
path: path.into(),
error,
},
})?;

let fd = bpf_get_object(&path_string).map_err(|(_, io_error)| SyscallError {
Expand Down Expand Up @@ -580,16 +592,15 @@ impl MapData {
}

pub(crate) fn pin<P: AsRef<Path>>(&mut self, name: &str, path: P) -> Result<(), PinError> {
use std::os::unix::ffi::OsStrExt as _;

let Self { fd, pinned, obj: _ } = self;
if *pinned {
return Err(PinError::AlreadyPinned { name: name.into() });
}
let map_path = path.as_ref().join(name);
let path_string = CString::new(map_path.to_string_lossy().into_owned()).map_err(|e| {
PinError::InvalidPinPath {
error: e.to_string(),
}
})?;
let path = path.as_ref().join(name);
let path_string = CString::new(path.as_os_str().as_bytes())
.map_err(|error| PinError::InvalidPinPath { path, error })?;
bpf_pin_object(*fd, &path_string).map_err(|(_, io_error)| SyscallError {
call: "BPF_OBJ_PIN",
io_error,
Expand Down
10 changes: 7 additions & 3 deletions aya/src/pin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@ pub enum PinError {
name: String,
},
/// The path for the BPF object is not valid.
#[error("invalid pin path `{error}`")]
#[error("invalid pin path `{}`", path.display())]
InvalidPinPath {
/// The error message.
error: String,
/// The path.
path: std::path::PathBuf,

#[source]
/// The source error.
error: std::ffi::NulError,
},
/// An error ocurred making a syscall.
#[error(transparent)]
Expand Down
10 changes: 7 additions & 3 deletions aya/src/programs/kprobe.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
//! Kernel space probes.
use std::{io, os::fd::AsFd as _, path::Path};
use std::{
io,
os::fd::AsFd as _,
path::{Path, PathBuf},
};
use thiserror::Error;

use crate::{
Expand Down Expand Up @@ -68,7 +72,7 @@ impl KProbe {
///
/// The returned value can be used to detach from the given function, see [KProbe::detach].
pub fn attach(&mut self, fn_name: &str, offset: u64) -> Result<KProbeLinkId, ProgramError> {
attach(&mut self.data, self.kind, fn_name, offset, None)
attach(&mut self.data, self.kind, Path::new(fn_name), offset, None)
}

/// Detaches the program.
Expand Down Expand Up @@ -114,7 +118,7 @@ pub enum KProbeError {
#[error("`{filename}`")]
FileError {
/// The file name
filename: String,
filename: PathBuf,
/// The [`io::Error`] returned from the file operation
#[source]
io_error: io::Error,
Expand Down
22 changes: 14 additions & 8 deletions aya/src/programs/links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,19 +146,22 @@ impl FdLink {
/// # Ok::<(), Error>(())
/// ```
pub fn pin<P: AsRef<Path>>(self, path: P) -> Result<PinnedLink, PinError> {
let path_string =
CString::new(path.as_ref().to_string_lossy().into_owned()).map_err(|e| {
PinError::InvalidPinPath {
error: e.to_string(),
}
})?;
use std::os::unix::ffi::OsStrExt as _;

let path = path.as_ref();
let path_string = CString::new(path.as_os_str().as_bytes()).map_err(|error| {
PinError::InvalidPinPath {
path: path.into(),
error,
}
})?;
bpf_pin_object(self.fd.as_raw_fd(), &path_string).map_err(|(_, io_error)| {
SyscallError {
call: "BPF_OBJ_PIN",
io_error,
}
})?;
Ok(PinnedLink::new(PathBuf::from(path.as_ref()), self))
Ok(PinnedLink::new(path.into(), self))
}
}

Expand Down Expand Up @@ -203,7 +206,10 @@ impl PinnedLink {

/// Creates a [`crate::programs::links::PinnedLink`] from a valid path on bpffs.
pub fn from_pin<P: AsRef<Path>>(path: P) -> Result<Self, LinkError> {
let path_string = CString::new(path.as_ref().to_string_lossy().to_string()).unwrap();
use std::os::unix::ffi::OsStrExt as _;

// TODO: avoid this unwrap by adding a new error variant.
let path_string = CString::new(path.as_ref().as_os_str().as_bytes()).unwrap();
let fd = bpf_get_object(&path_string).map_err(|(_, io_error)| {
LinkError::SyscallError(SyscallError {
call: "BPF_OBJ_GET",
Expand Down
23 changes: 15 additions & 8 deletions aya/src/programs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,9 +487,10 @@ impl<T: Link> ProgramData<T> {
path: P,
verifier_log_level: VerifierLogLevel,
) -> Result<Self, ProgramError> {
let path_string =
CString::new(path.as_ref().as_os_str().to_string_lossy().as_bytes()).unwrap();
use std::os::unix::ffi::OsStrExt as _;

// TODO: avoid this unwrap by adding a new error variant.
let path_string = CString::new(path.as_ref().as_os_str().as_bytes()).unwrap();
let fd = bpf_get_object(&path_string).map_err(|(_, io_error)| SyscallError {
call: "bpf_obj_get",
io_error,
Expand Down Expand Up @@ -520,18 +521,21 @@ fn unload_program<T: Link>(data: &mut ProgramData<T>) -> Result<(), ProgramError
}

fn pin_program<T: Link, P: AsRef<Path>>(data: &ProgramData<T>, path: P) -> Result<(), PinError> {
use std::os::unix::ffi::OsStrExt as _;

let fd = data.fd.as_ref().ok_or(PinError::NoFd {
name: data
.name
.as_deref()
.unwrap_or("<unknown program>")
.to_string(),
})?;
let path_string = CString::new(path.as_ref().to_string_lossy().into_owned()).map_err(|e| {
PinError::InvalidPinPath {
error: e.to_string(),
}
})?;
let path = path.as_ref();
let path_string =
CString::new(path.as_os_str().as_bytes()).map_err(|error| PinError::InvalidPinPath {
path: path.into(),
error,
})?;
bpf_pin_object(fd.as_fd().as_raw_fd(), &path_string).map_err(|(_, io_error)| SyscallError {
call: "BPF_OBJ_PIN",
io_error,
Expand Down Expand Up @@ -1073,7 +1077,10 @@ impl ProgramInfo {

/// Loads a program from a pinned path in bpffs.
pub fn from_pin<P: AsRef<Path>>(path: P) -> Result<Self, ProgramError> {
let path_string = CString::new(path.as_ref().to_str().unwrap()).unwrap();
use std::os::unix::ffi::OsStrExt as _;

// TODO: avoid this unwrap by adding a new error variant.
let path_string = CString::new(path.as_ref().as_os_str().as_bytes()).unwrap();
let fd = bpf_get_object(&path_string).map_err(|(_, io_error)| SyscallError {
call: "BPF_OBJ_GET",
io_error,
Expand Down
Loading

0 comments on commit 0bba9b1

Please sign in to comment.