netdev - Re: [PATCH bpf-next v3] libbpf: support weak typed ksyms.

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4Bzbhtty_XjpPxSjfe4zEHAfWuQ4th15eLgomT2BDHUQ7jw@mail.gmail.com>
Date:   Fri, 13 Aug 2021 16:01:43 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Hao Luo <haoluo@...gle.com>
Cc:     Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andriin@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>
Subject: Re: [PATCH bpf-next v3] libbpf: support weak typed ksyms.

On Wed, Aug 11, 2021 at 5:40 PM Hao Luo <haoluo@...gle.com> wrote:
>
> Currently weak typeless ksyms have default value zero, when they don't
> exist in the kernel. However, weak typed ksyms are rejected by libbpf
> if they can not be resolved. This means that if a bpf object contains
> the declaration of a nonexistent weak typed ksym, it will be rejected
> even if there is no program that references the symbol.
>
> Nonexistent weak typed ksyms can also default to zero just like
> typeless ones. This allows programs that access weak typed ksyms to be
> accepted by verifier, if the accesses are guarded. For example,
>
> extern const int bpf_link_fops3 __ksym __weak;
>
> /* then in BPF program */
>
> if (&bpf_link_fops3) {
>    /* use bpf_link_fops3 */
> }
>
> If actual use of nonexistent typed ksym is not guarded properly,
> verifier would see that register is not PTR_TO_BTF_ID and wouldn't
> allow to use it for direct memory reads or passing it to BPF helpers.
>
> Signed-off-by: Hao Luo <haoluo@...gle.com>
> ---

Looks good, applied to bpf-next. For the future, please split libbpf
and selftests changes into separate patches, it's nicer to have those
logically separate.

At some point we should probably also improve libbpf error reporting
for such situations, for better user experience. We have a similar
problem with CO-RE relocation, verifier doesn't know about those
concepts, so verifier log is not very helpful, but libbpf can make
sense out of it with some extra BPF verifier log parsing.

> Changes since v2:
>  - Move special handling and warning from find_ksym_btf_id() to
>    bpf_object__resolve_ksym_var_btf_id().
>  - Removed bpf_link_fops3 from tests since it's not used.
>  - Separated variable declaration and statements.
>
> Changes since v1:
>  - Weak typed symbols default to zero, as suggested by Andrii.
>  - Use ASSERT_XXX() for tests.
>
>  tools/lib/bpf/libbpf.c                        | 16 +++---
>  .../selftests/bpf/prog_tests/ksyms_btf.c      | 31 ++++++++++
>  .../selftests/bpf/progs/test_ksyms_weak.c     | 56 +++++++++++++++++++
>  3 files changed, 96 insertions(+), 7 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/progs/test_ksyms_weak.c
>

[...]