Skip to content

Commit 0318093

Browse files
committed
feat: improve handling of path buffers
Since paths can be in non UTF-8 formats, we should strive to use types that are able to handle this situation when possible. In the case of the exe_path and filename fields that come from the kernel, handling has been changed to treat them as binary blobs (same way the kernel d_path function does) and the length is now being sent to userspace. With this change, userspace is able to reconstruct a path in a fully safe way from the path buffer, treating it as a slice and using the first len - 1 (exclude the null terminator) bytes. This way we are able to preserve the path all the way to the gRPC event, which requires UTF-8 compliant strings. In a future change we might want to change path handling in our protobuffs to use byte blobs instead, but this will probably also require an effort from front end to properly display the data.
1 parent 37d5783 commit 0318093

File tree

7 files changed

+37
-22
lines changed

7 files changed

+37
-22
lines changed

fact-ebpf/src/bpf/d_path.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ __always_inline static long __d_path(const struct path* path, char* buf, int buf
7777
dentry = parent;
7878
}
7979

80-
bpf_probe_read_str(buf, buflen, &helper->buf[offset]);
80+
bpf_probe_read(buf, buflen - offset, &helper->buf[offset]);
8181
return buflen - offset;
8282
}
8383

fact-ebpf/src/bpf/events.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
__always_inline static void submit_event(struct metrics_by_hook_t* m,
1515
file_activity_type_t event_type,
16-
const char filename[PATH_MAX],
16+
struct bound_path_t* path,
1717
inode_key_t* inode,
1818
bool use_bpf_d_path) {
1919
struct event_t* event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0);
@@ -25,7 +25,8 @@ __always_inline static void submit_event(struct metrics_by_hook_t* m,
2525
event->type = event_type;
2626
event->timestamp = bpf_ktime_get_boot_ns();
2727
inode_copy_or_reset(&event->inode, inode);
28-
bpf_probe_read_str(event->filename, PATH_MAX, filename);
28+
bpf_probe_read(event->filename, path->len & (PATH_MAX - 1), path->path);
29+
event->filename_len = path->len;
2930

3031
struct helper_t* helper = get_helper();
3132
if (helper == NULL) {

fact-ebpf/src/bpf/main.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ int BPF_PROG(trace_file_open, struct file* file) {
5656
break;
5757
}
5858

59-
submit_event(&m->file_open, event_type, path->path, &inode_key, true);
59+
submit_event(&m->file_open, event_type, path, &inode_key, true);
6060

6161
return 0;
6262

@@ -116,7 +116,7 @@ int BPF_PROG(trace_path_unlink, struct path* dir, struct dentry* dentry) {
116116

117117
submit_event(&m->path_unlink,
118118
FILE_ACTIVITY_UNLINK,
119-
path->path,
119+
path,
120120
&inode_key,
121121
path_unlink_supports_bpf_d_path);
122122
return 0;

fact-ebpf/src/bpf/process.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ __always_inline static int64_t process_fill(process_t* p, bool use_bpf_d_path) {
131131
return -1;
132132
}
133133

134-
d_path(&task->mm->exe_file->f_path, p->exe_path, PATH_MAX, use_bpf_d_path);
134+
p->exe_path_len = d_path(&task->mm->exe_file->f_path, p->exe_path, PATH_MAX, use_bpf_d_path);
135135

136136
const char* cg = get_memory_cgroup(helper);
137137
if (cg != NULL) {

fact-ebpf/src/bpf/types.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ typedef struct process_t {
2222
char args[4096];
2323
unsigned int args_len;
2424
char exe_path[PATH_MAX];
25+
short unsigned int exe_path_len;
2526
char memory_cgroup[PATH_MAX];
2627
unsigned int uid;
2728
unsigned int gid;
@@ -53,6 +54,7 @@ struct event_t {
5354
unsigned long timestamp;
5455
process_t process;
5556
char filename[PATH_MAX];
57+
short unsigned int filename_len;
5658
inode_key_t inode;
5759
file_activity_type_t type;
5860
};

fact/src/event/mod.rs

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,28 @@
11
#[cfg(test)]
22
use std::time::{SystemTime, UNIX_EPOCH};
3-
use std::{ffi::CStr, os::raw::c_char, path::PathBuf};
3+
use std::{
4+
ffi::{CStr, OsString},
5+
os::{raw::c_char, unix::ffi::OsStringExt},
6+
path::PathBuf,
7+
};
48

59
use serde::Serialize;
610

7-
use fact_ebpf::{event_t, file_activity_type_t, inode_key_t, PATH_MAX};
11+
use fact_ebpf::{event_t, file_activity_type_t, inode_key_t};
812

913
use crate::host_info;
1014
use process::Process;
1115

1216
pub(crate) mod process;
1317

18+
fn slice_to_pathbuf(s: &[c_char]) -> PathBuf {
19+
#[cfg(target_arch = "x86_64")]
20+
let v = s.iter().map(|c| *c as u8).collect::<Vec<u8>>();
21+
#[cfg(not(target_arch = "x86_64"))]
22+
let v = s.to_vec();
23+
OsString::from_vec(v).into()
24+
}
25+
1426
fn slice_to_string(s: &[c_char]) -> anyhow::Result<String> {
1527
Ok(unsafe { CStr::from_ptr(s.as_ptr()) }.to_str()?.to_owned())
1628
}
@@ -85,7 +97,9 @@ impl TryFrom<&event_t> for Event {
8597
fn try_from(value: &event_t) -> Result<Self, Self::Error> {
8698
let process = Process::try_from(value.process)?;
8799
let timestamp = host_info::get_boot_time() + value.timestamp;
88-
let file = FileData::new(value.type_, value.filename, value.inode)?;
100+
let filename_len = value.filename_len as usize;
101+
let (filename, _) = value.filename.as_slice().split_at(filename_len - 1);
102+
let file = FileData::new(value.type_, filename, value.inode)?;
89103

90104
Ok(Event {
91105
timestamp,
@@ -128,7 +142,7 @@ pub enum FileData {
128142
impl FileData {
129143
pub fn new(
130144
event_type: file_activity_type_t,
131-
filename: [c_char; PATH_MAX as usize],
145+
filename: &[c_char],
132146
inode: inode_key_t,
133147
) -> anyhow::Result<Self> {
134148
let inner = BaseFileData::new(filename, inode)?;
@@ -185,8 +199,8 @@ pub struct BaseFileData {
185199
}
186200

187201
impl BaseFileData {
188-
pub fn new(filename: [c_char; PATH_MAX as usize], inode: inode_key_t) -> anyhow::Result<Self> {
189-
let filename = slice_to_string(&filename)?.into();
202+
pub fn new(filename: &[c_char], inode: inode_key_t) -> anyhow::Result<Self> {
203+
let filename = slice_to_pathbuf(filename);
190204

191205
Ok(BaseFileData {
192206
filename,

fact/src/event/process.rs

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
use std::ffi::CStr;
1+
use std::{ffi::CStr, path::PathBuf};
22

33
use fact_ebpf::{lineage_t, process_t};
44
use serde::Serialize;
55
use uuid::Uuid;
66

77
use crate::host_info;
88

9-
use super::slice_to_string;
9+
use super::{slice_to_pathbuf, slice_to_string};
1010

1111
#[derive(Debug, Clone, Default, Serialize)]
1212
pub struct Lineage {
@@ -48,7 +48,7 @@ impl From<Lineage> for fact_api::process_signal::LineageInfo {
4848
pub struct Process {
4949
comm: String,
5050
args: Vec<String>,
51-
exe_path: String,
51+
exe_path: PathBuf,
5252
container_id: Option<String>,
5353
uid: u32,
5454
username: &'static str,
@@ -66,11 +66,7 @@ impl Process {
6666
pub fn current() -> Self {
6767
use crate::host_info::{get_host_mount_ns, get_mount_ns};
6868

69-
let exe_path = std::env::current_exe()
70-
.expect("Failed to get current exe")
71-
.into_os_string()
72-
.into_string()
73-
.unwrap();
69+
let exe_path = std::env::current_exe().expect("Failed to get current exe");
7470
let args = std::env::args().collect::<Vec<_>>();
7571
let cgroup = std::fs::read_to_string("/proc/self/cgroup").expect("Failed to read cgroup");
7672
let container_id = Process::extract_container_id(&cgroup);
@@ -142,7 +138,9 @@ impl TryFrom<process_t> for Process {
142138

143139
fn try_from(value: process_t) -> Result<Self, Self::Error> {
144140
let comm = slice_to_string(value.comm.as_slice())?;
145-
let exe_path = slice_to_string(value.exe_path.as_slice())?;
141+
let exe_path_len = value.exe_path_len as usize;
142+
let (exe_path, _) = value.exe_path.as_slice().split_at(exe_path_len - 1);
143+
let exe_path = slice_to_pathbuf(exe_path);
146144
let memory_cgroup = unsafe { CStr::from_ptr(value.memory_cgroup.as_ptr()) }.to_str()?;
147145
let container_id = Process::extract_container_id(memory_cgroup);
148146
let in_root_mount_ns = value.in_root_mount_ns != 0;
@@ -213,7 +211,7 @@ impl From<Process> for fact_api::ProcessSignal {
213211
creation_time: None,
214212
name: comm,
215213
args,
216-
exec_file_path: exe_path,
214+
exec_file_path: exe_path.to_string_lossy().into_owned(),
217215
pid,
218216
uid,
219217
gid,

0 commit comments

Comments
 (0)