netdev - Re: [PATCH bpf 1/3] bpf: Enforce id generation for all may-be-null register type

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: <CAADnVQJkpgY1aowy5UhZJyeKwAFQb=5W+4j-G8DJSqMLDM5DkA@mail.gmail.com>
Date:   Mon, 19 Oct 2020 16:23:18 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Martin KaFai Lau <kafai@...com>
Cc:     bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Hao Luo <haoluo@...gle.com>, Kernel Team <kernel-team@...com>,
        Network Development <netdev@...r.kernel.org>,
        Yonghong Song <yhs@...com>
Subject: Re: [PATCH bpf 1/3] bpf: Enforce id generation for all may-be-null
 register type

On Mon, Oct 19, 2020 at 12:43 PM Martin KaFai Lau <kafai@...com> wrote:
>
> The commit af7ec1383361 ("bpf: Add bpf_skc_to_tcp6_sock() helper")
> introduces RET_PTR_TO_BTF_ID_OR_NULL and
> the commit eaa6bcb71ef6 ("bpf: Introduce bpf_per_cpu_ptr()")
> introduces RET_PTR_TO_MEM_OR_BTF_ID_OR_NULL.
> Note that for RET_PTR_TO_MEM_OR_BTF_ID_OR_NULL, the reg0->type
> could become PTR_TO_MEM_OR_NULL which is not covered by
> BPF_PROBE_MEM.
>
> The BPF_REG_0 will then hold a _OR_NULL pointer type. This _OR_NULL
> pointer type requires the bpf program to explicitly do a NULL check first.
> After NULL check, the verifier will mark all registers having
> the same reg->id as safe to use.  However, the reg->id
> is not set for those new _OR_NULL return types.  One of the ways
> that may be wrong is, checking NULL for one btf_id typed pointer will
> end up validating all other btf_id typed pointers because
> all of them have id == 0.  The later tests will exercise
> this path.
>
> To fix it and also avoid similar issue in the future, this patch
> moves the id generation logic out of each individual RET type
> test in check_helper_call().  Instead, it does one
> reg_type_may_be_null() test and then do the id generation
> if needed.
>
> This patch also adds a WARN_ON_ONCE in mark_ptr_or_null_reg()
> to catch future breakage.
>
> The _OR_NULL pointer usage in the bpf_iter_reg.ctx_arg_info is
> fine because it just happens that the existing id generation after
> check_ctx_access() has covered it.  It is also using the
> reg_type_may_be_null() to decide if id generation is needed or not.
>
> Fixes: af7ec1383361 ("bpf: Add bpf_skc_to_tcp6_sock() helper")
> Fixes: eaa6bcb71ef6 ("bpf: Introduce bpf_per_cpu_ptr()")
> Cc: Yonghong Song <yhs@...com>
> Cc: Hao Luo <haoluo@...gle.com>
> Signed-off-by: Martin KaFai Lau <kafai@...com>

Good catch. The fix makes sense to me. Applied.