mirror of
https://github.com/torvalds/linux.git
synced 2026-09-28 20:12:01 +02:00
bpf: Assign lock identity to callback map values
A nested bpf_for_each_map_elem() callback can unlock a different element
of the same map:
static long inner(void *map, int *key, struct value *v,
struct value **outer_value)
{
bpf_spin_lock(&v->lock);
bpf_spin_unlock(&(*outer_value)->lock);
return 0;
}
static long outer(void *map, int *key, struct value *v, void *ctx)
{
bpf_for_each_map_elem(map, inner, &v, 0);
return 0;
}
Both callback values currently have ID zero and the same map_ptr.
process_spin_lock() compares those two fields, so it accepts the unlock
even though the two callbacks can receive different map elements.
Assign a fresh ID to every callback map value in the for-each,
timer/workqueue, and task-work constructors. Copies of one callback
argument retain its ID, so locking and unlocking through that argument
continues to work. Distinct callbacks also get distinct IDs for
single-element arrays, including inner arrays sharing inner_map_meta.
Preserve map_uid for every inner-map lookup and compare it through
check_ids() during state pruning. This preserves relationships between
maps, keys, and values while allowing equivalent states with different
lookup IDs to match. It avoids field-specific rules for when an inner map
needs an identity.
Move map_uid out of the metadata union and next to the other IDs, so
register comparisons can use the existing memcmp() ranges and remap the
IDs separately. Clear it when resetting a register or converting a map
lookup result to a socket pointer. Shrink frameno to u8, which is enough
for MAX_CALL_FRAMES, to make room without growing bpf_reg_state.
Fixes: d0d78c1df9 ("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>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://patch.msgid.link/20260917233222.2542500-9-memxor@gmail.com
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
This commit is contained in:
parent
3440505aca
commit
71919742c8
|
|
@ -46,17 +46,11 @@ struct bpf_reg_state {
|
|||
/* valid when type == PTR_TO_PACKET */
|
||||
int range;
|
||||
|
||||
/* valid when type == CONST_PTR_TO_MAP | PTR_TO_MAP_VALUE |
|
||||
* PTR_TO_MAP_VALUE_OR_NULL
|
||||
/*
|
||||
* For CONST_PTR_TO_MAP, PTR_TO_MAP_KEY, PTR_TO_MAP_VALUE and
|
||||
* PTR_TO_INSN.
|
||||
*/
|
||||
struct {
|
||||
struct bpf_map *map_ptr;
|
||||
/* To distinguish map lookups from outer map
|
||||
* the map_uid is non-zero for registers
|
||||
* pointing to inner maps.
|
||||
*/
|
||||
u32 map_uid;
|
||||
};
|
||||
struct bpf_map *map_ptr;
|
||||
|
||||
/* for PTR_TO_BTF_ID */
|
||||
struct {
|
||||
|
|
@ -155,13 +149,20 @@ struct bpf_reg_state {
|
|||
* gets parent_id set to the dynptr's id.
|
||||
*/
|
||||
u32 parent_id;
|
||||
/* Inside the callee two registers can be both PTR_TO_STACK like
|
||||
/*
|
||||
* Distinguishes inner-map lookups and their keys and values. Zero for
|
||||
* other registers. Kept outside the metadata union for ID remapping
|
||||
* during state comparisons.
|
||||
*/
|
||||
u32 map_uid;
|
||||
/*
|
||||
* Inside the callee two registers can be both PTR_TO_STACK like
|
||||
* R1=fp-8 and R2=fp-8, but one of them points to this function stack
|
||||
* while another to the caller's stack. To differentiate them 'frameno'
|
||||
* is used which is an index in bpf_verifier_state->frame[] array
|
||||
* pointing to bpf_func_state.
|
||||
*/
|
||||
u32 frameno;
|
||||
u8 frameno;
|
||||
/* if (!precise && SCALAR_VALUE) min/max/tnum don't affect safety */
|
||||
bool precise;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -491,7 +491,8 @@ static bool regs_exact(const struct bpf_reg_state *rold,
|
|||
{
|
||||
return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0 &&
|
||||
check_ids(rold->id, rcur->id, idmap) &&
|
||||
check_ids(rold->parent_id, rcur->parent_id, idmap);
|
||||
check_ids(rold->parent_id, rcur->parent_id, idmap) &&
|
||||
check_ids(rold->map_uid, rcur->map_uid, idmap);
|
||||
}
|
||||
|
||||
enum exact_level {
|
||||
|
|
@ -616,7 +617,8 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
|
|||
range_within(rold, rcur) &&
|
||||
tnum_in(rold->var_off, rcur->var_off) &&
|
||||
check_ids(rold->id, rcur->id, idmap) &&
|
||||
check_ids(rold->parent_id, rcur->parent_id, idmap);
|
||||
check_ids(rold->parent_id, rcur->parent_id, idmap) &&
|
||||
check_ids(rold->map_uid, rcur->map_uid, idmap);
|
||||
case PTR_TO_PACKET_META:
|
||||
case PTR_TO_PACKET:
|
||||
/* We must have at least as much range as the old ptr
|
||||
|
|
|
|||
|
|
@ -1864,6 +1864,7 @@ static void __mark_reg_known(struct bpf_reg_state *reg, u64 imm)
|
|||
offsetof(struct bpf_reg_state, var_off) - sizeof(reg->type));
|
||||
reg->id = 0;
|
||||
reg->parent_id = 0;
|
||||
reg->map_uid = 0;
|
||||
___mark_reg_known(reg, imm);
|
||||
}
|
||||
|
||||
|
|
@ -1925,17 +1926,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
|
||||
/*
|
||||
* transfer reg's id which is unique for every map_lookup_elem
|
||||
* as UID of the inner map.
|
||||
*/
|
||||
if (btf_record_has_field(map->inner_map_meta->record,
|
||||
BPF_TIMER | BPF_WORKQUEUE | BPF_TASK_WORK))
|
||||
reg->map_uid = reg->id;
|
||||
reg->map_uid = reg->id;
|
||||
} else if (map->map_type == BPF_MAP_TYPE_XSKMAP) {
|
||||
reg->type = PTR_TO_XDP_SOCK | maybe_null;
|
||||
reg->map_uid = 0;
|
||||
} else if (map->map_type == BPF_MAP_TYPE_SOCKMAP ||
|
||||
map->map_type == BPF_MAP_TYPE_SOCKHASH) {
|
||||
reg->type = PTR_TO_SOCKET | maybe_null;
|
||||
reg->map_uid = 0;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -10048,6 +10050,7 @@ 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;
|
||||
callee->regs[BPF_REG_3].id = ++env->id_gen;
|
||||
|
||||
/* pointer to stack or null */
|
||||
callee->regs[BPF_REG_4] = caller->regs[BPF_REG_3];
|
||||
|
|
@ -10144,6 +10147,7 @@ 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;
|
||||
callee->regs[BPF_REG_3].id = ++env->id_gen;
|
||||
|
||||
/* unused */
|
||||
bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
|
||||
|
|
@ -10262,6 +10266,7 @@ 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;
|
||||
callee->regs[BPF_REG_3].id = ++env->id_gen;
|
||||
|
||||
/* unused */
|
||||
bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user