selftests/bpf: Add -Wuninitialized flag to bpf prog flags

Per C99 standard [0], Section 6.7.8, Paragraph 10:

  If an object that has automatic storage duration is not initialized
  explicitly, its value is indeterminate.

And in the same document, in appendix "J.2 Undefined behavior":

  The behavior is undefined in the following circumstances:
  [...]
  The value of an object with automatic storage duration is used while
  it is indeterminate (6.2.4, 6.7.8, 6.8).

This means that use of an uninitialized stack variable is undefined
behavior, and therefore that clang can choose to do a variety of scary
things, such as not generating bytecode for "bunch of useful code" in
the below example:

  void some_func()
  {
    int i;
    if (!i)
      return;
    // bunch of useful code
  }

To add insult to injury, if some_func above is a helper function for
some BPF program, clang can choose to not generate an "exit" insn,
causing verifier to fail with "last insn is not an exit or jmp". Going
from that verification failure to the root cause of uninitialized use
is certain to be frustrating.

This patch adds -Wuninitialized to the cflags for selftest BPF progs and
fixes up existing instances of uninitialized use.

  [0]: https://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf

Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
Cc: David Vernet <void@manifault.com>
Cc: Tejun Heo <tj@kernel.org>
Acked-by: David Vernet <void@manifault.com>
Link: https://lore.kernel.org/r/20230303005500.1614874-1-davemarchevsky@fb.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index f40606a85a0f0ced6122aa42f37753c62e440530..eab3cf5399f52e4a96842cb896213c7849ff9ecc 100644 (file)
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -357,7 +357,7 @@ BPF_CFLAGS = -g -Werror -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN)               \
             -I$(abspath $(OUTPUT)/../usr/include)
 
 CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \
-              -Wno-compare-distinct-pointer-types
+              -Wno-compare-distinct-pointer-types -Wuninitialized
 
 $(OUTPUT)/test_l4lb_noinline.o: BPF_CFLAGS += -fno-inline
 $(OUTPUT)/test_xdp_noinline.o: BPF_CFLAGS += -fno-inline

diff --git a/tools/testing/selftests/bpf/progs/rbtree.c b/tools/testing/selftests/bpf/progs/rbtree.c
index e5db1a4287e50da2b5ba6ddb99f1659222a275a9..4c90aa6abddd95087fb8e6d4b75d99a026ee08fc 100644 (file)
--- a/tools/testing/selftests/bpf/progs/rbtree.c
+++ b/tools/testing/selftests/bpf/progs/rbtree.c
@@ -75,7 +75,7 @@ SEC("tc")
 long rbtree_add_and_remove(void *ctx)
 {
        struct bpf_rb_node *res = NULL;
-       struct node_data *n, *m;
+       struct node_data *n, *m = NULL;
 
        n = bpf_obj_new(typeof(*n));
        if (!n)

diff --git a/tools/testing/selftests/bpf/progs/rbtree_fail.c b/tools/testing/selftests/bpf/progs/rbtree_fail.c
index bf3cba115897efa27fa02cbbdd2ce15a3149e0e3..1ced900f3fced6c33d7bd03bc53845c1fddf98a8 100644 (file)
--- a/tools/testing/selftests/bpf/progs/rbtree_fail.c
+++ b/tools/testing/selftests/bpf/progs/rbtree_fail.c
@@ -232,8 +232,11 @@ long rbtree_api_first_release_unlock_escape(void *ctx)
 
        bpf_spin_lock(&glock);
        res = bpf_rbtree_first(&groot);
-       if (res)
-               n = container_of(res, struct node_data, node);
+       if (!res) {
+               bpf_spin_unlock(&glock);
+               return 1;
+       }
+       n = container_of(res, struct node_data, node);
        bpf_spin_unlock(&glock);
 
        bpf_spin_lock(&glock);

diff --git a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c
index 2fbef3cc7ad841544da1da389d39f435a511950f..2dde8e3fe4c9a2e4bba8a54af0fcf7dfcbd12060 100644 (file)
--- a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c
+++ b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c
@@ -48,7 +48,7 @@ SEC("?lsm.s/bpf")
 __failure __msg("arg#0 expected pointer to stack or dynptr_ptr")
 int BPF_PROG(not_ptr_to_stack, int cmd, union bpf_attr *attr, unsigned int size)
 {
-       unsigned long val;
+       unsigned long val = 0;
 
        return bpf_verify_pkcs7_signature((struct bpf_dynptr *)val,
                                          (struct bpf_dynptr *)val, NULL);

diff --git a/tools/testing/selftests/bpf/progs/test_sk_lookup_kern.c b/tools/testing/selftests/bpf/progs/test_sk_lookup_kern.c
index b502e5c92e3389c32eceb62124695397800a5ada..6ccf6d546074e867ce1ace666da4e6f2f950be49 100644 (file)
--- a/tools/testing/selftests/bpf/progs/test_sk_lookup_kern.c
+++ b/tools/testing/selftests/bpf/progs/test_sk_lookup_kern.c
@@ -23,8 +23,8 @@ static struct bpf_sock_tuple *get_tuple(void *data, __u64 nh_off,
                                        bool *ipv4)
 {
        struct bpf_sock_tuple *result;
+       __u64 ihl_len = 0;
        __u8 proto = 0;
-       __u64 ihl_len;
 
        if (eth_proto == bpf_htons(ETH_P_IP)) {
                struct iphdr *iph = (struct iphdr *)(data + nh_off);

diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
index 508da4a23c4f0ec3cbf06853183f8899317b8ab4..95b4aa0928baa578da680ae304a30d7c75d1e990 100644 (file)
--- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
+++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
@@ -324,11 +324,11 @@ int ip4ip6erspan_get_tunnel(struct __sk_buff *skb)
 SEC("tc")
 int vxlan_set_tunnel_dst(struct __sk_buff *skb)
 {
-       int ret;
        struct bpf_tunnel_key key;
        struct vxlan_metadata md;
        __u32 index = 0;
        __u32 *local_ip = NULL;
+       int ret = 0;
 
        local_ip = bpf_map_lookup_elem(&local_ip_map, &index);
        if (!local_ip) {
@@ -363,11 +363,11 @@ int vxlan_set_tunnel_dst(struct __sk_buff *skb)
 SEC("tc")
 int vxlan_set_tunnel_src(struct __sk_buff *skb)
 {
-       int ret;
        struct bpf_tunnel_key key;
        struct vxlan_metadata md;
        __u32 index = 0;
        __u32 *local_ip = NULL;
+       int ret = 0;
 
        local_ip = bpf_map_lookup_elem(&local_ip_map, &index);
        if (!local_ip) {
@@ -494,9 +494,9 @@ SEC("tc")
 int ip6vxlan_set_tunnel_dst(struct __sk_buff *skb)
 {
        struct bpf_tunnel_key key;
-       int ret;
        __u32 index = 0;
        __u32 *local_ip;
+       int ret = 0;
 
        local_ip = bpf_map_lookup_elem(&local_ip_map, &index);
        if (!local_ip) {
@@ -525,9 +525,9 @@ SEC("tc")
 int ip6vxlan_set_tunnel_src(struct __sk_buff *skb)
 {
        struct bpf_tunnel_key key;
-       int ret;
        __u32 index = 0;
        __u32 *local_ip;
+       int ret = 0;
 
        local_ip = bpf_map_lookup_elem(&local_ip_map, &index);
        if (!local_ip) {
@@ -556,9 +556,9 @@ SEC("tc")
 int ip6vxlan_get_tunnel_src(struct __sk_buff *skb)
 {
        struct bpf_tunnel_key key;
-       int ret;
        __u32 index = 0;
        __u32 *local_ip;
+       int ret = 0;
 
        local_ip = bpf_map_lookup_elem(&local_ip_map, &index);
        if (!local_ip) {