bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] BPF: make verifier 'misconfigured' errors more meaningful
@ 2023-04-04  9:52 zhongjun
  2023-04-04 11:05 ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: zhongjun @ 2023-04-04  9:52 UTC (permalink / raw)
  To: bpf; +Cc: zhongjun

From: zhongjun <zhongjun@uniontech.com>

There are too many so-called 'misconfigured' errors potentially
feed back to user-space, that make it very hard to judge on
a glance the reason a verification failure occurred.
This patch make those similar error outputs more sensitive and readible.

base-commit: 738a96c4a8c36950803fdd27e7c30aca92dccefd
---
 kernel/bpf/verifier.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d517d13878cf..f19534f919c2 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12684,7 +12684,8 @@ static int check_ld_imm(struct bpf_verifier_env *env, struct bpf_insn *insn)
 			dst_reg->btf_id = aux->btf_var.btf_id;
 			break;
 		default:
-			verbose(env, "bpf verifier is misconfigured\n");
+			verbose(env, "bpf verifier is misconfigured: dst_reg->type = %d\n",
+					dst_reg->type);
 			return -EFAULT;
 		}
 		return 0;
@@ -12722,7 +12723,8 @@ static int check_ld_imm(struct bpf_verifier_env *env, struct bpf_insn *insn)
 		   insn->src_reg == BPF_PSEUDO_MAP_IDX) {
 		dst_reg->type = CONST_PTR_TO_MAP;
 	} else {
-		verbose(env, "bpf verifier is misconfigured\n");
+		verbose(env, "bpf verifier is misconfigured: insn->src_reg = %d\n",
+				(int)insn->src_reg);
 		return -EINVAL;
 	}
 
@@ -12769,7 +12771,7 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
 	}
 
 	if (!env->ops->gen_ld_abs) {
-		verbose(env, "bpf verifier is misconfigured\n");
+		verbose(env, "bpf verifier is misconfigured: gen_ld_abs is NULL\n");
 		return -EINVAL;
 	}
 
@@ -15814,13 +15816,14 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
 
 	if (ops->gen_prologue || env->seen_direct_write) {
 		if (!ops->gen_prologue) {
-			verbose(env, "bpf verifier is misconfigured\n");
+			verbose(env, "bpf verifier is misconfigured: gen_prologue is NULL\n");
 			return -EINVAL;
 		}
 		cnt = ops->gen_prologue(insn_buf, env->seen_direct_write,
 					env->prog);
 		if (cnt >= ARRAY_SIZE(insn_buf)) {
-			verbose(env, "bpf verifier is misconfigured\n");
+			verbose(env, "bpf verifier is misconfigured: cnt=%d exceeds limit@%lu\n",
+					cnt, ARRAY_SIZE(insn_buf));
 			return -EINVAL;
 		} else if (cnt) {
 			new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt);
@@ -15951,7 +15954,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
 					 &target_size);
 		if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf) ||
 		    (ctx_field_size && !target_size)) {
-			verbose(env, "bpf verifier is misconfigured\n");
+			verbose(env, "bpf verifier is misconfigured: ins[%d] cnt=%d ctx_s=%u tg_s=%u\n",
+					i, cnt, ctx_field_size, target_size);
 			return -EINVAL;
 		}
 
@@ -16400,7 +16404,8 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
 		     BPF_MODE(insn->code) == BPF_IND)) {
 			cnt = env->ops->gen_ld_abs(insn, insn_buf);
 			if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf)) {
-				verbose(env, "bpf verifier is misconfigured\n");
+				verbose(env, "bpf verifier is misconfigured: cnt=%d exceeds limit@%lu\n",
+						cnt, ARRAY_SIZE(insn_buf));
 				return -EINVAL;
 			}
 
@@ -16647,7 +16652,8 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
 				if (cnt == -EOPNOTSUPP)
 					goto patch_map_ops_generic;
 				if (cnt <= 0 || cnt >= ARRAY_SIZE(insn_buf)) {
-					verbose(env, "bpf verifier is misconfigured\n");
+					verbose(env, "bpf verifier is misconfigured: cnt=%d exceeds limit@%lu\n",
+							cnt, ARRAY_SIZE(insn_buf));
 					return -EINVAL;
 				}
 
@@ -16848,7 +16854,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
 		if (!map_ptr->ops->map_poke_track ||
 		    !map_ptr->ops->map_poke_untrack ||
 		    !map_ptr->ops->map_poke_run) {
-			verbose(env, "bpf verifier is misconfigured\n");
+			verbose(env, "bpf verifier is misconfigured: map_poke_xxx is NULL\n");
 			return -EINVAL;
 		}
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] BPF: make verifier 'misconfigured' errors more meaningful
  2023-04-04  9:52 [PATCH] BPF: make verifier 'misconfigured' errors more meaningful zhongjun
@ 2023-04-04 11:05 ` Greg KH
  0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2023-04-04 11:05 UTC (permalink / raw)
  To: zhongjun; +Cc: bpf

On Tue, Apr 04, 2023 at 05:52:02PM +0800, zhongjun@uniontech.com wrote:
> From: zhongjun <zhongjun@uniontech.com>
> 
> There are too many so-called 'misconfigured' errors potentially
> feed back to user-space, that make it very hard to judge on
> a glance the reason a verification failure occurred.
> This patch make those similar error outputs more sensitive and readible.
> 
> base-commit: 738a96c4a8c36950803fdd27e7c30aca92dccefd
> ---
>  kernel/bpf/verifier.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch does not have a Signed-off-by: line.  Please read the
  kernel file, Documentation/process/submitting-patches.rst and resend
  it after adding that line.  Note, the line needs to be in the body of
  the email, before the patch, not at the bottom of the patch or in the
  email signature.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] BPF: make verifier 'misconfigured' errors more meaningful
  2023-04-06  1:43 zhongjun
  2023-04-06  1:58 ` Alexei Starovoitov
  2023-04-06  4:30 ` kernel test robot
@ 2023-04-06  5:11 ` kernel test robot
  2 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2023-04-06  5:11 UTC (permalink / raw)
  To: zhongjun, bpf; +Cc: llvm, oe-kbuild-all, zhongjun

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 738a96c4a8c36950803fdd27e7c30aca92dccefd]

url:    https://github.com/intel-lab-lkp/linux/commits/zhongjun-uniontech-com/BPF-make-verifier-misconfigured-errors-more-meaningful/20230406-094605
base:   738a96c4a8c36950803fdd27e7c30aca92dccefd
patch link:    https://lore.kernel.org/r/20230406014351.8984-1-zhongjun%40uniontech.com
patch subject: [PATCH] BPF: make verifier 'misconfigured' errors more meaningful
config: arm-randconfig-r021-20230403 (https://download.01.org/0day-ci/archive/20230406/202304061209.cT4cNzgn-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
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 arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/intel-lab-lkp/linux/commit/e5263a5893bdd6f559e1dbc9e585339a933c7351
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review zhongjun-uniontech-com/BPF-make-verifier-misconfigured-errors-more-meaningful/20230406-094605
        git checkout e5263a5893bdd6f559e1dbc9e585339a933c7351
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash kernel/bpf/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304061209.cT4cNzgn-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> kernel/bpf/verifier.c:15826:11: warning: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Wformat]
                                           cnt, ARRAY_SIZE(insn_buf));
                                                ^~~~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:55:25: note: expanded from macro 'ARRAY_SIZE'
   #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:16408:12: warning: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Wformat]
                                                   cnt, ARRAY_SIZE(insn_buf));
                                                        ^~~~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:55:25: note: expanded from macro 'ARRAY_SIZE'
   #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/verifier.c:16656:13: warning: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Wformat]
                                                           cnt, ARRAY_SIZE(insn_buf));
                                                                ^~~~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:55:25: note: expanded from macro 'ARRAY_SIZE'
   #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   3 warnings generated.


vim +15826 kernel/bpf/verifier.c

 15800	
 15801	/* convert load instructions that access fields of a context type into a
 15802	 * sequence of instructions that access fields of the underlying structure:
 15803	 *     struct __sk_buff    -> struct sk_buff
 15804	 *     struct bpf_sock_ops -> struct sock
 15805	 */
 15806	static int convert_ctx_accesses(struct bpf_verifier_env *env)
 15807	{
 15808		const struct bpf_verifier_ops *ops = env->ops;
 15809		int i, cnt, size, ctx_field_size, delta = 0;
 15810		const int insn_cnt = env->prog->len;
 15811		struct bpf_insn insn_buf[16], *insn;
 15812		u32 target_size, size_default, off;
 15813		struct bpf_prog *new_prog;
 15814		enum bpf_access_type type;
 15815		bool is_narrower_load;
 15816	
 15817		if (ops->gen_prologue || env->seen_direct_write) {
 15818			if (!ops->gen_prologue) {
 15819				verbose(env, "bpf verifier is misconfigured: gen_prologue is NULL\n");
 15820				return -EINVAL;
 15821			}
 15822			cnt = ops->gen_prologue(insn_buf, env->seen_direct_write,
 15823						env->prog);
 15824			if (cnt >= ARRAY_SIZE(insn_buf)) {
 15825				verbose(env, "bpf verifier is misconfigured: cnt=%d exceeds limit@%lu\n",
 15826						cnt, ARRAY_SIZE(insn_buf));
 15827				return -EINVAL;
 15828			} else if (cnt) {
 15829				new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt);
 15830				if (!new_prog)
 15831					return -ENOMEM;
 15832	
 15833				env->prog = new_prog;
 15834				delta += cnt - 1;
 15835			}
 15836		}
 15837	
 15838		if (bpf_prog_is_offloaded(env->prog->aux))
 15839			return 0;
 15840	
 15841		insn = env->prog->insnsi + delta;
 15842	
 15843		for (i = 0; i < insn_cnt; i++, insn++) {
 15844			bpf_convert_ctx_access_t convert_ctx_access;
 15845			bool ctx_access;
 15846	
 15847			if (insn->code == (BPF_LDX | BPF_MEM | BPF_B) ||
 15848			    insn->code == (BPF_LDX | BPF_MEM | BPF_H) ||
 15849			    insn->code == (BPF_LDX | BPF_MEM | BPF_W) ||
 15850			    insn->code == (BPF_LDX | BPF_MEM | BPF_DW)) {
 15851				type = BPF_READ;
 15852				ctx_access = true;
 15853			} else if (insn->code == (BPF_STX | BPF_MEM | BPF_B) ||
 15854				   insn->code == (BPF_STX | BPF_MEM | BPF_H) ||
 15855				   insn->code == (BPF_STX | BPF_MEM | BPF_W) ||
 15856				   insn->code == (BPF_STX | BPF_MEM | BPF_DW) ||
 15857				   insn->code == (BPF_ST | BPF_MEM | BPF_B) ||
 15858				   insn->code == (BPF_ST | BPF_MEM | BPF_H) ||
 15859				   insn->code == (BPF_ST | BPF_MEM | BPF_W) ||
 15860				   insn->code == (BPF_ST | BPF_MEM | BPF_DW)) {
 15861				type = BPF_WRITE;
 15862				ctx_access = BPF_CLASS(insn->code) == BPF_STX;
 15863			} else {
 15864				continue;
 15865			}
 15866	
 15867			if (type == BPF_WRITE &&
 15868			    env->insn_aux_data[i + delta].sanitize_stack_spill) {
 15869				struct bpf_insn patch[] = {
 15870					*insn,
 15871					BPF_ST_NOSPEC(),
 15872				};
 15873	
 15874				cnt = ARRAY_SIZE(patch);
 15875				new_prog = bpf_patch_insn_data(env, i + delta, patch, cnt);
 15876				if (!new_prog)
 15877					return -ENOMEM;
 15878	
 15879				delta    += cnt - 1;
 15880				env->prog = new_prog;
 15881				insn      = new_prog->insnsi + i + delta;
 15882				continue;
 15883			}
 15884	
 15885			if (!ctx_access)
 15886				continue;
 15887	
 15888			switch ((int)env->insn_aux_data[i + delta].ptr_type) {
 15889			case PTR_TO_CTX:
 15890				if (!ops->convert_ctx_access)
 15891					continue;
 15892				convert_ctx_access = ops->convert_ctx_access;
 15893				break;
 15894			case PTR_TO_SOCKET:
 15895			case PTR_TO_SOCK_COMMON:
 15896				convert_ctx_access = bpf_sock_convert_ctx_access;
 15897				break;
 15898			case PTR_TO_TCP_SOCK:
 15899				convert_ctx_access = bpf_tcp_sock_convert_ctx_access;
 15900				break;
 15901			case PTR_TO_XDP_SOCK:
 15902				convert_ctx_access = bpf_xdp_sock_convert_ctx_access;
 15903				break;
 15904			case PTR_TO_BTF_ID:
 15905			case PTR_TO_BTF_ID | PTR_UNTRUSTED:
 15906			/* PTR_TO_BTF_ID | MEM_ALLOC always has a valid lifetime, unlike
 15907			 * PTR_TO_BTF_ID, and an active ref_obj_id, but the same cannot
 15908			 * be said once it is marked PTR_UNTRUSTED, hence we must handle
 15909			 * any faults for loads into such types. BPF_WRITE is disallowed
 15910			 * for this case.
 15911			 */
 15912			case PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED:
 15913				if (type == BPF_READ) {
 15914					insn->code = BPF_LDX | BPF_PROBE_MEM |
 15915						BPF_SIZE((insn)->code);
 15916					env->prog->aux->num_exentries++;
 15917				}
 15918				continue;
 15919			default:
 15920				continue;
 15921			}
 15922	
 15923			ctx_field_size = env->insn_aux_data[i + delta].ctx_field_size;
 15924			size = BPF_LDST_BYTES(insn);
 15925	
 15926			/* If the read access is a narrower load of the field,
 15927			 * convert to a 4/8-byte load, to minimum program type specific
 15928			 * convert_ctx_access changes. If conversion is successful,
 15929			 * we will apply proper mask to the result.
 15930			 */
 15931			is_narrower_load = size < ctx_field_size;
 15932			size_default = bpf_ctx_off_adjust_machine(ctx_field_size);
 15933			off = insn->off;
 15934			if (is_narrower_load) {
 15935				u8 size_code;
 15936	
 15937				if (type == BPF_WRITE) {
 15938					verbose(env, "bpf verifier narrow ctx access misconfigured\n");
 15939					return -EINVAL;
 15940				}
 15941	
 15942				size_code = BPF_H;
 15943				if (ctx_field_size == 4)
 15944					size_code = BPF_W;
 15945				else if (ctx_field_size == 8)
 15946					size_code = BPF_DW;
 15947	
 15948				insn->off = off & ~(size_default - 1);
 15949				insn->code = BPF_LDX | BPF_MEM | size_code;
 15950			}
 15951	
 15952			target_size = 0;
 15953			cnt = convert_ctx_access(type, insn, insn_buf, env->prog,
 15954						 &target_size);
 15955			if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf) ||
 15956			    (ctx_field_size && !target_size)) {
 15957				verbose(env, "bpf verifier is misconfigured: ins[%d] cnt=%d ctx_s=%u tg_s=%u\n",
 15958						i, cnt, ctx_field_size, target_size);
 15959				return -EINVAL;
 15960			}
 15961	
 15962			if (is_narrower_load && size < target_size) {
 15963				u8 shift = bpf_ctx_narrow_access_offset(
 15964					off, size, size_default) * 8;
 15965				if (shift && cnt + 1 >= ARRAY_SIZE(insn_buf)) {
 15966					verbose(env, "bpf verifier narrow ctx load misconfigured\n");
 15967					return -EINVAL;
 15968				}
 15969				if (ctx_field_size <= 4) {
 15970					if (shift)
 15971						insn_buf[cnt++] = BPF_ALU32_IMM(BPF_RSH,
 15972										insn->dst_reg,
 15973										shift);
 15974					insn_buf[cnt++] = BPF_ALU32_IMM(BPF_AND, insn->dst_reg,
 15975									(1 << size * 8) - 1);
 15976				} else {
 15977					if (shift)
 15978						insn_buf[cnt++] = BPF_ALU64_IMM(BPF_RSH,
 15979										insn->dst_reg,
 15980										shift);
 15981					insn_buf[cnt++] = BPF_ALU64_IMM(BPF_AND, insn->dst_reg,
 15982									(1ULL << size * 8) - 1);
 15983				}
 15984			}
 15985	
 15986			new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
 15987			if (!new_prog)
 15988				return -ENOMEM;
 15989	
 15990			delta += cnt - 1;
 15991	
 15992			/* keep walking new program and skip insns we just inserted */
 15993			env->prog = new_prog;
 15994			insn      = new_prog->insnsi + i + delta;
 15995		}
 15996	
 15997		return 0;
 15998	}
 15999	

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] BPF: make verifier 'misconfigured' errors more meaningful
  2023-04-06  1:43 zhongjun
  2023-04-06  1:58 ` Alexei Starovoitov
@ 2023-04-06  4:30 ` kernel test robot
  2023-04-06  5:11 ` kernel test robot
  2 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2023-04-06  4:30 UTC (permalink / raw)
  To: zhongjun, bpf; +Cc: oe-kbuild-all, zhongjun

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 738a96c4a8c36950803fdd27e7c30aca92dccefd]

url:    https://github.com/intel-lab-lkp/linux/commits/zhongjun-uniontech-com/BPF-make-verifier-misconfigured-errors-more-meaningful/20230406-094605
base:   738a96c4a8c36950803fdd27e7c30aca92dccefd
patch link:    https://lore.kernel.org/r/20230406014351.8984-1-zhongjun%40uniontech.com
patch subject: [PATCH] BPF: make verifier 'misconfigured' errors more meaningful
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20230406/202304061244.4wKWT2bk-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 12.1.0
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
        # https://github.com/intel-lab-lkp/linux/commit/e5263a5893bdd6f559e1dbc9e585339a933c7351
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review zhongjun-uniontech-com/BPF-make-verifier-misconfigured-errors-more-meaningful/20230406-094605
        git checkout e5263a5893bdd6f559e1dbc9e585339a933c7351
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh SHELL=/bin/bash kernel/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304061244.4wKWT2bk-lkp@intel.com/

All warnings (new ones prefixed by >>):

   kernel/bpf/verifier.c: In function 'convert_ctx_accesses':
>> kernel/bpf/verifier.c:15825:93: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Wformat=]
   15825 |                         verbose(env, "bpf verifier is misconfigured: cnt=%d exceeds limit@%lu\n",
         |                                                                                           ~~^
         |                                                                                             |
         |                                                                                             long unsigned int
         |                                                                                           %u
   kernel/bpf/verifier.c: In function 'do_misc_fixups':
   kernel/bpf/verifier.c:16407:101: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Wformat=]
   16407 |                                 verbose(env, "bpf verifier is misconfigured: cnt=%d exceeds limit@%lu\n",
         |                                                                                                   ~~^
         |                                                                                                     |
         |                                                                                                     long unsigned int
         |                                                                                                   %u
   kernel/bpf/verifier.c:16655:109: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Wformat=]
   16655 |                                         verbose(env, "bpf verifier is misconfigured: cnt=%d exceeds limit@%lu\n",
         |                                                                                                           ~~^
         |                                                                                                             |
         |                                                                                                             long unsigned int
         |                                                                                                           %u


vim +15825 kernel/bpf/verifier.c

 15800	
 15801	/* convert load instructions that access fields of a context type into a
 15802	 * sequence of instructions that access fields of the underlying structure:
 15803	 *     struct __sk_buff    -> struct sk_buff
 15804	 *     struct bpf_sock_ops -> struct sock
 15805	 */
 15806	static int convert_ctx_accesses(struct bpf_verifier_env *env)
 15807	{
 15808		const struct bpf_verifier_ops *ops = env->ops;
 15809		int i, cnt, size, ctx_field_size, delta = 0;
 15810		const int insn_cnt = env->prog->len;
 15811		struct bpf_insn insn_buf[16], *insn;
 15812		u32 target_size, size_default, off;
 15813		struct bpf_prog *new_prog;
 15814		enum bpf_access_type type;
 15815		bool is_narrower_load;
 15816	
 15817		if (ops->gen_prologue || env->seen_direct_write) {
 15818			if (!ops->gen_prologue) {
 15819				verbose(env, "bpf verifier is misconfigured: gen_prologue is NULL\n");
 15820				return -EINVAL;
 15821			}
 15822			cnt = ops->gen_prologue(insn_buf, env->seen_direct_write,
 15823						env->prog);
 15824			if (cnt >= ARRAY_SIZE(insn_buf)) {
 15825				verbose(env, "bpf verifier is misconfigured: cnt=%d exceeds limit@%lu\n",
 15826						cnt, ARRAY_SIZE(insn_buf));
 15827				return -EINVAL;
 15828			} else if (cnt) {
 15829				new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt);
 15830				if (!new_prog)
 15831					return -ENOMEM;
 15832	
 15833				env->prog = new_prog;
 15834				delta += cnt - 1;
 15835			}
 15836		}
 15837	
 15838		if (bpf_prog_is_offloaded(env->prog->aux))
 15839			return 0;
 15840	
 15841		insn = env->prog->insnsi + delta;
 15842	
 15843		for (i = 0; i < insn_cnt; i++, insn++) {
 15844			bpf_convert_ctx_access_t convert_ctx_access;
 15845			bool ctx_access;
 15846	
 15847			if (insn->code == (BPF_LDX | BPF_MEM | BPF_B) ||
 15848			    insn->code == (BPF_LDX | BPF_MEM | BPF_H) ||
 15849			    insn->code == (BPF_LDX | BPF_MEM | BPF_W) ||
 15850			    insn->code == (BPF_LDX | BPF_MEM | BPF_DW)) {
 15851				type = BPF_READ;
 15852				ctx_access = true;
 15853			} else if (insn->code == (BPF_STX | BPF_MEM | BPF_B) ||
 15854				   insn->code == (BPF_STX | BPF_MEM | BPF_H) ||
 15855				   insn->code == (BPF_STX | BPF_MEM | BPF_W) ||
 15856				   insn->code == (BPF_STX | BPF_MEM | BPF_DW) ||
 15857				   insn->code == (BPF_ST | BPF_MEM | BPF_B) ||
 15858				   insn->code == (BPF_ST | BPF_MEM | BPF_H) ||
 15859				   insn->code == (BPF_ST | BPF_MEM | BPF_W) ||
 15860				   insn->code == (BPF_ST | BPF_MEM | BPF_DW)) {
 15861				type = BPF_WRITE;
 15862				ctx_access = BPF_CLASS(insn->code) == BPF_STX;
 15863			} else {
 15864				continue;
 15865			}
 15866	
 15867			if (type == BPF_WRITE &&
 15868			    env->insn_aux_data[i + delta].sanitize_stack_spill) {
 15869				struct bpf_insn patch[] = {
 15870					*insn,
 15871					BPF_ST_NOSPEC(),
 15872				};
 15873	
 15874				cnt = ARRAY_SIZE(patch);
 15875				new_prog = bpf_patch_insn_data(env, i + delta, patch, cnt);
 15876				if (!new_prog)
 15877					return -ENOMEM;
 15878	
 15879				delta    += cnt - 1;
 15880				env->prog = new_prog;
 15881				insn      = new_prog->insnsi + i + delta;
 15882				continue;
 15883			}
 15884	
 15885			if (!ctx_access)
 15886				continue;
 15887	
 15888			switch ((int)env->insn_aux_data[i + delta].ptr_type) {
 15889			case PTR_TO_CTX:
 15890				if (!ops->convert_ctx_access)
 15891					continue;
 15892				convert_ctx_access = ops->convert_ctx_access;
 15893				break;
 15894			case PTR_TO_SOCKET:
 15895			case PTR_TO_SOCK_COMMON:
 15896				convert_ctx_access = bpf_sock_convert_ctx_access;
 15897				break;
 15898			case PTR_TO_TCP_SOCK:
 15899				convert_ctx_access = bpf_tcp_sock_convert_ctx_access;
 15900				break;
 15901			case PTR_TO_XDP_SOCK:
 15902				convert_ctx_access = bpf_xdp_sock_convert_ctx_access;
 15903				break;
 15904			case PTR_TO_BTF_ID:
 15905			case PTR_TO_BTF_ID | PTR_UNTRUSTED:
 15906			/* PTR_TO_BTF_ID | MEM_ALLOC always has a valid lifetime, unlike
 15907			 * PTR_TO_BTF_ID, and an active ref_obj_id, but the same cannot
 15908			 * be said once it is marked PTR_UNTRUSTED, hence we must handle
 15909			 * any faults for loads into such types. BPF_WRITE is disallowed
 15910			 * for this case.
 15911			 */
 15912			case PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED:
 15913				if (type == BPF_READ) {
 15914					insn->code = BPF_LDX | BPF_PROBE_MEM |
 15915						BPF_SIZE((insn)->code);
 15916					env->prog->aux->num_exentries++;
 15917				}
 15918				continue;
 15919			default:
 15920				continue;
 15921			}
 15922	
 15923			ctx_field_size = env->insn_aux_data[i + delta].ctx_field_size;
 15924			size = BPF_LDST_BYTES(insn);
 15925	
 15926			/* If the read access is a narrower load of the field,
 15927			 * convert to a 4/8-byte load, to minimum program type specific
 15928			 * convert_ctx_access changes. If conversion is successful,
 15929			 * we will apply proper mask to the result.
 15930			 */
 15931			is_narrower_load = size < ctx_field_size;
 15932			size_default = bpf_ctx_off_adjust_machine(ctx_field_size);
 15933			off = insn->off;
 15934			if (is_narrower_load) {
 15935				u8 size_code;
 15936	
 15937				if (type == BPF_WRITE) {
 15938					verbose(env, "bpf verifier narrow ctx access misconfigured\n");
 15939					return -EINVAL;
 15940				}
 15941	
 15942				size_code = BPF_H;
 15943				if (ctx_field_size == 4)
 15944					size_code = BPF_W;
 15945				else if (ctx_field_size == 8)
 15946					size_code = BPF_DW;
 15947	
 15948				insn->off = off & ~(size_default - 1);
 15949				insn->code = BPF_LDX | BPF_MEM | size_code;
 15950			}
 15951	
 15952			target_size = 0;
 15953			cnt = convert_ctx_access(type, insn, insn_buf, env->prog,
 15954						 &target_size);
 15955			if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf) ||
 15956			    (ctx_field_size && !target_size)) {
 15957				verbose(env, "bpf verifier is misconfigured: ins[%d] cnt=%d ctx_s=%u tg_s=%u\n",
 15958						i, cnt, ctx_field_size, target_size);
 15959				return -EINVAL;
 15960			}
 15961	
 15962			if (is_narrower_load && size < target_size) {
 15963				u8 shift = bpf_ctx_narrow_access_offset(
 15964					off, size, size_default) * 8;
 15965				if (shift && cnt + 1 >= ARRAY_SIZE(insn_buf)) {
 15966					verbose(env, "bpf verifier narrow ctx load misconfigured\n");
 15967					return -EINVAL;
 15968				}
 15969				if (ctx_field_size <= 4) {
 15970					if (shift)
 15971						insn_buf[cnt++] = BPF_ALU32_IMM(BPF_RSH,
 15972										insn->dst_reg,
 15973										shift);
 15974					insn_buf[cnt++] = BPF_ALU32_IMM(BPF_AND, insn->dst_reg,
 15975									(1 << size * 8) - 1);
 15976				} else {
 15977					if (shift)
 15978						insn_buf[cnt++] = BPF_ALU64_IMM(BPF_RSH,
 15979										insn->dst_reg,
 15980										shift);
 15981					insn_buf[cnt++] = BPF_ALU64_IMM(BPF_AND, insn->dst_reg,
 15982									(1ULL << size * 8) - 1);
 15983				}
 15984			}
 15985	
 15986			new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
 15987			if (!new_prog)
 15988				return -ENOMEM;
 15989	
 15990			delta += cnt - 1;
 15991	
 15992			/* keep walking new program and skip insns we just inserted */
 15993			env->prog = new_prog;
 15994			insn      = new_prog->insnsi + i + delta;
 15995		}
 15996	
 15997		return 0;
 15998	}
 15999	

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] BPF: make verifier 'misconfigured' errors more meaningful
  2023-04-06  1:43 zhongjun
@ 2023-04-06  1:58 ` Alexei Starovoitov
  2023-04-06  4:30 ` kernel test robot
  2023-04-06  5:11 ` kernel test robot
  2 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2023-04-06  1:58 UTC (permalink / raw)
  To: zhongjun; +Cc: bpf

On Wed, Apr 5, 2023 at 6:53 PM <zhongjun@uniontech.com> wrote:
>
> From: zhongjun <zhongjun@uniontech.com>
>
> There are too many so-called 'misconfigured' errors potentially
> feed back to user-space, that make it very hard to judge on
> a glance the reason a verification failure occurred.
> This patch make those similar error outputs more sensitive and readible.
>
> Signed-off-by: Jun Zhong <zhongjun@uniontech.com>
> base-commit: 738a96c4a8c36950803fdd27e7c30aca92dccefd

This is a Nack. We don't add debug code to the kernel.
If you see these messages you're hacking the verifier incorrectly.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] BPF: make verifier 'misconfigured' errors more meaningful
@ 2023-04-06  1:43 zhongjun
  2023-04-06  1:58 ` Alexei Starovoitov
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: zhongjun @ 2023-04-06  1:43 UTC (permalink / raw)
  To: bpf; +Cc: zhongjun

From: zhongjun <zhongjun@uniontech.com>

There are too many so-called 'misconfigured' errors potentially
feed back to user-space, that make it very hard to judge on
a glance the reason a verification failure occurred.
This patch make those similar error outputs more sensitive and readible.

Signed-off-by: Jun Zhong <zhongjun@uniontech.com>
base-commit: 738a96c4a8c36950803fdd27e7c30aca92dccefd
---
 kernel/bpf/verifier.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d517d13878cf..f19534f919c2 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12684,7 +12684,8 @@ static int check_ld_imm(struct bpf_verifier_env *env, struct bpf_insn *insn)
 			dst_reg->btf_id = aux->btf_var.btf_id;
 			break;
 		default:
-			verbose(env, "bpf verifier is misconfigured\n");
+			verbose(env, "bpf verifier is misconfigured: dst_reg->type = %d\n",
+					dst_reg->type);
 			return -EFAULT;
 		}
 		return 0;
@@ -12722,7 +12723,8 @@ static int check_ld_imm(struct bpf_verifier_env *env, struct bpf_insn *insn)
 		   insn->src_reg == BPF_PSEUDO_MAP_IDX) {
 		dst_reg->type = CONST_PTR_TO_MAP;
 	} else {
-		verbose(env, "bpf verifier is misconfigured\n");
+		verbose(env, "bpf verifier is misconfigured: insn->src_reg = %d\n",
+				(int)insn->src_reg);
 		return -EINVAL;
 	}
 
@@ -12769,7 +12771,7 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
 	}
 
 	if (!env->ops->gen_ld_abs) {
-		verbose(env, "bpf verifier is misconfigured\n");
+		verbose(env, "bpf verifier is misconfigured: gen_ld_abs is NULL\n");
 		return -EINVAL;
 	}
 
@@ -15814,13 +15816,14 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
 
 	if (ops->gen_prologue || env->seen_direct_write) {
 		if (!ops->gen_prologue) {
-			verbose(env, "bpf verifier is misconfigured\n");
+			verbose(env, "bpf verifier is misconfigured: gen_prologue is NULL\n");
 			return -EINVAL;
 		}
 		cnt = ops->gen_prologue(insn_buf, env->seen_direct_write,
 					env->prog);
 		if (cnt >= ARRAY_SIZE(insn_buf)) {
-			verbose(env, "bpf verifier is misconfigured\n");
+			verbose(env, "bpf verifier is misconfigured: cnt=%d exceeds limit@%lu\n",
+					cnt, ARRAY_SIZE(insn_buf));
 			return -EINVAL;
 		} else if (cnt) {
 			new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt);
@@ -15951,7 +15954,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
 					 &target_size);
 		if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf) ||
 		    (ctx_field_size && !target_size)) {
-			verbose(env, "bpf verifier is misconfigured\n");
+			verbose(env, "bpf verifier is misconfigured: ins[%d] cnt=%d ctx_s=%u tg_s=%u\n",
+					i, cnt, ctx_field_size, target_size);
 			return -EINVAL;
 		}
 
@@ -16400,7 +16404,8 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
 		     BPF_MODE(insn->code) == BPF_IND)) {
 			cnt = env->ops->gen_ld_abs(insn, insn_buf);
 			if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf)) {
-				verbose(env, "bpf verifier is misconfigured\n");
+				verbose(env, "bpf verifier is misconfigured: cnt=%d exceeds limit@%lu\n",
+						cnt, ARRAY_SIZE(insn_buf));
 				return -EINVAL;
 			}
 
@@ -16647,7 +16652,8 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
 				if (cnt == -EOPNOTSUPP)
 					goto patch_map_ops_generic;
 				if (cnt <= 0 || cnt >= ARRAY_SIZE(insn_buf)) {
-					verbose(env, "bpf verifier is misconfigured\n");
+					verbose(env, "bpf verifier is misconfigured: cnt=%d exceeds limit@%lu\n",
+							cnt, ARRAY_SIZE(insn_buf));
 					return -EINVAL;
 				}
 
@@ -16848,7 +16854,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
 		if (!map_ptr->ops->map_poke_track ||
 		    !map_ptr->ops->map_poke_untrack ||
 		    !map_ptr->ops->map_poke_run) {
-			verbose(env, "bpf verifier is misconfigured\n");
+			verbose(env, "bpf verifier is misconfigured: map_poke_xxx is NULL\n");
 			return -EINVAL;
 		}
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-04-06  5:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-04  9:52 [PATCH] BPF: make verifier 'misconfigured' errors more meaningful zhongjun
2023-04-04 11:05 ` Greg KH
2023-04-06  1:43 zhongjun
2023-04-06  1:58 ` Alexei Starovoitov
2023-04-06  4:30 ` kernel test robot
2023-04-06  5:11 ` kernel test robot

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).