netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: kernel test robot <lkp@intel.com>
To: "Toke Høiland-Jørgensen" <toke@redhat.com>,
	"Alexei Starovoitov" <ast@kernel.org>
Cc: kbuild-all@lists.01.org, clang-built-linux@googlegroups.com,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@chromium.org>,
	netdev@vger.kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next 2/6] bpf: verifier: refactor check_attach_btf_id()
Date: Tue, 14 Jul 2020 10:50:34 +0800	[thread overview]
Message-ID: <202007141016.GfoxL8OD%lkp@intel.com> (raw)
In-Reply-To: <159467114191.370286.3577295271355257627.stgit@toke.dk>

[-- Attachment #1: Type: text/plain, Size: 9488 bytes --]

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 <lkp@intel.com>

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/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 75341 bytes --]

  reply	other threads:[~2020-07-14  3:59 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-13 20:12 [PATCH bpf-next 0/6] bpf: Support multi-attach for freplace programs Toke Høiland-Jørgensen
2020-07-13 20:12 ` [PATCH bpf-next 1/6] bpf: change logging calls from verbose() to bpf_log() and use log pointer Toke Høiland-Jørgensen
2020-07-13 20:12 ` [PATCH bpf-next 2/6] bpf: verifier: refactor check_attach_btf_id() Toke Høiland-Jørgensen
2020-07-14  2:50   ` kernel test robot [this message]
2020-07-13 20:12 ` [PATCH bpf-next 3/6] bpf: support attaching freplace programs to multiple attach points Toke Høiland-Jørgensen
2020-07-13 23:16   ` kernel test robot
2020-07-13 20:12 ` [PATCH bpf-next 4/6] tools: add new members to bpf_attr.raw_tracepoint in bpf.h Toke Høiland-Jørgensen
2020-07-14  5:21   ` BPF logging infrastructure. Was: " Andrii Nakryiko
2020-07-14 12:12     ` Toke Høiland-Jørgensen
2020-07-14 19:14       ` Andrii Nakryiko
2020-07-14 20:47         ` Toke Høiland-Jørgensen
2020-07-14 21:58           ` Andrii Nakryiko
2020-07-14 22:19             ` Toke Høiland-Jørgensen
2020-07-14 23:11               ` Alexei Starovoitov
2020-07-15 12:56                 ` Toke Høiland-Jørgensen
2020-07-15 23:41                   ` Alexei Starovoitov
2020-07-16  1:11                     ` Andrii Nakryiko
2020-07-16  5:44                       ` Alexei Starovoitov
2020-07-16 19:59                         ` Andrii Nakryiko
2020-07-16 20:19                           ` Toke Høiland-Jørgensen
2020-07-17  3:09                           ` Alexei Starovoitov
2020-07-18  3:54                             ` Andrii Nakryiko
2020-07-20 22:30                               ` Alexei Starovoitov
2020-07-21  3:00                                 ` Andrii Nakryiko
2020-07-15 19:02                 ` Andrii Nakryiko
2020-07-13 20:12 ` [PATCH bpf-next 5/6] libbpf: add support for supplying target to bpf_raw_tracepoint_open() Toke Høiland-Jørgensen
2020-07-13 20:12 ` [PATCH bpf-next 6/6] selftests: add test for multiple attachments of freplace program Toke Høiland-Jørgensen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202007141016.GfoxL8OD%lkp@intel.com \
    --to=lkp@intel.com \
    --cc=andriin@fb.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=clang-built-linux@googlegroups.com \
    --cc=daniel@iogearbox.net \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=kbuild-all@lists.01.org \
    --cc=kpsingh@chromium.org \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    --cc=toke@redhat.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).