On Mon, Oct 19, 2020 at 12:43 PM Martin KaFai Lau <kafai@xxxxxx> 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@xxxxxx>
> Cc: Hao Luo <haoluo@xxxxxxxxxx>
> Signed-off-by: Martin KaFai Lau <kafai@xxxxxx>
Good catch. The fix makes sense to me. Applied.
