[PATCH bpf v2 6/7] bpf: Assign lock identity to callback map values

[Kumar Kartikeya Dwivedi <memxor@gmail.com>]

The verifier identifies the allocation containing a bpf_spin_lock by the
pair of the map pointer and register ID. It permits ID zero for direct
map-value loads into single-element array maps because they have one stable
value.

Callback frame constructors also leave map-value arguments with ID zero,
but their maps can have multiple elements. Consequently, nested callbacks
can hold two distinct elements of the same map with an identical lock
identity. The verifier then permits a lock acquired through one element to
be released through another. The same confusion lets graph kfuncs operate
on one element while another element is locked, allowing concurrent list
corruption.

Give lockable callback map values a fresh ID in the for-each,
timer/workqueue, and task-work frame constructors. Keep ID zero for
top-level one-element array maps, whose callback argument and pseudo
map-value load alias the same stable allocation. Maps without locks remain
unchanged, while copies of one callback value continue to share an ID and
support balanced locking.

Map-in-map lookups need additional care. Distinct concrete inner maps share
the verifier-visible inner_map_meta, so a one-element inner array otherwise
looks like the same static allocation. Preserve lookup identity as map_uid
for lock-bearing inner maps and consult it before applying the ID-zero
exception. Restricting UID propagation to maps with identity-sensitive
fields avoids making state pruning conservative for every inner map.

Fixes: d0d78c1df9b1 ("bpf: Allow locking bpf_spin_lock global variables")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 9c797cc3df40..ad14a7fbed72 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -448,6 +448,13 @@ static bool reg_may_point_to_spin_lock(const struct bpf_reg_state *reg)
 	return btf_record_has_field(reg_btf_record(reg), BPF_SPIN_LOCK | BPF_RES_SPIN_LOCK);
 }
 
+static bool map_value_has_static_identity(const struct bpf_reg_state *reg)
+{
+	const struct bpf_map *map = reg->map_ptr;
+
+	return !reg->map_uid && map->map_type == BPF_MAP_TYPE_ARRAY && map->max_entries == 1;
+}
+
 static bool type_is_rdonly_mem(u32 type)
 {
 	return type & MEM_RDONLY;
@@ -1926,11 +1933,18 @@ static void refine_map_lookup_value(struct bpf_reg_state *reg)
 	if (map->inner_map_meta) {
 		reg->type = CONST_PTR_TO_MAP | maybe_null;
 		reg->map_ptr = map->inner_map_meta;
-		/* transfer reg's id which is unique for every map_lookup_elem
-		 * as UID of the inner map.
+		/*
+		 * Concrete inner maps share the verifier-visible inner_map_meta.
+		 * Preserve the lookup identity only for embedded objects whose
+		 * verification needs to distinguish concrete map instances. Doing
+		 * this for every inner map makes state pruning too conservative.
+		 *
+		 * Each map lookup has a unique register ID, so use it as the UID of
+		 * the inner map.
 		 */
 		if (btf_record_has_field(map->inner_map_meta->record,
-					 BPF_TIMER | BPF_WORKQUEUE | BPF_TASK_WORK))
+					 BPF_TIMER | BPF_WORKQUEUE | BPF_TASK_WORK |
+					 BPF_SPIN_LOCK | BPF_RES_SPIN_LOCK))
 			reg->map_uid = reg->id;
 	} else if (map->map_type == BPF_MAP_TYPE_XSKMAP) {
 		reg->type = PTR_TO_XDP_SOCK | maybe_null;
@@ -10036,6 +10050,9 @@ int map_set_for_each_callback_args(struct bpf_verifier_env *env,
 	__mark_reg_known_zero(&callee->regs[BPF_REG_3]);
 	callee->regs[BPF_REG_3].map_ptr = caller->regs[BPF_REG_1].map_ptr;
 	callee->regs[BPF_REG_3].map_uid = caller->regs[BPF_REG_1].map_uid;
+	if (reg_may_point_to_spin_lock(&callee->regs[BPF_REG_3]) &&
+	    !map_value_has_static_identity(&callee->regs[BPF_REG_3]))
+		callee->regs[BPF_REG_3].id = ++env->id_gen;
 
 	/* pointer to stack or null */
 	callee->regs[BPF_REG_4] = caller->regs[BPF_REG_3];
@@ -10132,6 +10149,9 @@ static int set_timer_callback_state(struct bpf_verifier_env *env,
 	__mark_reg_known_zero(&callee->regs[BPF_REG_3]);
 	callee->regs[BPF_REG_3].map_ptr = map_ptr;
 	callee->regs[BPF_REG_3].map_uid = map_uid;
+	if (reg_may_point_to_spin_lock(&callee->regs[BPF_REG_3]) &&
+	    !map_value_has_static_identity(&callee->regs[BPF_REG_3]))
+		callee->regs[BPF_REG_3].id = ++env->id_gen;
 
 	/* unused */
 	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
@@ -10250,6 +10270,9 @@ static int set_task_work_schedule_callback_state(struct bpf_verifier_env *env,
 	__mark_reg_known_zero(&callee->regs[BPF_REG_3]);
 	callee->regs[BPF_REG_3].map_ptr = map_ptr;
 	callee->regs[BPF_REG_3].map_uid = map_uid;
+	if (reg_may_point_to_spin_lock(&callee->regs[BPF_REG_3]) &&
+	    !map_value_has_static_identity(&callee->regs[BPF_REG_3]))
+		callee->regs[BPF_REG_3].id = ++env->id_gen;
 
 	/* unused */
 	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
-- 
2.53.0