From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from 66-220-144-179.mail-mxout.facebook.com (66-220-144-179.mail-mxout.facebook.com [66.220.144.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C0AD543169 for ; Sun, 11 May 2025 18:27:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=66.220.144.179 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1746988081; cv=none; b=SHtPyHGuWzWlXM46Qi6NHW2b73nNpGWhg3SOvkP+1hGYsBfW5jWcwwTakf0hy28EwCsGVQs0BQlC9rmM2b/4MKgdNuri/6S/qoeZFZrIoK0QiMMwdyQYrZEAHwdBw5UEWd7RthGnRdrT0gNn3OFiWyBlH7nrEo4PJCpPZEGUc9k= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1746988081; c=relaxed/simple; bh=bEomkCCMpJXvHwazxgoA2bpzuNNpboY38Nj9nDNkx3I=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version:Content-Type; b=fGgS5Av51Qpwym4tZEy41Alz4BQHKKEG2Ol1fx2VRDmP+f0yvLNfUkBK30MGTuoyjqSdYOsWwWTTc/k4Ro/61FvW6+/T/sM2ANWhcez8HZUwEYGZ4l3A6NpQJ33Cg+VMoG9iKdOLk9T+8MJD/5aDlvpxt0dgkrOihyPbzHf83zs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.dev; spf=fail smtp.mailfrom=linux.dev; arc=none smtp.client-ip=66.220.144.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=fail smtp.mailfrom=linux.dev Received: by devvm16039.vll0.facebook.com (Postfix, from userid 128203) id 85B0374F5F0F; Sun, 11 May 2025 11:27:44 -0700 (PDT) From: Yonghong Song To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , kernel-team@fb.com, Martin KaFai Lau Subject: [PATCH bpf-next 1/2] bpf: Warn with new bpf_unreachable() kfunc maybe due to uninitialized var Date: Sun, 11 May 2025 11:27:44 -0700 Message-ID: <20250511182744.1806792-1-yonghong.song@linux.dev> X-Mailer: git-send-email 2.47.1 Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Marc Su=C3=B1=C3=A9 (Isovalent, part of Cisco) reported an issue where an uninitialized variable caused generating bpf prog binary code not working as expected. The reproducer is in [1] where the flags =E2=80=9C-Wall -Werror=E2=80=9D are enabled, but there is no warning and = compiler may take advantage of uninit variable to do aggressive optimization. In llvm internals, uninitialized variable usage may generate 'unreachable' IR insn and these 'unreachable' IR insns may indicate uninit var impact on code optimization. With clang21 patch [2], those 'unreachable' IR insn are converted to func bpf_unreachable(). In kernel, a new kfunc bpf_unreachable() is added. If this kfunc (generated by [2]) is the last insn in the main prog or a subprog, the verifier will suggest the verification failure may be due to uninitialized var, so user can check their source code to find the root cause. Without this patch, the verifier will output last insn is not an exit or jmp and user will not know what is the potential root cause and it will take more time to debug this verification failure. [1] https://github.com/msune/clang_bpf/blob/main/Makefile#L3 [2] https://github.com/llvm/llvm-project/pull/131731 Signed-off-by: Yonghong Song --- kernel/bpf/helpers.c | 5 +++++ kernel/bpf/verifier.c | 17 ++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) In order to compile kernel successfully with the above [2], the following change is needed due to clang21 changes: --- a/Makefile +++ b/Makefile @@ -852,7 +852,7 @@ endif endif # may-sync-config endif # need-config =20 -KBUILD_CFLAGS +=3D -fno-delete-null-pointer-checks +KBUILD_CFLAGS +=3D -fno-delete-null-pointer-checks -Wno-default-const= -init-field-unsafe --- a/scripts/Makefile.extrawarn +++ b/scripts/Makefile.extrawarn @@ -19,6 +19,7 @@ KBUILD_CFLAGS +=3D $(call cc-disable-warning, frame-a= ddress) KBUILD_CFLAGS +=3D $(call cc-disable-warning, address-of-packed-member= ) KBUILD_CFLAGS +=3D -Wmissing-declarations KBUILD_CFLAGS +=3D -Wmissing-prototypes +KBUILD_CFLAGS +=3D -Wno-default-const-init-var-unsafe diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index fed53da75025..6048d7e19d4c 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -3283,6 +3283,10 @@ __bpf_kfunc void bpf_local_irq_restore(unsigned lo= ng *flags__irq_flag) local_irq_restore(*flags__irq_flag); } =20 +__bpf_kfunc void bpf_unreachable(void) +{ +} + __bpf_kfunc_end_defs(); =20 BTF_KFUNCS_START(generic_btf_ids) @@ -3388,6 +3392,7 @@ BTF_ID_FLAGS(func, bpf_iter_kmem_cache_next, KF_ITE= R_NEXT | KF_RET_NULL | KF_SLE BTF_ID_FLAGS(func, bpf_iter_kmem_cache_destroy, KF_ITER_DESTROY | KF_SLE= EPABLE) BTF_ID_FLAGS(func, bpf_local_irq_save) BTF_ID_FLAGS(func, bpf_local_irq_restore) +BTF_ID_FLAGS(func, bpf_unreachable) BTF_KFUNCS_END(common_btf_ids) =20 static const struct btf_kfunc_id_set common_kfunc_set =3D { diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 28f5a7899bd6..d26aec0a90d0 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -206,6 +206,7 @@ static int ref_set_non_owning(struct bpf_verifier_env= *env, static void specialize_kfunc(struct bpf_verifier_env *env, u32 func_id, u16 offset, unsigned long *addr); static bool is_trusted_reg(const struct bpf_reg_state *reg); +static void verbose_insn(struct bpf_verifier_env *env, struct bpf_insn *= insn); =20 static bool bpf_map_ptr_poisoned(const struct bpf_insn_aux_data *aux) { @@ -3398,7 +3399,10 @@ static int check_subprogs(struct bpf_verifier_env = *env) int i, subprog_start, subprog_end, off, cur_subprog =3D 0; struct bpf_subprog_info *subprog =3D env->subprog_info; struct bpf_insn *insn =3D env->prog->insnsi; + bool is_bpf_unreachable =3D false; int insn_cnt =3D env->prog->len; + const struct btf_type *t; + const char *tname; =20 /* now check that all jumps are within the same subprog */ subprog_start =3D subprog[cur_subprog].start; @@ -3433,7 +3437,18 @@ static int check_subprogs(struct bpf_verifier_env = *env) if (code !=3D (BPF_JMP | BPF_EXIT) && code !=3D (BPF_JMP32 | BPF_JA) && code !=3D (BPF_JMP | BPF_JA)) { - verbose(env, "last insn is not an exit or jmp\n"); + verbose_insn(env, &insn[i]); + if (btf_vmlinux && insn[i].code =3D=3D (BPF_CALL | BPF_JMP) && + insn[i].src_reg =3D=3D BPF_PSEUDO_KFUNC_CALL) { + t =3D btf_type_by_id(btf_vmlinux, insn[i].imm); + tname =3D btf_name_by_offset(btf_vmlinux, t->name_off); + if (strcmp(tname, "bpf_unreachable") =3D=3D 0) + is_bpf_unreachable =3D true; + } + if (is_bpf_unreachable) + verbose(env, "last insn is bpf_unreachable, due to uninitialized va= r?\n"); + else + verbose(env, "last insn is not an exit or jmp\n"); return -EINVAL; } subprog_start =3D subprog_end; --=20 2.47.1