Skip to content

Commit

Permalink
fix: dereference of modified ctx ptr with clang16 (#177)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mmat11
mmat11 authored Oct 11, 2023
1 parent 84e2581 commit 039ceef
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 46 deletions.
20 changes: 7 additions & 13 deletions GPL/Events/File/Probe.bpf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ static int vfs_unlink__exit(int ret)
ebpf_pid_info__fill(&event->pids, task);

struct path p;
p.dentry = &state->unlink.de;
p.dentry = state->unlink.de;
p.mnt = state->unlink.mnt;
event->mntns = mntns(task);
bpf_get_current_comm(event->comm, TASK_COMM_LEN);
Expand Down Expand Up @@ -162,7 +162,7 @@ int BPF_KRETPROBE(kretprobe__vfs_unlink, int ret)
return vfs_unlink__exit(ret);
}

static int vfs_unlink__enter(struct dentry de)
static int vfs_unlink__enter(struct dentry *de)
{
struct ebpf_events_state *state = ebpf_events_state__get(EBPF_EVENTS_STATE_UNLINK);
if (!state || state->unlink.step != UNLINK_STATE_MOUNT_SET) {
Expand All @@ -180,18 +180,15 @@ static int vfs_unlink__enter(struct dentry de)
SEC("fentry/vfs_unlink")
int BPF_PROG(fentry__vfs_unlink)
{
struct dentry *tmp = FUNC_ARG_READ(___type(tmp), vfs_unlink, dentry);
struct dentry de;
bpf_core_read(&de, sizeof(de), tmp);
struct dentry *de = FUNC_ARG_READ(___type(de), vfs_unlink, dentry);
return vfs_unlink__enter(de);
}

SEC("kprobe/vfs_unlink")
int BPF_KPROBE(kprobe__vfs_unlink)
{
struct dentry de;
int err = FUNC_ARG_READ_PTREGS(de, vfs_unlink, dentry);
if (err) {
struct dentry *de;
if (FUNC_ARG_READ_PTREGS(de, vfs_unlink, dentry)) {
bpf_printk("kprobe__vfs_unlink: error reading dentry\n");
return 0;
}
Expand Down Expand Up @@ -346,7 +343,6 @@ int BPF_PROG(fentry__vfs_rename)
SEC("kprobe/vfs_rename")
int BPF_KPROBE(kprobe__vfs_rename)
{
int err;
struct dentry *old_dentry, *new_dentry;

if (FUNC_ARG_EXISTS(vfs_rename, rd)) {
Expand All @@ -357,13 +353,11 @@ int BPF_KPROBE(kprobe__vfs_rename)
new_dentry = rd.new_dentry;
} else {
/* Dentries are accessible from ctx */
err = FUNC_ARG_READ_PTREGS_NODEREF(old_dentry, vfs_rename, old_dentry);
if (err) {
if (FUNC_ARG_READ_PTREGS(old_dentry, vfs_rename, old_dentry)) {
bpf_printk("kprobe__vfs_rename: error reading old_dentry\n");
return 0;
}
err = FUNC_ARG_READ_PTREGS_NODEREF(new_dentry, vfs_rename, new_dentry);
if (err) {
if (FUNC_ARG_READ_PTREGS(new_dentry, vfs_rename, new_dentry)) {
bpf_printk("kprobe__vfs_rename: error reading new_dentry\n");
return 0;
}
Expand Down
36 changes: 4 additions & 32 deletions GPL/Events/Helpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,9 @@ const volatile int consumer_pid = 0;
})

/*
* Reads the specified argument from struct pt_regs without dereferencing it
* (unlike FUNC_ARG_READ_PTREGS) (i.e. we get a pointer to the argument, not
* the argument itself). Note that we first have to read the value in struct
* pt_regs into a volatile temporary (_dst). Without this, LLVM can generate
* code like the following, which will fail to verify:
* Reads the specified argument from struct pt_regs without dereferencing it. Note that
* we first have to read the value in struct pt_regs into a volatile temporary (_dst).
* Without this, LLVM can generate code like the following, which will fail to verify:
*
* r3 = 8 # The register value we want to read is at offset 8 in the context
* r2 = r1 # r1 = ctx pointer
Expand All @@ -52,7 +50,7 @@ const volatile int consumer_pid = 0;
* r3 = *(u64 *)(r2 + 8) # Dereference it, putting the increment in the dereference insn
* ...pass r3 to a function
*/
#define FUNC_ARG_READ_PTREGS_NODEREF(dst, func, arg) \
#define FUNC_ARG_READ_PTREGS(dst, func, arg) \
({ \
int ret = 0; \
volatile typeof(dst) _dst; \
Expand Down Expand Up @@ -80,32 +78,6 @@ const volatile int consumer_pid = 0;
ret; \
})

#define FUNC_ARG_READ_PTREGS(dst, func, arg) \
({ \
int ret = 0; \
switch (arg__##func##__##arg##__) { \
case 0: \
bpf_core_read(&dst, sizeof(dst), (void *)PT_REGS_PARM1(ctx)); \
break; \
case 1: \
bpf_core_read(&dst, sizeof(dst), (void *)PT_REGS_PARM2(ctx)); \
break; \
case 2: \
bpf_core_read(&dst, sizeof(dst), (void *)PT_REGS_PARM3(ctx)); \
break; \
case 3: \
bpf_core_read(&dst, sizeof(dst), (void *)PT_REGS_PARM4(ctx)); \
break; \
case 4: \
bpf_core_read(&dst, sizeof(dst), (void *)PT_REGS_PARM5(ctx)); \
break; \
default: \
ret = -1; \
}; \
barrier(); \
ret; \
})

#define DECL_FUNC_RET(func) const volatile int ret__##func##__ = 0;
#define FUNC_RET_READ(type, func) \
({ \
Expand Down
2 changes: 1 addition & 1 deletion GPL/Events/State.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ enum ebpf_events_unlink_state_step {
struct ebpf_events_unlink_state {
enum ebpf_events_unlink_state_step step;
struct vfsmount *mnt;
struct dentry de;
struct dentry *de;
};

enum ebpf_events_rename_state_step {
Expand Down

0 comments on commit 039ceef

Please sign in to comment.