On Wed, Feb 17, 2021 at 10:18:07AM -0800, Yonghong Song wrote:
> @@ -5893,6 +6004,14 @@ static int retrieve_ptr_limit(const struct bpf_reg_state *ptr_reg,
> else
> *ptr_limit = -off;
> return 0;
> + case PTR_TO_MAP_KEY:
> + if (mask_to_left) {
> + *ptr_limit = ptr_reg->umax_value + ptr_reg->off;
> + } else {
> + off = ptr_reg->smin_value + ptr_reg->off;
> + *ptr_limit = ptr_reg->map_ptr->key_size - off;
> + }
> + return 0;
This part cannot be exercised because for_each will require cap_bpf.
Eventually we might relax this requirement and above code will be necessary.
Could you manually test it that it's working as expected by forcing
sanitize_ptr_alu() to act on it?
> case PTR_TO_MAP_VALUE:
> if (mask_to_left) {
> *ptr_limit = ptr_reg->umax_value + ptr_reg->off;
> @@ -6094,6 +6213,7 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
> verbose(env, "R%d pointer arithmetic on %s prohibited\n",
> dst, reg_type_str[ptr_reg->type]);
> return -EACCES;
> + case PTR_TO_MAP_KEY:
> case PTR_TO_MAP_VALUE:
> if (!env->allow_ptr_leaks && !known && (smin_val < 0) != (smax_val < 0)) {
> verbose(env, "R%d has unknown scalar with mixed signed bounds, pointer arithmetic with it prohibited for !root\n",
> @@ -8273,6 +8393,21 @@ static int check_ld_imm(struct bpf_verifier_env *env, struct bpf_insn *insn)
> return 0;
> }
>
> + if (insn->src_reg == BPF_PSEUDO_FUNC) {
> + struct bpf_prog_aux *aux = env->prog->aux;
> + u32 subprogno = insn[1].imm;
> +
> + if (aux->func_info &&
> + aux->func_info_aux[subprogno].linkage != BTF_FUNC_STATIC) {
Could you change above to "!aux->func_info || aux..." ?
That will force for_each to be available only when funcs are annotated.
The subprogs without annotations were added only to be able to manually
craft asm test cases for subprogs in test_verifier.
The for_each selftests in patches 10 and 11 are strong enough.
The asm test would not add any value.
So I would like to avoid supporting something that has no real use.
