Hi Florent,

I love your patch! Perhaps something to improve:

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

url:    
https://github.com/0day-ci/linux/commits/Florent-Revest/bpf-Add-a-ARG_PTR_TO_CONST_STR-argument-type/20210311-070306
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: i386-randconfig-s001-20210308 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-262-g5e674421-dirty
        # 
https://github.com/0day-ci/linux/commit/cbb95ec99fafe0955aeada270c9be3d1477c3866
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review 
Florent-Revest/bpf-Add-a-ARG_PTR_TO_CONST_STR-argument-type/20210311-070306
        git checkout cbb95ec99fafe0955aeada270c9be3d1477c3866
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <l...@intel.com>


"sparse warnings: (new ones prefixed by >>)"
   kernel/bpf/verifier.c:11728:76: sparse: sparse: subtraction of functions? 
Share your drugs
   kernel/bpf/verifier.c:12136:81: sparse: sparse: subtraction of functions? 
Share your drugs
   kernel/bpf/verifier.c:12140:81: sparse: sparse: subtraction of functions? 
Share your drugs
   kernel/bpf/verifier.c:12144:81: sparse: sparse: subtraction of functions? 
Share your drugs
   kernel/bpf/verifier.c:12148:79: sparse: sparse: subtraction of functions? 
Share your drugs
   kernel/bpf/verifier.c:12152:78: sparse: sparse: subtraction of functions? 
Share your drugs
   kernel/bpf/verifier.c:12156:79: sparse: sparse: subtraction of functions? 
Share your drugs
   kernel/bpf/verifier.c:12160:78: sparse: sparse: subtraction of functions? 
Share your drugs
   kernel/bpf/verifier.c:12204:38: sparse: sparse: subtraction of functions? 
Share your drugs
>> kernel/bpf/verifier.c:4918:36: sparse: sparse: non size-preserving integer 
>> to pointer cast

vim +4918 kernel/bpf/verifier.c

  4695  
  4696  static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
  4697                            struct bpf_call_arg_meta *meta,
  4698                            const struct bpf_func_proto *fn)
  4699  {
  4700          u32 regno = BPF_REG_1 + arg;
  4701          struct bpf_reg_state *regs = cur_regs(env), *reg = &regs[regno];
  4702          enum bpf_arg_type arg_type = fn->arg_type[arg];
  4703          enum bpf_reg_type type = reg->type;
  4704          int err = 0;
  4705  
  4706          if (arg_type == ARG_DONTCARE)
  4707                  return 0;
  4708  
  4709          err = check_reg_arg(env, regno, SRC_OP);
  4710          if (err)
  4711                  return err;
  4712  
  4713          if (arg_type == ARG_ANYTHING) {
  4714                  if (is_pointer_value(env, regno)) {
  4715                          verbose(env, "R%d leaks addr into helper 
function\n",
  4716                                  regno);
  4717                          return -EACCES;
  4718                  }
  4719                  return 0;
  4720          }
  4721  
  4722          if (type_is_pkt_pointer(type) &&
  4723              !may_access_direct_pkt_data(env, meta, BPF_READ)) {
  4724                  verbose(env, "helper access to the packet is not 
allowed\n");
  4725                  return -EACCES;
  4726          }
  4727  
  4728          if (arg_type == ARG_PTR_TO_MAP_VALUE ||
  4729              arg_type == ARG_PTR_TO_UNINIT_MAP_VALUE ||
  4730              arg_type == ARG_PTR_TO_MAP_VALUE_OR_NULL) {
  4731                  err = resolve_map_arg_type(env, meta, &arg_type);
  4732                  if (err)
  4733                          return err;
  4734          }
  4735  
  4736          if (register_is_null(reg) && arg_type_may_be_null(arg_type))
  4737                  /* A NULL register has a SCALAR_VALUE type, so skip
  4738                   * type checking.
  4739                   */
  4740                  goto skip_type_check;
  4741  
  4742          err = check_reg_type(env, regno, arg_type, fn->arg_btf_id[arg]);
  4743          if (err)
  4744                  return err;
  4745  
  4746          if (type == PTR_TO_CTX) {
  4747                  err = check_ctx_reg(env, reg, regno);
  4748                  if (err < 0)
  4749                          return err;
  4750          }
  4751  
  4752  skip_type_check:
  4753          if (reg->ref_obj_id) {
  4754                  if (meta->ref_obj_id) {
  4755                          verbose(env, "verifier internal error: more 
than one arg with ref_obj_id R%d %u %u\n",
  4756                                  regno, reg->ref_obj_id,
  4757                                  meta->ref_obj_id);
  4758                          return -EFAULT;
  4759                  }
  4760                  meta->ref_obj_id = reg->ref_obj_id;
  4761          }
  4762  
  4763          if (arg_type == ARG_CONST_MAP_PTR) {
  4764                  /* bpf_map_xxx(map_ptr) call: remember that map_ptr */
  4765                  meta->map_ptr = reg->map_ptr;
  4766          } else if (arg_type == ARG_PTR_TO_MAP_KEY) {
  4767                  /* bpf_map_xxx(..., map_ptr, ..., key) call:
  4768                   * check that [key, key + map->key_size) are within
  4769                   * stack limits and initialized
  4770                   */
  4771                  if (!meta->map_ptr) {
  4772                          /* in function declaration map_ptr must come 
before
  4773                           * map_key, so that it's verified and known 
before
  4774                           * we have to check map_key here. Otherwise it 
means
  4775                           * that kernel subsystem misconfigured verifier
  4776                           */
  4777                          verbose(env, "invalid map_ptr to access 
map->key\n");
  4778                          return -EACCES;
  4779                  }
  4780                  err = check_helper_mem_access(env, regno,
  4781                                                meta->map_ptr->key_size, 
false,
  4782                                                NULL);
  4783          } else if (arg_type == ARG_PTR_TO_MAP_VALUE ||
  4784                     (arg_type == ARG_PTR_TO_MAP_VALUE_OR_NULL &&
  4785                      !register_is_null(reg)) ||
  4786                     arg_type == ARG_PTR_TO_UNINIT_MAP_VALUE) {
  4787                  /* bpf_map_xxx(..., map_ptr, ..., value) call:
  4788                   * check [value, value + map->value_size) validity
  4789                   */
  4790                  if (!meta->map_ptr) {
  4791                          /* kernel subsystem misconfigured verifier */
  4792                          verbose(env, "invalid map_ptr to access 
map->value\n");
  4793                          return -EACCES;
  4794                  }
  4795                  meta->raw_mode = (arg_type == 
ARG_PTR_TO_UNINIT_MAP_VALUE);
  4796                  err = check_helper_mem_access(env, regno,
  4797                                                
meta->map_ptr->value_size, false,
  4798                                                meta);
  4799          } else if (arg_type == ARG_PTR_TO_PERCPU_BTF_ID) {
  4800                  if (!reg->btf_id) {
  4801                          verbose(env, "Helper has invalid btf_id in 
R%d\n", regno);
  4802                          return -EACCES;
  4803                  }
  4804                  meta->ret_btf = reg->btf;
  4805                  meta->ret_btf_id = reg->btf_id;
  4806          } else if (arg_type == ARG_PTR_TO_SPIN_LOCK) {
  4807                  if (meta->func_id == BPF_FUNC_spin_lock) {
  4808                          if (process_spin_lock(env, regno, true))
  4809                                  return -EACCES;
  4810                  } else if (meta->func_id == BPF_FUNC_spin_unlock) {
  4811                          if (process_spin_lock(env, regno, false))
  4812                                  return -EACCES;
  4813                  } else {
  4814                          verbose(env, "verifier internal error\n");
  4815                          return -EFAULT;
  4816                  }
  4817          } else if (arg_type == ARG_PTR_TO_FUNC) {
  4818                  meta->subprogno = reg->subprogno;
  4819          } else if (arg_type_is_mem_ptr(arg_type)) {
  4820                  /* The access to this pointer is only checked when we 
hit the
  4821                   * next is_mem_size argument below.
  4822                   */
  4823                  meta->raw_mode = (arg_type == ARG_PTR_TO_UNINIT_MEM);
  4824          } else if (arg_type_is_mem_size(arg_type)) {
  4825                  bool zero_size_allowed = (arg_type == 
ARG_CONST_SIZE_OR_ZERO);
  4826  
  4827                  /* This is used to refine r0 return value bounds for 
helpers
  4828                   * that enforce this value as an upper bound on return 
values.
  4829                   * See do_refine_retval_range() for helpers that can 
refine
  4830                   * the return value. C type of helper is u32 so we pull 
register
  4831                   * bound from umax_value however, if negative verifier 
errors
  4832                   * out. Only upper bounds can be learned because retval 
is an
  4833                   * int type and negative retvals are allowed.
  4834                   */
  4835                  meta->msize_max_value = reg->umax_value;
  4836  
  4837                  /* The register is SCALAR_VALUE; the access check
  4838                   * happens using its boundaries.
  4839                   */
  4840                  if (!tnum_is_const(reg->var_off))
  4841                          /* For unprivileged variable accesses, disable 
raw
  4842                           * mode so that the program is required to
  4843                           * initialize all the memory that the helper 
could
  4844                           * just partially fill up.
  4845                           */
  4846                          meta = NULL;
  4847  
  4848                  if (reg->smin_value < 0) {
  4849                          verbose(env, "R%d min value is negative, either 
use unsigned or 'var &= const'\n",
  4850                                  regno);
  4851                          return -EACCES;
  4852                  }
  4853  
  4854                  if (reg->umin_value == 0) {
  4855                          err = check_helper_mem_access(env, regno - 1, 0,
  4856                                                        zero_size_allowed,
  4857                                                        meta);
  4858                          if (err)
  4859                                  return err;
  4860                  }
  4861  
  4862                  if (reg->umax_value >= BPF_MAX_VAR_SIZ) {
  4863                          verbose(env, "R%d unbounded memory access, use 
'var &= const' or 'if (var < const)'\n",
  4864                                  regno);
  4865                          return -EACCES;
  4866                  }
  4867                  err = check_helper_mem_access(env, regno - 1,
  4868                                                reg->umax_value,
  4869                                                zero_size_allowed, meta);
  4870                  if (!err)
  4871                          err = mark_chain_precision(env, regno);
  4872          } else if (arg_type_is_alloc_size(arg_type)) {
  4873                  if (!tnum_is_const(reg->var_off)) {
  4874                          verbose(env, "R%d is not a known constant'\n",
  4875                                  regno);
  4876                          return -EACCES;
  4877                  }
  4878                  meta->mem_size = reg->var_off.value;
  4879          } else if (arg_type_is_int_ptr(arg_type)) {
  4880                  int size = int_ptr_type_to_size(arg_type);
  4881  
  4882                  err = check_helper_mem_access(env, regno, size, false, 
meta);
  4883                  if (err)
  4884                          return err;
  4885                  err = check_ptr_alignment(env, reg, 0, size, true);
  4886          } else if (arg_type == ARG_PTR_TO_CONST_STR) {
  4887                  struct bpf_map *map = reg->map_ptr;
  4888                  int map_off, i;
  4889                  u64 map_addr;
  4890                  char *map_ptr;
  4891  
  4892                  if (!map || !bpf_map_is_rdonly(map)) {
  4893                          verbose(env, "R%d does not point to a readonly 
map'\n", regno);
  4894                          return -EACCES;
  4895                  }
  4896  
  4897                  if (!tnum_is_const(reg->var_off)) {
  4898                          verbose(env, "R%d is not a constant 
address'\n", regno);
  4899                          return -EACCES;
  4900                  }
  4901  
  4902                  if (!map->ops->map_direct_value_addr) {
  4903                          verbose(env, "no direct value access support 
for this map type\n");
  4904                          return -EACCES;
  4905                  }
  4906  
  4907                  err = check_helper_mem_access(env, regno,
  4908                                                map->value_size - 
reg->off,
  4909                                                false, meta);
  4910                  if (err)
  4911                          return err;
  4912  
  4913                  map_off = reg->off + reg->var_off.value;
  4914                  err = map->ops->map_direct_value_addr(map, &map_addr, 
map_off);
  4915                  if (err)
  4916                          return err;
  4917  
> 4918                  map_ptr = (char *)(map_addr);
  4919                  for (i = map_off; map_ptr[i] != '\0'; i++) {
  4920                          if (i == map->value_size - 1) {
  4921                                  verbose(env, "map does not contain a 
NULL-terminated string\n");
  4922                                  return -EACCES;
  4923                          }
  4924                  }
  4925          }
  4926  
  4927          return err;
  4928  }
  4929  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org

Attachment: .config.gz
Description: application/gzip

Reply via email to