Thread (29 messages) 29 messages, 4 authors, 2021-02-23

Re: [PATCH bpf-next v2 04/11] bpf: add bpf_for_each_map_elem() helper

From: Alexei Starovoitov <hidden>
Date: 2021-02-22 21:00:02

On Wed, Feb 17, 2021 at 10:18:07AM -0800, Yonghong Song wrote:
quoted hunk ↗ jump to hunk
@@ -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?
quoted hunk ↗ jump to hunk
 	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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help