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.

Sign up for GitHub

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

fix(driver): simplify exe_upper_layer extraction #1960

Merged
merged 3 commits into from
Jul 23, 2024
Merged

fix(driver): simplify exe_upper_layer extraction #1960

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9299ebe
cleanup(driver): simplify exe_upper_layer extraction
Andreagit97 Jul 15, 2024
8b3c688
fix(driver): correctly handle upper_dentry in the kmod
Andreagit97 Jul 19, 2024
1aded82
fix(driver/kmod): avoid `mixed declarations and code` error
Andreagit97 Jul 23, 2024
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
125 changes: 37 additions & 88 deletions driver/bpf/fillers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2314,48 +2314,28 @@ static __always_inline bool get_exe_writable(struct inode *inode, struct cred *c
return false;
}

static __always_inline bool get_exe_upper_layer(struct dentry *dentry, struct super_block *sb)
static __always_inline bool get_exe_upper_layer(struct file *file)
{
struct dentry* dentry = NULL;
bpf_probe_read_kernel(&dentry, sizeof(dentry), &file->f_path.dentry);
struct super_block* sb = (struct super_block*)_READ(dentry->d_sb);
unsigned long sb_magic = _READ(sb->s_magic);
if(sb_magic == PPM_OVERLAYFS_SUPER_MAGIC)
{
struct dentry *upper_dentry = NULL;
char *vfs_inode = (char *)_READ(dentry->d_inode);

// Pointer arithmetics due to unexported ovl_inode struct
// warning: this works if and only if the dentry pointer is placed right after the inode struct
struct dentry *tmp = (struct dentry *)(vfs_inode + sizeof(struct inode));
upper_dentry = _READ(tmp);
if(!upper_dentry)
{
return false;
}

unsigned int d_flags = _READ(dentry->d_flags);
bool disconnected = (d_flags & DCACHE_DISCONNECTED);
if(disconnected)
{
return true;
}

#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 16, 0)
struct ovl_entry *oe = (struct ovl_entry*)_READ(dentry->d_fsdata);
unsigned long has_upper = (unsigned long)_READ(oe->has_upper);
#elif LINUX_VERSION_CODE < KERNEL_VERSION(6, 5, 0)
struct ovl_entry *oe = (struct ovl_entry*)_READ(dentry->d_fsdata);
unsigned long flags = _READ(oe->flags);
unsigned long has_upper = (flags & (1U << (OVL_E_UPPER_ALIAS)));
#else
unsigned long flags = (unsigned long)_READ(dentry->d_fsdata);
unsigned long has_upper = (flags & (1U << (OVL_E_UPPER_ALIAS)));
#endif
if(sb_magic != PPM_OVERLAYFS_SUPER_MAGIC)
{
return false;
}

if(has_upper)
{
return true;
}
char *vfs_inode = (char *)_READ(dentry->d_inode);
struct dentry *upper_dentry = NULL;
bpf_probe_read_kernel(&upper_dentry, sizeof(upper_dentry), (char *)vfs_inode + sizeof(struct inode));
if(!upper_dentry)
{
return false;
}
return false;

struct inode *upper_ino = _READ(upper_dentry->d_inode);
return _READ(upper_ino->i_ino) != 0;
}

FILLER(proc_startupdate, true)
Expand Down Expand Up @@ -2867,43 +2847,27 @@ FILLER(execve_extra_tail_1, true)
struct cred *cred = (struct cred *)_READ(task->cred);
struct file *exe_file = get_exe_file(task);
struct inode *inode = get_file_inode(exe_file);
struct path f_path = (struct path)_READ(exe_file->f_path);
struct dentry* dentry = f_path.dentry;

#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 18, 0)
struct super_block* sb = _READ(dentry->d_sb);
#else
struct super_block *sb = _READ(inode->i_sb);
#endif
Comment on lines -2873 to -2877
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@therealbobo @loresuso It's unclear why we need this distinction...why cannot we take the superblock always from the dentry?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR LGTM! Thanks andre!
Once ☝️ gets an answer, i am ready to approve :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember it was a bit of trial and error. Something changed in OverlayFS in kernel 4.18 but I can't recall at the moment, trying to look around and I'll keep you posted.
Do we have tests to run if we try to get it always from the dentry? We could try that in the meantime in my opinion, to check whether it is working as expected

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes in our current CI, we have 2 tests for it that are running on a kernel version > 4.18... Moreover, I tested it on at least 3 different machines during the implementation and this seems fine 🤔 I expect the dentry always to have a reference to the superblock, am I missing something?

Copy link
Member

@loresuso loresuso Jul 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the dentry should always have a reference to the superblock. Anyway, is it hard to add another kernel version for testing that is less than 4.18?
Coverage for this would be really helpful

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately at the moment, we don't have a setup for these machines. But in this case, I touched the ifdef for kernels >= 4.18 so the kernels < 4.18 should be untouched

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember the exact reason and it was the outcome of some trial and error. BTW I tested (with the driver_tests framework) your implementation with 4.14, 4.17, 4.20, 5.4, 6.2,6.9 e 6.10 kernels and it seems to work just fine 😄


/* `exe_writable` and `exe_upper_layer` flag logic */
bool exe_writable = false;
bool exe_upper_layer = false;

uint32_t flags = 0;
kuid_t euid;
kuid_t euid = {0};

if(sb && inode)
if(inode)
{
/*
* exe_writable
*/
exe_writable = get_exe_writable(inode, cred);
bool exe_writable = get_exe_writable(inode, cred);
if (exe_writable)
{
flags |= PPM_EXE_WRITABLE;
}
}

/*
* exe_upper_layer
*/
exe_upper_layer = get_exe_upper_layer(dentry,sb);
if (exe_upper_layer)
{
flags |= PPM_EXE_UPPER_LAYER;
}

// write all additional flags for execve family here...
/*
* exe_upper_layer
*/
if(exe_file && get_exe_upper_layer(exe_file))
{
flags |= PPM_EXE_UPPER_LAYER;
}

if(exe_file && get_exe_from_memfd(exe_file))
Expand Down Expand Up @@ -6791,42 +6755,27 @@ FILLER(sched_prog_exec_4, false)
struct cred *cred = (struct cred *)_READ(task->cred);
struct file *exe_file = get_exe_file(task);
struct inode *inode = get_file_inode(exe_file);
struct path f_path = (struct path)_READ(exe_file->f_path);
struct dentry* dentry = f_path.dentry;

#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 18, 0)
struct super_block* sb = _READ(dentry->d_sb);
#else
struct super_block *sb = _READ(inode->i_sb);
#endif

/* `exe_writable` and `exe_upper_layer` flag logic */
bool exe_writable = false;
bool exe_upper_layer = false;
uint32_t flags = 0;
kuid_t euid;
kuid_t euid = {0};

if(sb && inode)
if(inode)
{
/*
* exe_writable
*/
exe_writable = get_exe_writable(inode, cred);
bool exe_writable = get_exe_writable(inode, cred);
if (exe_writable)
{
flags |= PPM_EXE_WRITABLE;
}
}

/*
* exe_upper_layer
*/
exe_upper_layer = get_exe_upper_layer(dentry,sb);
if (exe_upper_layer)
{
flags |= PPM_EXE_UPPER_LAYER;
}

// write all additional flags for execve family here...
/*
* exe_upper_layer
*/
if (exe_file && get_exe_upper_layer(exe_file))
{
flags |= PPM_EXE_UPPER_LAYER;
}

if(exe_file && get_exe_from_memfd(exe_file))
Expand Down
31 changes: 0 additions & 31 deletions driver/bpf/missing_definitions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,37 +10,6 @@ or GPL2.txt for full copies of the license.
#ifndef __BPF_MISSING_DEFINITIONS_H__
#define __BPF_MISSING_DEFINITIONS_H__

#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 16, 0)
struct ovl_entry {
union {
struct {
unsigned long has_upper;
bool opaque;
};
struct rcu_head rcu;
};
unsigned numlower;
struct path lowerstack[];
};
#else
struct ovl_entry {
union {
struct {
unsigned long flags;
};
struct rcu_head rcu;
};
unsigned numlower;
//struct ovl_path lowerstack[];
};

enum ovl_entry_flag {
OVL_E_UPPER_ALIAS,
OVL_E_OPAQUE,
OVL_E_CONNECTED,
};
#endif

#include <linux/mount.h>
/* This require the inlclude `linux/mount.h` for `vfsmount` definition */
struct mount {
Expand Down
5 changes: 0 additions & 5 deletions driver/modern_bpf/definitions/missing_definitions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1586,9 +1586,4 @@
#define MODULE_INIT_COMPRESSED_FILE 4
/*==================================== FINIT FLAGS ================================*/

/*==================================== OVERLAY FLAGS ================================*/
#define DCACHE_DISCONNECTED 0x20
#define OVL_E_UPPER_ALIAS 0
/*==================================== OVERLAY FLAGS ================================*/

#endif /* __MISSING_DEFINITIONS_H__ */
5 changes: 0 additions & 5 deletions driver/modern_bpf/definitions/struct_flavors.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,6 @@ struct inode___v6_7 {
struct timespec64 __i_mtime;
};

struct ovl_entry___before_v6_5
{
long unsigned int flags;
};

#ifndef BPF_NO_PRESERVE_ACCESS_INDEX
#pragma clang attribute pop
#endif
Expand Down
65 changes: 20 additions & 45 deletions driver/modern_bpf/helpers/extract/extract_from_kernel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -831,60 +831,35 @@ static __always_inline void extract__egid(struct task_struct *task, uint32_t *eg
// EXECVE FLAGS EXTRACTION
////////////////////////

static __always_inline bool extract__exe_upper_layer(struct inode *inode, struct file *exe_file)
static __always_inline bool extract__exe_upper_layer(struct file *file)
{
unsigned long sb_magic = BPF_CORE_READ(inode, i_sb, s_magic);
struct dentry *dentry = (struct dentry *)BPF_CORE_READ(file, f_path.dentry);
unsigned long sb_magic = BPF_CORE_READ(dentry, d_sb, s_magic);

if(sb_magic == PPM_OVERLAYFS_SUPER_MAGIC)
if(sb_magic != PPM_OVERLAYFS_SUPER_MAGIC)
{
struct dentry *upper_dentry = NULL;
char *vfs_inode = (char *)inode;
unsigned long inode_size = bpf_core_type_size(struct inode);
if(!inode_size)
{
return false;
}

bpf_probe_read_kernel(&upper_dentry, sizeof(upper_dentry), vfs_inode + inode_size);

if(!upper_dentry)
{
return false;
}

struct dentry *dentry = (struct dentry *)BPF_CORE_READ(exe_file, f_path.dentry);

unsigned int d_flags = BPF_CORE_READ(dentry, d_flags);
bool disconnected = (d_flags & DCACHE_DISCONNECTED);
if(disconnected)
{
return true;
}

unsigned long flags = 0;
if(bpf_core_field_exists(((struct ovl_entry___before_v6_5*)0)->flags))
{
// kernel <6.5
struct ovl_entry___before_v6_5 *oe = (struct ovl_entry___before_v6_5*)BPF_CORE_READ(dentry, d_fsdata);
flags = (unsigned long)BPF_CORE_READ(oe, flags);
}
else
{
// In kernels >=6.5 d_fsdata represents an ovl_entry_flag.
flags = (unsigned long)BPF_CORE_READ(dentry, d_fsdata);
}
return false;
}

unsigned long has_upper = (flags & (1U << (OVL_E_UPPER_ALIAS)));
if(has_upper)
{
return true;
}
char *vfs_inode = (char *)BPF_CORE_READ(dentry, d_inode);
// We need to compute the size of the inode struct at load time since it can change between kernel versions
unsigned long inode_size = bpf_core_type_size(struct inode);
if(!inode_size)
{
return false;
}

struct dentry *upper_dentry = NULL;
bpf_probe_read_kernel(&upper_dentry, sizeof(upper_dentry), (char *)vfs_inode + inode_size);
if(!upper_dentry)
{
return false;
}

return false;
return BPF_CORE_READ(upper_dentry, d_inode, i_ino) != 0;
}


/*
* Detect whether the file being referenced is an anonymous file created using memfd_create()
* and is being executed by referencing its file descriptor (fd). This type of file does not
Expand Down
2 changes: 1 addition & 1 deletion driver/modern_bpf/programs/attached/events/sched_process_exec.bpf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ int BPF_PROG(t1_sched_p_exec,
{
flags |= PPM_EXE_WRITABLE;
}
if(extract__exe_upper_layer(exe_inode, exe_file))
if(extract__exe_upper_layer(exe_file))
{
flags |= PPM_EXE_UPPER_LAYER;
}
Expand Down
2 changes: 1 addition & 1 deletion driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/execve.bpf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ int BPF_PROG(t1_execve_x,
{
flags |= PPM_EXE_WRITABLE;
}
if(extract__exe_upper_layer(exe_inode, exe_file))
if(extract__exe_upper_layer(exe_file))
{
flags |= PPM_EXE_UPPER_LAYER;
}
Expand Down
2 changes: 1 addition & 1 deletion driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/execveat.bpf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ int BPF_PROG(t1_execveat_x,
{
flags |= PPM_EXE_WRITABLE;
}
if(extract__exe_upper_layer(exe_inode, exe_file))
if(extract__exe_upper_layer(exe_file))
{
flags |= PPM_EXE_UPPER_LAYER;
}
Expand Down
Loading
Loading