Subject: Re: [PATCH bpf-next 2/6] bpf: verifier: refactor check_attach_btf_id()

From: kernel test robot <[email-masked]>
Date: Tue, 14 Jul 2020 10:50:34 +0800

Hi "Toke,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on vhost/linux-next ipvs/master v5.8-rc5 next-20200713]
[cannot apply to bpf-next/master bpf/master net/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Toke-H-iland-J-rgensen/bpf-Support-multi-attach-for-freplace-programs/20200714-041410
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 528ae84a34ffd40da5d3fbff740d28d6dc2c8f8a
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 02946de3802d3bc65bc9f2eb9b8d4969b5a7add8)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email-masked]>

All warnings (new ones prefixed by >>):

>> kernel/bpf/verifier.c:10876:7: warning: variable 'addr' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
                   if (ret)
                       ^~~
   kernel/bpf/verifier.c:10923:14: note: uninitialized use occurs here
           *tgt_addr = addr;
                       ^~~~
   kernel/bpf/verifier.c:10876:3: note: remove the 'if' if its condition is always true
                   if (ret)
                   ^~~~~~~~
   kernel/bpf/verifier.c:10861:7: warning: variable 'addr' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
                   if (!btf_type_is_func_proto(t))
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:10923:14: note: uninitialized use occurs here
           *tgt_addr = addr;
                       ^~~~
   kernel/bpf/verifier.c:10861:3: note: remove the 'if' if its condition is always true
                   if (!btf_type_is_func_proto(t))
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:10750:11: note: initialize the variable 'addr' to silence this warning
           long addr;
                    ^
                     = 0
   2 warnings generated.

vim +10876 kernel/bpf/verifier.c

 10733	
 10734	int bpf_check_attach_target(struct bpf_verifier_log *log,
 10735				    const struct bpf_prog *prog,
 10736				    const struct bpf_prog *tgt_prog,
 10737				    u32 btf_id,
 10738				    struct btf_func_model *fmodel,
 10739				    long *tgt_addr,
 10740				    const char **tgt_name,
 10741				    const struct btf_type **tgt_type)
 10742	{
 10743		bool prog_extension = prog->type == BPF_PROG_TYPE_EXT;
 10744		const char prefix[] = "btf_trace_";
 10745		int ret = 0, subprog = -1, i;
 10746		const struct btf_type *t;
 10747		bool conservative = true;
 10748		const char *tname;
 10749		struct btf *btf;
 10750		long addr;
 10751	
 10752		if (!btf_id) {
 10753			bpf_log(log, "Tracing programs must provide btf_id\n");
 10754			return -EINVAL;
 10755		}
 10756		btf = bpf_prog_get_target_btf(prog);
 10757		if (!btf) {
 10758			bpf_log(log,
 10759				"FENTRY/FEXIT program can only be attached to another program annotated with BTF\n");
 10760			return -EINVAL;
 10761		}
 10762		t = btf_type_by_id(btf, btf_id);
 10763		if (!t) {
 10764			bpf_log(log, "attach_btf_id %u is invalid\n", btf_id);
 10765			return -EINVAL;
 10766		}
 10767		tname = btf_name_by_offset(btf, t->name_off);
 10768		if (!tname) {
 10769			bpf_log(log, "attach_btf_id %u doesn't have a name\n", btf_id);
 10770			return -EINVAL;
 10771		}
 10772		if (tgt_prog) {
 10773			struct bpf_prog_aux *aux = tgt_prog->aux;
 10774	
 10775			for (i = 0; i < aux->func_info_cnt; i++)
 10776				if (aux->func_info[i].type_id == btf_id) {
 10777					subprog = i;
 10778					break;
 10779				}
 10780			if (subprog == -1) {
 10781				bpf_log(log, "Subprog %s doesn't exist\n", tname);
 10782				return -EINVAL;
 10783			}
 10784			conservative = aux->func_info_aux[subprog].unreliable;
 10785			if (prog_extension) {
 10786				if (conservative) {
 10787					bpf_log(log,
 10788						"Cannot replace static functions\n");
 10789					return -EINVAL;
 10790				}
 10791				if (!prog->jit_requested) {
 10792					bpf_log(log,
 10793						"Extension programs should be JITed\n");
 10794					return -EINVAL;
 10795				}
 10796			}
 10797			if (!tgt_prog->jited) {
 10798				bpf_log(log, "Can attach to only JITed progs\n");
 10799				return -EINVAL;
 10800			}
 10801			if (tgt_prog->type == prog->type) {
 10802				/* Cannot fentry/fexit another fentry/fexit program.
 10803				 * Cannot attach program extension to another extension.
 10804				 * It's ok to attach fentry/fexit to extension program.
 10805				 */
 10806				bpf_log(log, "Cannot recursively attach\n");
 10807				return -EINVAL;
 10808			}
 10809			if (tgt_prog->type == BPF_PROG_TYPE_TRACING &&
 10810			    prog_extension &&
 10811			    (tgt_prog->expected_attach_type == BPF_TRACE_FENTRY ||
 10812			     tgt_prog->expected_attach_type == BPF_TRACE_FEXIT)) {
 10813				/* Program extensions can extend all program types
 10814				 * except fentry/fexit. The reason is the following.
 10815				 * The fentry/fexit programs are used for performance
 10816				 * analysis, stats and can be attached to any program
 10817				 * type except themselves. When extension program is
 10818				 * replacing XDP function it is necessary to allow
 10819				 * performance analysis of all functions. Both original
 10820				 * XDP program and its program extension. Hence
 10821				 * attaching fentry/fexit to BPF_PROG_TYPE_EXT is
 10822				 * allowed. If extending of fentry/fexit was allowed it
 10823				 * would be possible to create long call chain
 10824				 * fentry->extension->fentry->extension beyond
 10825				 * reasonable stack size. Hence extending fentry is not
 10826				 * allowed.
 10827				 */
 10828				bpf_log(log, "Cannot extend fentry/fexit\n");
 10829				return -EINVAL;
 10830			}
 10831		} else {
 10832			if (prog_extension) {
 10833				bpf_log(log, "Cannot replace kernel functions\n");
 10834				return -EINVAL;
 10835			}
 10836		}
 10837	
 10838		switch (prog->expected_attach_type) {
 10839		case BPF_TRACE_RAW_TP:
 10840			if (tgt_prog) {
 10841				bpf_log(log,
 10842					"Only FENTRY/FEXIT progs are attachable to another BPF prog\n");
 10843				return -EINVAL;
 10844			}
 10845			if (!btf_type_is_typedef(t)) {
 10846				bpf_log(log, "attach_btf_id %u is not a typedef\n",
 10847					btf_id);
 10848				return -EINVAL;
 10849			}
 10850			if (strncmp(prefix, tname, sizeof(prefix) - 1)) {
 10851				bpf_log(log, "attach_btf_id %u points to wrong type name %s\n",
 10852					btf_id, tname);
 10853				return -EINVAL;
 10854			}
 10855			tname += sizeof(prefix) - 1;
 10856			t = btf_type_by_id(btf, t->type);
 10857			if (!btf_type_is_ptr(t))
 10858				/* should never happen in valid vmlinux build */
 10859				return -EINVAL;
 10860			t = btf_type_by_id(btf, t->type);
 10861			if (!btf_type_is_func_proto(t))
 10862				/* should never happen in valid vmlinux build */
 10863				return -EINVAL;
 10864	
 10865			break;
 10866		case BPF_TRACE_ITER:
 10867			if (!btf_type_is_func(t)) {
 10868				bpf_log(log, "attach_btf_id %u is not a function\n",
 10869					btf_id);
 10870				return -EINVAL;
 10871			}
 10872			t = btf_type_by_id(btf, t->type);
 10873			if (!btf_type_is_func_proto(t))
 10874				return -EINVAL;
 10875			ret = btf_distill_func_proto(log, btf, t, tname, fmodel);
 10876			if (ret)
 10877				return ret;
 10878			break;
 10879		default:
 10880			if (!prog_extension)
 10881				return -EINVAL;
 10882			/* fallthrough */
 10883		case BPF_MODIFY_RETURN:
 10884		case BPF_LSM_MAC:
 10885		case BPF_TRACE_FENTRY:
 10886		case BPF_TRACE_FEXIT:
 10887			if (!btf_type_is_func(t)) {
 10888				bpf_log(log, "attach_btf_id %u is not a function\n",
 10889					btf_id);
 10890				return -EINVAL;
 10891			}
 10892			if (prog_extension &&
 10893			    btf_check_type_match(log, prog, btf, t))
 10894				return -EINVAL;
 10895			t = btf_type_by_id(btf, t->type);
 10896			if (!btf_type_is_func_proto(t))
 10897				return -EINVAL;
 10898	
 10899			if (tgt_prog && conservative)
 10900				t = NULL;
 10901	
 10902			ret = btf_distill_func_proto(log, btf, t, tname, fmodel);
 10903			if (ret < 0)
 10904				return ret;
 10905	
 10906			if (tgt_prog) {
 10907				if (subprog == 0)
 10908					addr = (long) tgt_prog->bpf_func;
 10909				else
 10910					addr = (long) tgt_prog->aux->func[subprog]->bpf_func;
 10911			} else {
 10912				addr = kallsyms_lookup_name(tname);
 10913				if (!addr) {
 10914					bpf_log(log,
 10915						"The address of function %s cannot be found\n",
 10916						tname);
 10917					return -ENOENT;
 10918				}
 10919			}
 10920			break;
 10921		}
 10922	
 10923		*tgt_addr = addr;
 10924		if (tgt_name)
 10925			*tgt_name = tname;
 10926		if (tgt_type)
 10927			*tgt_type = t;
 10928		return 0;
 10929	}
 10930	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email-masked]