[bpf-next,v6,3/6] bpf: Allow helpers to accept pointers with a fixed size

Commit Message

Maxim Mikityanskiy April 22, 2022, 5:24 p.m. UTC

Before this commit, the BPF verifier required ARG_PTR_TO_MEM arguments
to be followed by ARG_CONST_SIZE holding the size of the memory region.
The helpers had to check that size in runtime.

There are cases where the size expected by a helper is a compile-time
constant. Checking it in runtime is an unnecessary overhead and waste of
BPF registers.

This commit allows helpers to accept ARG_PTR_TO_MEM arguments without
the corresponding ARG_CONST_SIZE, given that they define the memory
region size in struct bpf_func_proto.

Signed-off-by: Maxim Mikityanskiy <[e-mail]>
Reviewed-by: Tariq Toukan <[e-mail]>
---
 include/linux/bpf.h   | 10 ++++++++++
 kernel/bpf/verifier.c | 26 +++++++++++++++-----------
 2 files changed, 25 insertions(+), 11 deletions(-)

Comments

kernel test robot April 23, 2022, 8:38 a.m. UTC | #1

Hi Maxim,

Thank you for the patch! Perhaps something to improve:

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

url:    https://github.com/intel-lab-lkp/linux/commits/Maxim-Mikityanskiy/bpf-Use-ipv6_only_sock-in-bpf_tcp_gen_syncookie/20220423-022511
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: arc-randconfig-r036-20220422 (https://download.01.org/0day-ci/archive/20220423/[e-mail]/config)
compiler: arc-elf-gcc (GCC) 11.2.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/e374c2f19a674b586212d155ecb708aaa86dcd2c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Maxim-Mikityanskiy/bpf-Use-ipv6_only_sock-in-bpf_tcp_gen_syncookie/20220423-022511
        git checkout e374c2f19a674b586212d155ecb708aaa86dcd2c
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash kernel/bpf/

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

All warnings (new ones prefixed by >>):

   kernel/bpf/verifier.c: In function 'check_helper_call':
>> kernel/bpf/verifier.c:5954:49: warning: array subscript 5 is above array bounds of 'const enum bpf_arg_type[5]' [-Warray-bounds]
    5954 |         return arg_type_is_mem_size(fn->arg_type[arg + 1]) || fn->arg_size[arg];
         |                                     ~~~~~~~~~~~~^~~~~~~~~
   In file included from include/linux/bpf-cgroup.h:5,
                    from kernel/bpf/verifier.c:7:
   include/linux/bpf.h:456:35: note: while referencing 'arg_type'
     456 |                 enum bpf_arg_type arg_type[5];
         |                                   ^~~~~~~~
   kernel/bpf/verifier.c:5952:57: warning: array subscript 5 is above array bounds of 'const enum bpf_arg_type[5]' [-Warray-bounds]
    5952 |                 return arg_type_is_mem_size(fn->arg_type[arg + 1]) ==
         |                                             ~~~~~~~~~~~~^~~~~~~~~
   In file included from include/linux/bpf-cgroup.h:5,
                    from kernel/bpf/verifier.c:7:
   include/linux/bpf.h:456:35: note: while referencing 'arg_type'
     456 |                 enum bpf_arg_type arg_type[5];
         |                                   ^~~~~~~~


vim +5954 kernel/bpf/verifier.c

  5948	
  5949	static bool check_args_pair_invalid(const struct bpf_func_proto *fn, int arg)
  5950	{
  5951		if (arg_type_is_mem_ptr(fn->arg_type[arg]))
  5952			return arg_type_is_mem_size(fn->arg_type[arg + 1]) ==
  5953				!!fn->arg_size[arg];
> 5954		return arg_type_is_mem_size(fn->arg_type[arg + 1]) || fn->arg_size[arg];
  5955	}
  5956

Patch

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 7bf441563ffc..914b571bbf3a 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -465,6 +465,16 @@  struct bpf_func_proto {
 		};
 		u32 *arg_btf_id[5];
 	};
+	union {
+		struct {
+			size_t arg1_size;
+			size_t arg2_size;
+			size_t arg3_size;
+			size_t arg4_size;
+			size_t arg5_size;
+		};
+		size_t arg_size[5];
+	};
 	int *ret_btf_id; /* return value btf_id */
 	bool (*allowed)(const struct bpf_prog *prog);
 };
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 71827d14724a..368fab3dfca5 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5602,6 +5602,11 @@  static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 		 * next is_mem_size argument below.
 		 */
 		meta->raw_mode = (arg_type == ARG_PTR_TO_UNINIT_MEM);
+		if (fn->arg_size[arg]) {
+			err = check_helper_mem_access(env, regno,
+						      fn->arg_size[arg], false,
+						      meta);
+		}
 	} else if (arg_type_is_mem_size(arg_type)) {
 		bool zero_size_allowed = (arg_type == ARG_CONST_SIZE_OR_ZERO);
 
@@ -5941,13 +5946,12 @@  static bool check_raw_mode_ok(const struct bpf_func_proto *fn)
 	return count <= 1;
 }
 
-static bool check_args_pair_invalid(enum bpf_arg_type arg_curr,
-				    enum bpf_arg_type arg_next)
+static bool check_args_pair_invalid(const struct bpf_func_proto *fn, int arg)
 {
-	return (arg_type_is_mem_ptr(arg_curr) &&
-	        !arg_type_is_mem_size(arg_next)) ||
-	       (!arg_type_is_mem_ptr(arg_curr) &&
-		arg_type_is_mem_size(arg_next));
+	if (arg_type_is_mem_ptr(fn->arg_type[arg]))
+		return arg_type_is_mem_size(fn->arg_type[arg + 1]) ==
+			!!fn->arg_size[arg];
+	return arg_type_is_mem_size(fn->arg_type[arg + 1]) || fn->arg_size[arg];
 }
 
 static bool check_arg_pair_ok(const struct bpf_func_proto *fn)
@@ -5958,11 +5962,11 @@  static bool check_arg_pair_ok(const struct bpf_func_proto *fn)
 	 * helper function specification.
 	 */
 	if (arg_type_is_mem_size(fn->arg1_type) ||
-	    arg_type_is_mem_ptr(fn->arg5_type)  ||
-	    check_args_pair_invalid(fn->arg1_type, fn->arg2_type) ||
-	    check_args_pair_invalid(fn->arg2_type, fn->arg3_type) ||
-	    check_args_pair_invalid(fn->arg3_type, fn->arg4_type) ||
-	    check_args_pair_invalid(fn->arg4_type, fn->arg5_type))
+	    (arg_type_is_mem_ptr(fn->arg5_type) && !fn->arg5_size) ||
+	    check_args_pair_invalid(fn, 1) ||
+	    check_args_pair_invalid(fn, 2) ||
+	    check_args_pair_invalid(fn, 3) ||
+	    check_args_pair_invalid(fn, 4))
 		return false;
 
 	return true;