[PATCH bpf-next v2 1/2] bpf: Warn with new bpf_unreachable() kfunc maybe due to uninitialized var

[PATCH bpf-next v2 1/2] bpf: Warn with new bpf_unreachable() kfunc maybe due to uninitialized var

[Yonghong Song]

Marc Suñé (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
“-Wall -Werror” 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.

bpf_unreachable() is also possible in the middle of the prog.
If bpf_unreachable() is hit during normal do_check() verification,
verification will fail.

  [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 <yonghong.song@linux.dev>
---
 kernel/bpf/helpers.c  |  5 +++++
 kernel/bpf/verifier.c | 22 +++++++++++++++++++++-
 2 files changed, 26 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

  -KBUILD_CFLAGS  += -fno-delete-null-pointer-checks
  +KBUILD_CFLAGS  += -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 += $(call cc-disable-warning, frame-address)
   KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
   KBUILD_CFLAGS += -Wmissing-declarations
   KBUILD_CFLAGS += -Wmissing-prototypes
  +KBUILD_CFLAGS += -Wno-default-const-init-var-unsafe

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index c1113b74e1e2..4852c36b1c51 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -3273,6 +3273,10 @@ __bpf_kfunc void bpf_local_irq_restore(unsigned long *flags__irq_flag)
 	local_irq_restore(*flags__irq_flag);
 }
 
+__bpf_kfunc void bpf_unreachable(void)
+{
+}
+
 __bpf_kfunc_end_defs();
 
 BTF_KFUNCS_START(generic_btf_ids)
@@ -3386,6 +3390,7 @@ BTF_ID_FLAGS(func, bpf_copy_from_user_dynptr, KF_SLEEPABLE)
 BTF_ID_FLAGS(func, bpf_copy_from_user_str_dynptr, KF_SLEEPABLE)
 BTF_ID_FLAGS(func, bpf_copy_from_user_task_dynptr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_copy_from_user_task_str_dynptr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_unreachable)
 BTF_KFUNCS_END(common_btf_ids)
 
 static const struct btf_kfunc_id_set common_kfunc_set = {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f6d3655b3a7a..5496775a884e 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);
 
 static bool bpf_map_ptr_poisoned(const struct bpf_insn_aux_data *aux)
 {
@@ -3399,7 +3400,10 @@ static int check_subprogs(struct bpf_verifier_env *env)
 	int i, subprog_start, subprog_end, off, cur_subprog = 0;
 	struct bpf_subprog_info *subprog = env->subprog_info;
 	struct bpf_insn *insn = env->prog->insnsi;
+	bool is_bpf_unreachable = false;
 	int insn_cnt = env->prog->len;
+	const struct btf_type *t;
+	const char *tname;
 
 	/* now check that all jumps are within the same subprog */
 	subprog_start = subprog[cur_subprog].start;
@@ -3434,7 +3438,18 @@ static int check_subprogs(struct bpf_verifier_env *env)
 			if (code != (BPF_JMP | BPF_EXIT) &&
 			    code != (BPF_JMP32 | BPF_JA) &&
 			    code != (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 == (BPF_CALL | BPF_JMP) &&
+				    insn[i].src_reg == BPF_PSEUDO_KFUNC_CALL) {
+					t = btf_type_by_id(btf_vmlinux, insn[i].imm);
+					tname = btf_name_by_offset(btf_vmlinux, t->name_off);
+					if (strcmp(tname, "bpf_unreachable") == 0)
+						is_bpf_unreachable = true;
+				}
+				if (is_bpf_unreachable)
+					verbose(env, "last insn is bpf_unreachable, due to uninitialized var?\n");
+				else
+					verbose(env, "last insn is not an exit or jmp\n");
 				return -EINVAL;
 			}
 			subprog_start = subprog_end;
@@ -12122,6 +12137,7 @@ enum special_kfunc_type {
 	KF_bpf_res_spin_unlock,
 	KF_bpf_res_spin_lock_irqsave,
 	KF_bpf_res_spin_unlock_irqrestore,
+	KF_bpf_unreachable,
 };
 
 BTF_SET_START(special_kfunc_set)
@@ -12225,6 +12241,7 @@ BTF_ID(func, bpf_res_spin_lock)
 BTF_ID(func, bpf_res_spin_unlock)
 BTF_ID(func, bpf_res_spin_lock_irqsave)
 BTF_ID(func, bpf_res_spin_unlock_irqrestore)
+BTF_ID(func, bpf_unreachable)
 
 static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
 {
@@ -13525,6 +13542,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 			return err;
 		}
 		__mark_btf_func_reg_size(env, regs, BPF_REG_0, sizeof(u32));
+	} else if (insn->imm == special_kfunc_list[KF_bpf_unreachable]) {
+		verbose(env, "unexpected hit bpf_unreachable, due to uninit var or incorrect verification?\n");
+		return -EFAULT;
 	}
 
 	if (is_kfunc_destructive(&meta) && !capable(CAP_SYS_BOOT)) {
-- 
2.47.1

[PATCH bpf-next v2 2/2] selftests/bpf: Add a test with bpf_unreachable() kfunc

[Yonghong Song]

The test case is from [1] reported by Marc Suñé. When compiled with [2],
the object code looks like:

  0000000000000000 <repro>:
  ; {
       0:       bf 16 00 00 00 00 00 00 r6 = r1
  ;       bpf_printk("Start");
       1:       18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0x0 ll
                0000000000000008:  R_BPF_64_64  .rodata
       3:       b4 02 00 00 06 00 00 00 w2 = 0x6
       4:       85 00 00 00 06 00 00 00 call 0x6
  ; DEFINE_FUNC_CTX_POINTER(data)
       5:       61 61 4c 00 00 00 00 00 w1 = *(u32 *)(r6 + 0x4c)
  ;       bpf_printk("pre ipv6_hdrlen_offset");
       6:       18 01 00 00 06 00 00 00 00 00 00 00 00 00 00 00 r1 = 0x6 ll
                0000000000000030:  R_BPF_64_64  .rodata
       8:       b4 02 00 00 17 00 00 00 w2 = 0x17
       9:       85 00 00 00 06 00 00 00 call 0x6
      10:       85 10 00 00 ff ff ff ff call -0x1
                0000000000000050:  R_BPF_64_32  bpf_unreachable

You can see the last insn is bpf_unreachable() func and bpf verifier
can take advantage of such information and emits the following error
message:
  (85) call bpf_unreachable#74465
  last insn is bpf_unreachable, due to uninitialized var?

Three other tests are naked functions. They test
  - bpf_unreachable() at the end of the func,
  - bpf_unreachable() in the middle of the func and verification didn't hit it, and
  - bpf_unreachable() in the middle of the func and verification hit it

  [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 <yonghong.song@linux.dev>
---
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../selftests/bpf/progs/verifier_uninit_var.c | 248 ++++++++++++++++++
 2 files changed, 250 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_uninit_var.c

diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index e66a57970d28..bc2765d130c0 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -87,6 +87,7 @@
 #include "verifier_tailcall_jit.skel.h"
 #include "verifier_typedef.skel.h"
 #include "verifier_uninit.skel.h"
+#include "verifier_uninit_var.skel.h"
 #include "verifier_unpriv.skel.h"
 #include "verifier_unpriv_perf.skel.h"
 #include "verifier_value_adj_spill.skel.h"
@@ -220,6 +221,7 @@ void test_verifier_subreg(void)               { RUN(verifier_subreg); }
 void test_verifier_tailcall_jit(void)         { RUN(verifier_tailcall_jit); }
 void test_verifier_typedef(void)              { RUN(verifier_typedef); }
 void test_verifier_uninit(void)               { RUN(verifier_uninit); }
+void test_verifier_uninit_var(void)           { RUN(verifier_uninit_var); }
 void test_verifier_unpriv(void)               { RUN(verifier_unpriv); }
 void test_verifier_unpriv_perf(void)          { RUN(verifier_unpriv_perf); }
 void test_verifier_value_adj_spill(void)      { RUN(verifier_value_adj_spill); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_uninit_var.c b/tools/testing/selftests/bpf/progs/verifier_uninit_var.c
new file mode 100644
index 000000000000..a1a45b46e99e
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_uninit_var.c
@@ -0,0 +1,248 @@
+#include <linux/bpf.h>
+#include <linux/pkt_cls.h>
+#include <linux/if_ether.h>
+#include <linux/in.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <linux/udp.h>
+#include <linux/tcp.h>
+#include <stdbool.h>
+#include <linux/icmpv6.h>
+#include <linux/in.h>
+
+#include <bpf/bpf_endian.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+
+union macaddr {
+	struct {
+		__u32 p1;
+		__u16 p2;
+	};
+	__u8 addr[6];
+};
+
+union v6addr {
+	struct {
+		__u32 p1;
+		__u32 p2;
+		__u32 p3;
+		__u32 p4;
+	};
+	struct {
+		__u64 d1;
+		__u64 d2;
+	};
+	__u8 addr[16];
+};
+
+/* Number of extension headers that can be skipped */
+#define IPV6_MAX_HEADERS 4
+
+#define NEXTHDR_HOP             0       /* Hop-by-hop option header. */
+#define NEXTHDR_TCP             6       /* TCP segment. */
+#define NEXTHDR_UDP             17      /* UDP message. */
+#define NEXTHDR_IPV6            41      /* IPv6 in IPv6 */
+#define NEXTHDR_ROUTING         43      /* Routing header. */
+#define NEXTHDR_FRAGMENT        44      /* Fragmentation/reassembly header. */
+#define NEXTHDR_GRE             47      /* GRE header. */
+#define NEXTHDR_ESP             50      /* Encapsulating security payload. */
+#define NEXTHDR_AUTH            51      /* Authentication header. */
+#define NEXTHDR_ICMP            58      /* ICMP for IPv6. */
+#define NEXTHDR_NONE            59      /* No next header */
+#define NEXTHDR_DEST            60      /* Destination options header. */
+#define NEXTHDR_SCTP            132     /* SCTP message. */
+#define NEXTHDR_MOBILITY        135     /* Mobility header. */
+
+#define NEXTHDR_MAX             255
+
+#define IPV6_SADDR_OFF		offsetof(struct ipv6hdr, saddr)
+#define IPV6_DADDR_OFF		offsetof(struct ipv6hdr, daddr)
+
+#define NEXTHDR_ICMP 58
+#define ICMP6_NS_MSG_TYPE		135
+
+#define DROP_INVALID		-134
+#define DROP_INVALID_EXTHDR	-156
+#define DROP_FRAG_NOSUPPORT	-157
+
+#define ctx_load_bytes		skb_load_bytes
+
+#define DEFINE_FUNC_CTX_POINTER(FIELD)						\
+static __always_inline void *							\
+ctx_ ## FIELD(const struct __sk_buff *ctx)					\
+{										\
+	void *ptr;								\
+										\
+	/* LLVM may generate u32 assignments of ctx->{data,data_end,data_meta}.	\
+	 * With this inline asm, LLVM loses track of the fact this field is on	\
+	 * 32 bits.								\
+	 */									\
+	asm volatile("%0 = *(u32 *)(%1 + %2)"					\
+		     : "=r"(ptr)						\
+		     : "r"(ctx), "i"(offsetof(struct __sk_buff, FIELD)));	\
+	return ptr;								\
+}
+/* This defines ctx_data(). */
+DEFINE_FUNC_CTX_POINTER(data)
+#undef DEFINE_FUNC_CTX_POINTER
+
+
+static __always_inline int ipv6_optlen(const struct ipv6_opt_hdr *opthdr)
+{
+	return (opthdr->hdrlen + 1) << 3;
+}
+
+static __always_inline int ipv6_authlen(const struct ipv6_opt_hdr *opthdr)
+{
+	return (opthdr->hdrlen + 2) << 2;
+}
+
+static __always_inline int ipv6_hdrlen_offset(struct __sk_buff *ctx,
+					      __u8 *nexthdr, int l3_off)
+{
+	int i, len = sizeof(struct ipv6hdr);
+	struct ipv6_opt_hdr opthdr __attribute__((aligned(8)));
+	__u8 nh = *nexthdr;
+
+#pragma unroll
+	for (i = 0; i < IPV6_MAX_HEADERS; i++) {
+		switch (nh) {
+		case NEXTHDR_NONE:
+			return DROP_INVALID_EXTHDR;
+
+		case NEXTHDR_FRAGMENT:
+			return DROP_FRAG_NOSUPPORT;
+
+		case NEXTHDR_HOP:
+		case NEXTHDR_ROUTING:
+		case NEXTHDR_AUTH:
+		case NEXTHDR_DEST:
+			if (bpf_skb_load_bytes(ctx, l3_off + len, &opthdr,
+					   sizeof(opthdr)) < 0)
+				return DROP_INVALID;
+
+			if (nh == NEXTHDR_AUTH)
+				len += ipv6_authlen(&opthdr);
+			else
+				len += ipv6_optlen(&opthdr);
+
+			nh = opthdr.nexthdr;
+			break;
+
+		default:
+			bpf_printk("OKOK %d, len: %d", *nexthdr, len);
+			*nexthdr = nh;
+			return len;
+		}
+	}
+
+	bpf_printk("KO INVALID EXTHDR");
+
+	/* Reached limit of supported extension headers */
+	return DROP_INVALID_EXTHDR;
+}
+static __always_inline
+bool icmp6_ndisc_validate(struct __sk_buff *ctx, struct ipv6hdr *ip6,
+			  union macaddr *mac, union macaddr *smac,
+			  union v6addr *sip, union v6addr *tip)
+{
+	__u8 nexthdr;
+	struct icmp6hdr *icmp;
+	int l3_off, l4_off;
+
+	l3_off = (__u8 *)ip6 - (__u8 *)ctx_data(ctx);
+	bpf_printk("pre ipv6_hdrlen_offset");
+	l4_off = ipv6_hdrlen_offset(ctx, &nexthdr, l3_off);
+	bpf_printk("post ipv6_hdrlen_offset");
+
+	if (l4_off < 0 || nexthdr != NEXTHDR_ICMP) {
+		bpf_printk("KO 1");
+		return false;
+	}
+
+	icmp = (struct icmp6hdr *)((__u8 *)ctx_data(ctx) + l4_off);
+	if (icmp->icmp6_type != ICMP6_NS_MSG_TYPE) {
+		bpf_printk("KO 2");
+		return false;
+	}
+
+	/* Extract fields */
+#if 0
+	eth_load_saddr(ctx, &smac->addr[0], 0);
+	eth_load_daddr(ctx, &mac->addr[0], 0);
+	ipv6_load_saddr(ctx, l3_off, sip);
+	ipv6_load_daddr(ctx, l3_off, tip);
+#endif
+	bpf_printk("ACK ");
+
+	return true;
+}
+
+SEC("classifier")
+__description("agressive optimization due to uninitialized variable")
+#if __clang_major__ >= 21
+__failure __msg("last insn is bpf_unreachable, due to uninitialized var")
+#else
+__failure __msg("last insn is not an exit or jmp")
+#endif
+int classifier_uninit_var(struct __sk_buff *skb)
+{
+	struct ipv6hdr *ip6 = NULL;
+	union macaddr mac, smac;
+	union v6addr sip, tip;
+
+	bpf_printk("Start");
+	icmp6_ndisc_validate(skb, ip6, &mac, &smac, &sip, &tip);
+	bpf_printk("End");
+
+	return 0;
+}
+
+extern void bpf_unreachable(void) __weak __ksym;
+
+SEC("socket")
+__description("bpf_unreachable as the last func insn")
+__failure __msg("last insn is bpf_unreachable, due to uninitialized var")
+__naked void bpf_unreachable_at_func_end(void)
+{
+	asm volatile (
+	"r0 = 0;"
+	"call %[bpf_unreachable];"
+	:
+	: __imm(bpf_unreachable)
+	: __clobber_all);
+}
+
+SEC("socket")
+__description("dead code bpf_unreachable in the middle of code")
+__success
+__naked void dead_bpf_unreachable_in_middle(void)
+{
+	asm volatile (
+	"r0 = 0;"
+	"if r0 == 0 goto +1;"
+	"call %[bpf_unreachable];"
+	"exit;"
+	:
+	: __imm(bpf_unreachable)
+	: __clobber_all);
+}
+
+SEC("socket")
+__description("reachable bpf_unreachable in the middle of code")
+__failure __msg("unexpected hit bpf_unreachable, due to uninit var or incorrect verification?")
+__naked void live_bpf_unreachable_in_middle(void)
+{
+	asm volatile (
+	"r0 = 0;"
+	"if r0 == 1 goto +1;"
+	"call %[bpf_unreachable];"
+	"exit;"
+	:
+	: __imm(bpf_unreachable)
+	: __clobber_all);
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.47.1

Re: [PATCH bpf-next v2 1/2] bpf: Warn with new bpf_unreachable() kfunc maybe due to uninitialized var

[Alexei Starovoitov]

On Thu, May 15, 2025 at 1:06 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
> Marc Suñé (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
> “-Wall -Werror” 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.
>
> bpf_unreachable() is also possible in the middle of the prog.
> If bpf_unreachable() is hit during normal do_check() verification,
> verification will fail.
>
>   [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 <yonghong.song@linux.dev>

What's the difference between v1 and v2 ?
Pls spell it out in a cover letter or commit log.

> ---
>  kernel/bpf/helpers.c  |  5 +++++
>  kernel/bpf/verifier.c | 22 +++++++++++++++++++++-
>  2 files changed, 26 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
>
>   -KBUILD_CFLAGS  += -fno-delete-null-pointer-checks
>   +KBUILD_CFLAGS  += -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 += $(call cc-disable-warning, frame-address)
>    KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
>    KBUILD_CFLAGS += -Wmissing-declarations
>    KBUILD_CFLAGS += -Wmissing-prototypes
>   +KBUILD_CFLAGS += -Wno-default-const-init-var-unsafe
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index c1113b74e1e2..4852c36b1c51 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -3273,6 +3273,10 @@ __bpf_kfunc void bpf_local_irq_restore(unsigned long *flags__irq_flag)
>         local_irq_restore(*flags__irq_flag);
>  }
>
> +__bpf_kfunc void bpf_unreachable(void)
> +{
> +}
> +
>  __bpf_kfunc_end_defs();
>
>  BTF_KFUNCS_START(generic_btf_ids)
> @@ -3386,6 +3390,7 @@ BTF_ID_FLAGS(func, bpf_copy_from_user_dynptr, KF_SLEEPABLE)
>  BTF_ID_FLAGS(func, bpf_copy_from_user_str_dynptr, KF_SLEEPABLE)
>  BTF_ID_FLAGS(func, bpf_copy_from_user_task_dynptr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
>  BTF_ID_FLAGS(func, bpf_copy_from_user_task_str_dynptr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
> +BTF_ID_FLAGS(func, bpf_unreachable)
>  BTF_KFUNCS_END(common_btf_ids)
>
>  static const struct btf_kfunc_id_set common_kfunc_set = {
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index f6d3655b3a7a..5496775a884e 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);
>
>  static bool bpf_map_ptr_poisoned(const struct bpf_insn_aux_data *aux)
>  {
> @@ -3399,7 +3400,10 @@ static int check_subprogs(struct bpf_verifier_env *env)
>         int i, subprog_start, subprog_end, off, cur_subprog = 0;
>         struct bpf_subprog_info *subprog = env->subprog_info;
>         struct bpf_insn *insn = env->prog->insnsi;
> +       bool is_bpf_unreachable = false;
>         int insn_cnt = env->prog->len;
> +       const struct btf_type *t;
> +       const char *tname;
>
>         /* now check that all jumps are within the same subprog */
>         subprog_start = subprog[cur_subprog].start;
> @@ -3434,7 +3438,18 @@ static int check_subprogs(struct bpf_verifier_env *env)
>                         if (code != (BPF_JMP | BPF_EXIT) &&
>                             code != (BPF_JMP32 | BPF_JA) &&
>                             code != (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 == (BPF_CALL | BPF_JMP) &&
> +                                   insn[i].src_reg == BPF_PSEUDO_KFUNC_CALL) {

hmm. there is bpf_pseudo_kfunc_call() for that.

> +                                       t = btf_type_by_id(btf_vmlinux, insn[i].imm);
> +                                       tname = btf_name_by_offset(btf_vmlinux, t->name_off);
> +                                       if (strcmp(tname, "bpf_unreachable") == 0)

same issue as in v1. don't do strcmp.
Especially, since the 2nd hunk of this patch is doing it
via special_kfunc_list[].

> +                                               is_bpf_unreachable = true;

why extra bool ?
Just print the error and return.

> +                               }
> +                               if (is_bpf_unreachable)
> +                                       verbose(env, "last insn is bpf_unreachable, due to uninitialized var?\n");

bpf_unreachable()

..variable.

> +                               else
> +                                       verbose(env, "last insn is not an exit or jmp\n");
>                                 return -EINVAL;
>                         }
>                         subprog_start = subprog_end;
> @@ -12122,6 +12137,7 @@ enum special_kfunc_type {
>         KF_bpf_res_spin_unlock,
>         KF_bpf_res_spin_lock_irqsave,
>         KF_bpf_res_spin_unlock_irqrestore,
> +       KF_bpf_unreachable,
>  };
>
>  BTF_SET_START(special_kfunc_set)
> @@ -12225,6 +12241,7 @@ BTF_ID(func, bpf_res_spin_lock)
>  BTF_ID(func, bpf_res_spin_unlock)
>  BTF_ID(func, bpf_res_spin_lock_irqsave)
>  BTF_ID(func, bpf_res_spin_unlock_irqrestore)
> +BTF_ID(func, bpf_unreachable)
>
>  static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
>  {
> @@ -13525,6 +13542,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>                         return err;
>                 }
>                 __mark_btf_func_reg_size(env, regs, BPF_REG_0, sizeof(u32));
> +       } else if (insn->imm == special_kfunc_list[KF_bpf_unreachable]) {
> +               verbose(env, "unexpected hit bpf_unreachable, due to uninit var or incorrect verification?\n");

!insn->off must be checked as well.
The wording of the message is odd.
s/unexpected hit bpf_unreachable/unexpected bpf_unreachable()/

and I'd finish with "due to uninitialized variable?"
Humans will read it. Don't abbreviate.

"incorrect verification" part is weird. It won't convey
any useful information to users.

pw-bot: cr

> +               return -EFAULT;
>         }
>
>         if (is_kfunc_destructive(&meta) && !capable(CAP_SYS_BOOT)) {
> --
> 2.47.1
>

Re: [PATCH bpf-next v2 1/2] bpf: Warn with new bpf_unreachable() kfunc maybe due to uninitialized var

[Yonghong Song]

On 5/15/25 6:42 AM, Alexei Starovoitov wrote:
> On Thu, May 15, 2025 at 1:06 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>> Marc Suñé (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
>> “-Wall -Werror” 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.
>>
>> bpf_unreachable() is also possible in the middle of the prog.
>> If bpf_unreachable() is hit during normal do_check() verification,
>> verification will fail.
>>
>>    [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 <yonghong.song@linux.dev>
> What's the difference between v1 and v2 ?
> Pls spell it out in a cover letter or commit log.

Sorry, will have a commit log for the next version including
v1 -> v2 and v2 -> v3, etc.

>
>> ---
>>   kernel/bpf/helpers.c  |  5 +++++
>>   kernel/bpf/verifier.c | 22 +++++++++++++++++++++-
>>   2 files changed, 26 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
>>
>>    -KBUILD_CFLAGS  += -fno-delete-null-pointer-checks
>>    +KBUILD_CFLAGS  += -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 += $(call cc-disable-warning, frame-address)
>>     KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
>>     KBUILD_CFLAGS += -Wmissing-declarations
>>     KBUILD_CFLAGS += -Wmissing-prototypes
>>    +KBUILD_CFLAGS += -Wno-default-const-init-var-unsafe
>>
>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>> index c1113b74e1e2..4852c36b1c51 100644
>> --- a/kernel/bpf/helpers.c
>> +++ b/kernel/bpf/helpers.c
>> @@ -3273,6 +3273,10 @@ __bpf_kfunc void bpf_local_irq_restore(unsigned long *flags__irq_flag)
>>          local_irq_restore(*flags__irq_flag);
>>   }
>>
>> +__bpf_kfunc void bpf_unreachable(void)
>> +{
>> +}
>> +
>>   __bpf_kfunc_end_defs();
>>
>>   BTF_KFUNCS_START(generic_btf_ids)
>> @@ -3386,6 +3390,7 @@ BTF_ID_FLAGS(func, bpf_copy_from_user_dynptr, KF_SLEEPABLE)
>>   BTF_ID_FLAGS(func, bpf_copy_from_user_str_dynptr, KF_SLEEPABLE)
>>   BTF_ID_FLAGS(func, bpf_copy_from_user_task_dynptr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
>>   BTF_ID_FLAGS(func, bpf_copy_from_user_task_str_dynptr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
>> +BTF_ID_FLAGS(func, bpf_unreachable)
>>   BTF_KFUNCS_END(common_btf_ids)
>>
>>   static const struct btf_kfunc_id_set common_kfunc_set = {
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index f6d3655b3a7a..5496775a884e 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);
>>
>>   static bool bpf_map_ptr_poisoned(const struct bpf_insn_aux_data *aux)
>>   {
>> @@ -3399,7 +3400,10 @@ static int check_subprogs(struct bpf_verifier_env *env)
>>          int i, subprog_start, subprog_end, off, cur_subprog = 0;
>>          struct bpf_subprog_info *subprog = env->subprog_info;
>>          struct bpf_insn *insn = env->prog->insnsi;
>> +       bool is_bpf_unreachable = false;
>>          int insn_cnt = env->prog->len;
>> +       const struct btf_type *t;
>> +       const char *tname;
>>
>>          /* now check that all jumps are within the same subprog */
>>          subprog_start = subprog[cur_subprog].start;
>> @@ -3434,7 +3438,18 @@ static int check_subprogs(struct bpf_verifier_env *env)
>>                          if (code != (BPF_JMP | BPF_EXIT) &&
>>                              code != (BPF_JMP32 | BPF_JA) &&
>>                              code != (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 == (BPF_CALL | BPF_JMP) &&
>> +                                   insn[i].src_reg == BPF_PSEUDO_KFUNC_CALL) {
> hmm. there is bpf_pseudo_kfunc_call() for that.
>
>> +                                       t = btf_type_by_id(btf_vmlinux, insn[i].imm);
>> +                                       tname = btf_name_by_offset(btf_vmlinux, t->name_off);
>> +                                       if (strcmp(tname, "bpf_unreachable") == 0)
> same issue as in v1. don't do strcmp.
> Especially, since the 2nd hunk of this patch is doing it
> via special_kfunc_list[].

Okay, without strcmp, we essentially will avoid "last insn is not an exit or jmp\n" for
all kfunc's if we want to later special_kfunc_list[] based verification for
bpf_unreachable(). But even if we have this avoidance in check_subprogs(),
in check_cfg()/push_insn, we will hit the following error:

         if (w < 0 || w >= env->prog->len) {
                 verbose_linfo(env, t, "%d: ", t);
                 verbose(env, "jump out of range from insn %d to %d\n", t, w);
                 return -EINVAL;
         }

So I then decided to add an 'exit' insn after bpf_unreachable() in llvm.
See latest https://github.com/llvm/llvm-project/pull/131731 (commit #2).
So we won't have any control flow issues in code. With newer llvm change,
the kernel change will look like:

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index c1113b74e1e2..4852c36b1c51 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -3273,6 +3273,10 @@ __bpf_kfunc void bpf_local_irq_restore(unsigned long *flags__irq_flag)
         local_irq_restore(*flags__irq_flag);
  }
  
+__bpf_kfunc void bpf_unreachable(void)
+{
+}
+
  __bpf_kfunc_end_defs();
  
  BTF_KFUNCS_START(generic_btf_ids)
@@ -3386,6 +3390,7 @@ BTF_ID_FLAGS(func, bpf_copy_from_user_dynptr, KF_SLEEPABLE)
  BTF_ID_FLAGS(func, bpf_copy_from_user_str_dynptr, KF_SLEEPABLE)
  BTF_ID_FLAGS(func, bpf_copy_from_user_task_dynptr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
  BTF_ID_FLAGS(func, bpf_copy_from_user_task_str_dynptr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_unreachable)
  BTF_KFUNCS_END(common_btf_ids)
  
  static const struct btf_kfunc_id_set common_kfunc_set = {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f6d3655b3a7a..4890adc18478 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12122,6 +12122,7 @@ enum special_kfunc_type {
         KF_bpf_res_spin_unlock,
         KF_bpf_res_spin_lock_irqsave,
         KF_bpf_res_spin_unlock_irqrestore,
+       KF_bpf_unreachable,
  };
  
  BTF_SET_START(special_kfunc_set)
@@ -12225,6 +12226,7 @@ BTF_ID(func, bpf_res_spin_lock)
  BTF_ID(func, bpf_res_spin_unlock)
  BTF_ID(func, bpf_res_spin_lock_irqsave)
  BTF_ID(func, bpf_res_spin_unlock_irqrestore)
+BTF_ID(func, bpf_unreachable)
  
  static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
  {
@@ -13525,6 +13527,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
                         return err;
                 }
                 __mark_btf_func_reg_size(env, regs, BPF_REG_0, sizeof(u32));
+       } else if (!insn->off && insn->imm == special_kfunc_list[KF_bpf_unreachable]) {
+               verbose(env, "unexpected bpf_unreachable() due to uninitialized variable?\n");
+               return -EFAULT;
         }
  
         if (is_kfunc_destructive(&meta) && !capable(CAP_SYS_BOOT)) {

>
>> +                                               is_bpf_unreachable = true;
> why extra bool ?
> Just print the error and return.
>
>> +                               }
>> +                               if (is_bpf_unreachable)
>> +                                       verbose(env, "last insn is bpf_unreachable, due to uninitialized var?\n");
> bpf_unreachable()
>
> ..variable.
>
>> +                               else
>> +                                       verbose(env, "last insn is not an exit or jmp\n");
>>                                  return -EINVAL;
>>                          }
>>                          subprog_start = subprog_end;
>> @@ -12122,6 +12137,7 @@ enum special_kfunc_type {
>>          KF_bpf_res_spin_unlock,
>>          KF_bpf_res_spin_lock_irqsave,
>>          KF_bpf_res_spin_unlock_irqrestore,
>> +       KF_bpf_unreachable,
>>   };
>>
>>   BTF_SET_START(special_kfunc_set)
>> @@ -12225,6 +12241,7 @@ BTF_ID(func, bpf_res_spin_lock)
>>   BTF_ID(func, bpf_res_spin_unlock)
>>   BTF_ID(func, bpf_res_spin_lock_irqsave)
>>   BTF_ID(func, bpf_res_spin_unlock_irqrestore)
>> +BTF_ID(func, bpf_unreachable)
>>
>>   static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
>>   {
>> @@ -13525,6 +13542,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>                          return err;
>>                  }
>>                  __mark_btf_func_reg_size(env, regs, BPF_REG_0, sizeof(u32));
>> +       } else if (insn->imm == special_kfunc_list[KF_bpf_unreachable]) {
>> +               verbose(env, "unexpected hit bpf_unreachable, due to uninit var or incorrect verification?\n");
> !insn->off must be checked as well.

Will add.

> The wording of the message is odd.
> s/unexpected hit bpf_unreachable/unexpected bpf_unreachable()/
>
> and I'd finish with "due to uninitialized variable?"
> Humans will read it. Don't abbreviate.
>
> "incorrect verification" part is weird. It won't convey
> any useful information to users.

Okay, the new verbose message looks like:
   verbose(env, "unexpected bpf_unreachable() due to uninitialized variable?\n");

>
> pw-bot: cr
>
>> +               return -EFAULT;
>>          }
>>
>>          if (is_kfunc_destructive(&meta) && !capable(CAP_SYS_BOOT)) {
>> --
>> 2.47.1
>>

Re: [PATCH bpf-next v2 1/2] bpf: Warn with new bpf_unreachable() kfunc maybe due to uninitialized var

[Alexei Starovoitov]

On Fri, May 16, 2025 at 2:17 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
> So I then decided to add an 'exit' insn after bpf_unreachable() in llvm.
> See latest https://github.com/llvm/llvm-project/pull/131731 (commit #2).
> So we won't have any control flow issues in code. With newer llvm change,

That's a good idea. Certainly better than special case this 'noreturn'
semantic in the verifier.

Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add a test with bpf_unreachable() kfunc

[kernel test robot]

Hi Yonghong,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Yonghong-Song/selftests-bpf-Add-a-test-with-bpf_unreachable-kfunc/20250516-040928
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20250515200640.3428248-1-yonghong.song%40linux.dev
patch subject: [PATCH bpf-next v2 2/2] selftests/bpf: Add a test with bpf_unreachable() kfunc
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505171650.f5nomWW4-lkp@intel.com/

includecheck warnings: (new ones prefixed by >>)
>> tools/testing/selftests/bpf/progs/verifier_uninit_var.c: linux/in.h is included more than once.

vim +4 tools/testing/selftests/bpf/progs/verifier_uninit_var.c

   > 4	#include <linux/in.h>
     5	#include <linux/ip.h>
     6	#include <linux/ipv6.h>
     7	#include <linux/udp.h>
     8	#include <linux/tcp.h>
     9	#include <stdbool.h>
    10	#include <linux/icmpv6.h>
  > 11	#include <linux/in.h>
    12	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH bpf-next v2 1/2] bpf: Warn with new bpf_unreachable() kfunc maybe due to uninitialized var

[Yonghong Song]

On 5/16/25 5:31 AM, Alexei Starovoitov wrote:
> On Fri, May 16, 2025 at 2:17 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>>
>> So I then decided to add an 'exit' insn after bpf_unreachable() in llvm.
>> See latest https://github.com/llvm/llvm-project/pull/131731 (commit #2).
>> So we won't have any control flow issues in code. With newer llvm change,
> That's a good idea. Certainly better than special case this 'noreturn'
> semantic in the verifier.

Current latest llvm21 will cause kernel build failure:
   https://patchew.org/linux/20250506-default-const-init-clang-v2-1-fcfb69703264@kernel.org/
I will wait for the fix in bpf-next and then submit v3.

Re: [PATCH bpf-next v2 1/2] bpf: Warn with new bpf_unreachable() kfunc maybe due to uninitialized var

[Alexei Starovoitov]

On Sat, May 17, 2025 at 11:14 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
>
> On 5/16/25 5:31 AM, Alexei Starovoitov wrote:
> > On Fri, May 16, 2025 at 2:17 PM Yonghong Song <yonghong.song@linux.dev> wrote:
> >>
> >> So I then decided to add an 'exit' insn after bpf_unreachable() in llvm.
> >> See latest https://github.com/llvm/llvm-project/pull/131731 (commit #2).
> >> So we won't have any control flow issues in code. With newer llvm change,
> > That's a good idea. Certainly better than special case this 'noreturn'
> > semantic in the verifier.
>
> Current latest llvm21 will cause kernel build failure:
>    https://patchew.org/linux/20250506-default-const-init-clang-v2-1-fcfb69703264@kernel.org/
> I will wait for the fix in bpf-next and then submit v3.

that's not going to be soon. why delay the whole thing?
submit with selftests that include inline asm and delay C only selftest.

Re: [PATCH bpf-next v2 1/2] bpf: Warn with new bpf_unreachable() kfunc maybe due to uninitialized var

[Yonghong Song]

On 5/18/25 11:41 PM, Alexei Starovoitov wrote:
> On Sat, May 17, 2025 at 11:14 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>>
>>
>> On 5/16/25 5:31 AM, Alexei Starovoitov wrote:
>>> On Fri, May 16, 2025 at 2:17 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>>>> So I then decided to add an 'exit' insn after bpf_unreachable() in llvm.
>>>> See latest https://github.com/llvm/llvm-project/pull/131731 (commit #2).
>>>> So we won't have any control flow issues in code. With newer llvm change,
>>> That's a good idea. Certainly better than special case this 'noreturn'
>>> semantic in the verifier.
>> Current latest llvm21 will cause kernel build failure:
>>     https://patchew.org/linux/20250506-default-const-init-clang-v2-1-fcfb69703264@kernel.org/
>> I will wait for the fix in bpf-next and then submit v3.
> that's not going to be soon. why delay the whole thing?
> submit with selftests that include inline asm and delay C only selftest.

Sure. Will submit with inline asm tests soon.