netdev - Re: [PATCH v3 bpf-next 5/5] selftests/bpf: Tweak cgroup kfunc test.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Y/620WKXKpNIzHQL@maniforge>
Date:   Tue, 28 Feb 2023 20:22:09 -0600
From:   David Vernet <void@...ifault.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <martin.lau@...nel.org>,
        Dave Marchevsky <davemarchevsky@...a.com>,
        Tejun Heo <tj@...nel.org>,
        Kumar Kartikeya Dwivedi <memxor@...il.com>,
        Network Development <netdev@...r.kernel.org>,
        bpf <bpf@...r.kernel.org>, Kernel Team <kernel-team@...com>
Subject: Re: [PATCH v3 bpf-next 5/5] selftests/bpf: Tweak cgroup kfunc test.

On Tue, Feb 28, 2023 at 04:29:18PM -0800, Alexei Starovoitov wrote:
> On Tue, Feb 28, 2023 at 9:07 AM David Vernet <void@...ifault.com> wrote:
> > libbpf: prog 'on_lookup': failed to load: -13
> > libbpf: failed to load object 'cgrp_ls_recursion'
> > libbpf: failed to load BPF skeleton 'cgrp_ls_recursion': -13
> > test_recursion:FAIL:skel_open_and_load unexpected error: -13
> > #43/3    cgrp_local_storage/recursion:FAIL
> > #43      cgrp_local_storage:FAIL
> >
> > All error logs:
> > test_cgrp_local_storage:PASS:join_cgroup /cgrp_local_storage 0 nsec
> > libbpf: prog 'on_lookup': BPF program load failed: Permission denied
> > libbpf: prog 'on_lookup': -- BEGIN PROG LOAD LOG --
> > reg type unsupported for arg#0 function on_lookup#16
> > 0: R1=ctx(off=0,imm=0) R10=fp0
> > ; struct task_struct *task = bpf_get_current_task_btf();
> > 0: (85) call bpf_get_current_task_btf#158     ; R0_w=trusted_ptr_task_struct(off=0,imm=0)
> > 1: (bf) r6 = r0                       ; R0_w=trusted_ptr_task_struct(off=0,imm=0) R6_w=trusted_ptr_task_struct(off=0,imm=0)
> > ; bpf_cgrp_storage_delete(&map_a, task->cgroups->dfl_cgrp);
> > 2: (79) r1 = *(u64 *)(r6 +2296)       ; R1_w=rcu_ptr_or_null_css_set(off=0,imm=0) R6_w=trusted_ptr_task_struct(off=0,imm=0)
> > ; bpf_cgrp_storage_delete(&map_a, task->cgroups->dfl_cgrp);
> > 3: (79) r2 = *(u64 *)(r1 +120)
> > R1 invalid mem access 'rcu_ptr_or_null_'
> 
> This one was tricky :)
> Turned out btf_nested_type_is_trusted() was able to find
> 'cgroups' field in gcc compiled kernel and was failing on clang
> compiled kernel because patch 2 did:
> BTF_TYPE_SAFE_NESTED(struct task_struct) {
>         const cpumask_t *cpus_ptr;
>         struct css_set *cgroups;
> };
> instead of
> BTF_TYPE_SAFE_NESTED(struct task_struct) {
>         const cpumask_t *cpus_ptr;
>         struct css_set __rcu *cgroups;
> };
> The missing tag was causing a miscompare.

Ahh, sorry I missed that in review. Once your patch set lands I'll add a
very loud comment here so that it's not missed in the future.

> Something to keep in mind.
> This ugliness will go away once GCC supports btf tag.

Looking forward to that day.

Given that you'll apply that fix to [0] here's my stamp for this patch:

Acked-by: David Vernet <void@...ifault.com>

[0]: https://lore.kernel.org/all/20230228040121.94253-3-alexei.starovoitov@gmail.com/