On Sat, Jul 8, 2023 at 7:59 PM Yafang Shao <laoar.shao@xxxxxxxxx> wrote:
>
> We are utilizing BPF LSM to monitor BPF operations within our container
> environment. When we add support for raw_tracepoint, it hits below
> error.
>
> ; (const void *)attr->raw_tracepoint.name);
> 27: (79) r3 = *(u64 *)(r2 +0)
> access beyond the end of member map_type (mend:4) in struct (anon) with off 0 size 8
>
> It can be reproduced with below BPF prog.
>
> SEC("lsm/bpf")
> int BPF_PROG(bpf_audit, int cmd, union bpf_attr *attr, unsigned int size)
> {
> switch (cmd) {
> case BPF_RAW_TRACEPOINT_OPEN:
> bpf_printk("raw_tracepoint is %s", attr->raw_tracepoint.name);
> break;
> default:
> break;
> }
> return 0;
> }
>
> The reason is that when accessing a field in a union, such as bpf_attr,
> if the field is located within a nested struct that is not the first
> member of the union, it can result in incorrect field verification.
>
> union bpf_attr {
> struct {
> __u32 map_type; <<<< Actually it will find that field.
> __u32 key_size;
> __u32 value_size;
> ...
> };
> ...
> struct {
> __u64 name; <<<< We want to verify this field.
> __u32 prog_fd;
> } raw_tracepoint;
> };
>
> Considering the potential deep nesting levels, finding a perfect
> solution to address this issue has proven challenging. Therefore, I
> propose a solution where we simply skip the verification process if the
> field in question is located within a union.
>
> Fixes: 7e3617a72df3 ("bpf: Add array support to btf_struct_access")
> Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> ---
> kernel/bpf/btf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index fae6fc24a845..a542760c807a 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6368,7 +6368,7 @@ static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf,
> * that also allows using an array of int as a scratch
> * space. e.g. skb->cb[].
> */
> - if (off + size > mtrue_end) {
> + if (off + size > mtrue_end && !(*flag & PTR_UNTRUSTED)) {
The selftest for this condition is missing.
