iovisor / bcc

BCC - Tools for BPF-based Linux IO analysis, networking, monitoring, and more
Apache License 2.0
20.57k stars 3.88k forks source link

Void pointer in LSM probe arguments cannot be used #3551

Open sleepyMia opened 3 years ago

sleepyMia commented 3 years ago

When creating an LSM probe for security_inode_setxattr attempting to read the argument const void *value using bpf_probe_read_user_str causes the following error:

bpf: Failed to load program: Permission denied 0: (79) r6 = *(u64 *)(r1 +16) func 'bpf_lsm_inode_setxattr' arg2 type UNKNOWN is not a struct invalid bpf_context access off=16 size=8 processed 1 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0

This occurs with the following line bpf_probe_read_user_str(&new_xattr.x_value, sizeof(new_xattr.x_value), (char*)value);, seemingly due to the use of value.

I'm wondering what could be causing this, as I've been unable to fix this, and need to access the value referenced by the value pointer.

chenhengqi commented 3 years ago

const void *value should be arg4, do you have a wrong function signature ?

sleepyMia commented 3 years ago

I've checked and confirmed that I'm matching the function signature for the version of the kernel I'm working with (5.10.21-051021-generic). The error I'm getting seems to still be the same, and only occurs when I attempt to use bpf_probe_read_kernel_str(&new_xattr.x_value, sizeof(new_xattr.x_value), (char*)value); with the void pointer from LSM_PROBE(inode_setxattr, struct dentry *dentry, const char *name, const void *value, size_t size, int flags)

chenhengqi commented 3 years ago
#!/usr/bin/python

from bcc import BPF

bpf_text = '''
#include <linux/blkdev.h>

LSM_PROBE(inode_setxattr, struct dentry *dentry, const char *name, const void *value, size_t size, int flags)
{
    const char *raw = value;
    char data[64];
    int i;

    for (i = 0; i < size && i < 64; i++) {
        data[i] = raw[i];
    }
    return 0;
}
'''

if not BPF.support_lsm():
    raise 1

b = BPF(text=bpf_text, debug=12)
b.trace_print()

@sleepyMia You may try this trick to workaround the problem.

cc @yonghong-song for help.

yonghong-song commented 3 years ago

For error message 'bpf_lsm_inode_setxattr' arg2 type UNKNOWN is not a struct invalid bpf_context access off=16 size=8 The error is issued by verifier.

I think this is a verifier limitation. Could you change "const void value" to "void value" to see whether it works or not?

yonghong-song commented 3 years ago

Basically, in verifier, the value type looks like: ptr -> const -> void But the verifier only checks ptr -> void properly. The fix is in verifier we should remove all the modifiers for the pointee type before checking pointee type.

yonghong-song commented 3 years ago

@sleepyMia How do you decide "const void *value" is a user pointer? The kernel code certainly didn't have __user annotation.

sleepyMia commented 3 years ago

I think I may have tried that because of seeing __user for user_namespace, but that was a mistake. I later changed it to bpf_probe_read_kernel_str. I was a bit confused by the error message since I don't work much with C, so I was just trying things, I should have clarified that.

sleepyMia commented 3 years ago

I tried changing to void *value, and got the following error:


0: (79) r6 = *(u64 *)(r1 +16)
func 'bpf_lsm_inode_setxattr' arg2 type UNKNOWN is not a struct
invalid bpf_context access off=16 size=8
processed 1 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0```
yonghong-song commented 3 years ago

Okay, I see this actually needs the kernel change since the btf type will be extracted from kernel vmlinux btf instead of bpf program. If you could rebuild your own kernel, could you try whether the following kernel change can fix your issue?

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index c395024610ed..c3ff451ac3cf 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4709,6 +4709,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
        const char *tname = prog->aux->attach_func_name;
        struct bpf_verifier_log *log = info->log;
        const struct btf_param *args;
+       const struct btf_type *t1;
        u32 nr_args, arg;
        int i, ret;

@@ -4810,7 +4811,10 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
                }
        }

-       if (t->type == 0)
+       t1 = btf_type_by_id(btf, t->type);
+       while (btf_type_is_modifier(t1))
+               t = btf_type_by_id(btf, t->type);
+       if (btf_type_is_void(t))
                /* This is a pointer to void.
                 * It is the same as scalar from the verifier safety pov.
                 * No further pointer walking is allowed.