[PATCH bpf v1 7/8] 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. This permits ID zero for direct
map-value loads because those maps have a single element.

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. Preserve ID zero for
single-element array maps: their callback argument and a pseudo map-value
load are aliases of the same stable allocation. Maps without locks remain
unchanged, while copies of one callback value continue to share an ID and
support balanced locking.

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 | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1c3039f3fc32..b08501734ddf 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -447,6 +447,11 @@ 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_map *map)
+{
+	return map->map_type == BPF_MAP_TYPE_ARRAY && map->max_entries == 1;
+}
+
 static bool type_is_rdonly_mem(u32 type)
 {
 	return type & MEM_RDONLY;
@@ -10035,6 +10040,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].map_ptr))
+		callee->regs[BPF_REG_3].id = ++env->id_gen;
 
 	/* pointer to stack or null */
 	callee->regs[BPF_REG_4] = caller->regs[BPF_REG_3];
@@ -10131,6 +10139,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].map_ptr))
+		callee->regs[BPF_REG_3].id = ++env->id_gen;
 
 	/* unused */
 	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
@@ -10249,6 +10260,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].map_ptr))
+		callee->regs[BPF_REG_3].id = ++env->id_gen;
 
 	/* unused */
 	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
-- 
2.53.0