On Wed, Feb 16, 2022 at 1:13 AM Kumar Kartikeya Dwivedi
<memxor@xxxxxxxxx> wrote:
>
> When commit e6ac2450d6de ("bpf: Support bpf program calling kernel
> function") added kfunc support, it defined reg2btf_ids as a cheap way to
> translate the verifier reg type to the appropriate btf_vmlinux BTF ID,
> however commit c25b2ae13603 ("bpf: Replace PTR_TO_XXX_OR_NULL with
> PTR_TO_XXX | PTR_MAYBE_NULL") moved the __BPF_REG_TYPE_MAX from the last
> member of bpf_reg_type enum to after the base register types, and
> defined other variants using type flag composition. However, now, the
> direct usage of reg->type to index into reg2btf_ids may no longer fall
> into __BPF_REG_TYPE_MAX range, and hence lead to out of bounds access
> and kernel crash on dereference of bad pointer.
...
> [ 20.488393] RDX: 0000000000000000 RSI: 0000000000000004 RDI: 0000000000524156
> [ 20.489045] RBP: ffffffffa398c6d4 R08: ffffffffa14dd991 R09: 0000000000000000
> [ 20.489696] R10: fffffbfff484f31c R11: 0000000000000001 R12: ffff888007bf8600
> [ 20.490377] R13: ffff88800c2f6078 R14: 0000000000000000 R15: 0000000000000001
> [ 20.491065] FS: 00007fe06ae70740(0000) GS:ffff88808cc00000(0000) knlGS:0000000000000000
> [ 20.491782] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 20.492272] CR2: 0000000000524156 CR3: 000000000902a004 CR4: 0000000000770ef0
> [ 20.492925] PKRU: 55555554
Please do not include a full kernel dump in the commit log.
It provides no value.
The first paragraph was enough.
> Cc: Martin KaFai Lau <kafai@xxxxxx>
> Cc: Hao Luo <haoluo@xxxxxxxxxx>
> Fixes: c25b2ae13603 ("bpf: Replace PTR_TO_XXX_OR_NULL with PTR_TO_XXX | PTR_MAYBE_NULL")
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
> ---
> kernel/bpf/btf.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index e16dafeb2450..416345798e0a 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -5568,13 +5568,21 @@ int btf_check_type_match(struct bpf_verifier_log *log, const struct bpf_prog *pr
> return btf_check_func_type_match(log, btf1, t1, btf2, t2);
> }
>
> -static u32 *reg2btf_ids[__BPF_REG_TYPE_MAX] = {
> +static u32 *reg2btf_ids(enum bpf_reg_type type)
> +{
> + switch (type) {
> #ifdef CONFIG_NET
> - [PTR_TO_SOCKET] = &btf_sock_ids[BTF_SOCK_TYPE_SOCK],
> - [PTR_TO_SOCK_COMMON] = &btf_sock_ids[BTF_SOCK_TYPE_SOCK_COMMON],
> - [PTR_TO_TCP_SOCK] = &btf_sock_ids[BTF_SOCK_TYPE_TCP],
> + case PTR_TO_SOCKET:
> + return &btf_sock_ids[BTF_SOCK_TYPE_SOCK];
> + case PTR_TO_SOCK_COMMON:
> + return &btf_sock_ids[BTF_SOCK_TYPE_SOCK_COMMON];
> + case PTR_TO_TCP_SOCK:
> + return &btf_sock_ids[BTF_SOCK_TYPE_TCP];
> #endif
> -};
> + default:
> + return NULL;
> + }
> +}
>
> /* Returns true if struct is composed of scalars, 4 levels of nesting allowed */
> static bool __btf_type_is_scalar_struct(struct bpf_verifier_log *log,
> @@ -5688,7 +5696,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
> }
> if (check_ptr_off_reg(env, reg, regno))
> return -EINVAL;
> - } else if (is_kfunc && (reg->type == PTR_TO_BTF_ID || reg2btf_ids[reg->type])) {
> + } else if (is_kfunc && (reg->type == PTR_TO_BTF_ID || reg2btf_ids(reg->type))) {
Just use reg2btf_ids[base_type(reg->type)] instead?
