Skip to content

Commit 932fc2f

Browse files
author
Alexei Starovoitov
committed
Merge branch 'irq-save-restore'
Kumar Kartikeya Dwivedi says: ==================== IRQ save/restore This set introduces support for managing IRQ state from BPF programs. Two new kfuncs, bpf_local_irq_save, and bpf_local_irq_restore are introduced to enable this functionality. Intended use cases are writing IRQ safe data structures (e.g. memory allocator) in BPF programs natively, and use in new spin locking primitives intended to be introduced in the next few weeks. The set begins with some refactoring patches before the actual functionality is introduced. Patch 1 consolidates all resource related state in bpf_verifier_state, and moves it out from bpf_func_state. Patch 2 refactor acquire and release functions for reference state to make them reusable without duplication for other resource types. After this, patch 3 refactors stack slot liveness marking logic to be shared between dynptr, and iterators, in preparation for introducing same logic for irq flag object on stack. Finally, patch 4 and 7 introduce the new kfuncs and their selftests. For more details, please inspect the patch commit logs. Patch 5 makes the error message in case of resource leaks under BPF_EXIT a bit clearer. Patch 6 expands coverage of existing preempt-disable selftest to cover sleepable kfuncs. See individual patches for more details. Changelog: ---------- v5 -> v6 v5: https://lore.kernel.org/bpf/[email protected] * Add Eduard's Acked-by on patch 2 * Remove gen_id parameter to acquire_reference_state (Alexei) * Remove space before REF_TYPE_LOCK (Alexei) * Fix link to v4 in changelog v4 -> v5 v4: https://lore.kernel.org/bpf/[email protected] * Do regno - 1 when printing argument * Pass verifier state explicitly into print_{insn,verifier}_state (Eduard) * Pass frameno instead of bpf_func_state (Eduard) * Move bpf_reference_state *refs after parent to fill two holes in bpf_verifier_state (Eduard). The hunk fixing that bug is in the commit adding IRQ save/restore kfuncs, as it is only needed then. * Fix bug in release_reference_state breaking stack property (Eduard) * Add selftest for triggering and reproducing bug found by Eduard irq_ooo_refs_array in final patch * Print insn_idx and active_irq_id on error (Eduard) * Add more acks v3 -> v4 v3: https://lore.kernel.org/bpf/[email protected] * Add yet another missing kfunc declaration to silence s390 CI v2 -> v3 v2: https://lore.kernel.org/bpf/[email protected] * Drop REF_TYPE_LOCK_MASK * Add kfunc declarations to selftest to silence s390 CI errors v1 -> v2 v1: https://lore.kernel.org/bpf/[email protected] * Drop reference -> resource renaming in the verifier (Eduard, Alexei) * Change verifier log for check_resource_leak for BPF_EXIT (Eduard) * Remove id parameter from acquire_resource_state, read s->id (Eduard) * Rename erase to release for reference state (Eduard) * Move resource state to bpf_verifier_state (Eduard, Alexei) * Drop unnecessary casting to/from u64 in helpers (Eduard) * Add test for arg != PTR_TO_STACK (Eduard) * Drop now redundant tests (Eduard) * Address some other misc nits * Add Reviewed-by and Acked-by from Eduard ==================== Link: https://patch.msgid.link/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
2 parents c721d8f + 4fec4c2 commit 932fc2f

File tree

9 files changed

+949
-168
lines changed

9 files changed

+949
-168
lines changed

include/linux/bpf_verifier.h

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ enum bpf_stack_slot_type {
233233
*/
234234
STACK_DYNPTR,
235235
STACK_ITER,
236+
STACK_IRQ_FLAG,
236237
};
237238

238239
#define BPF_REG_SIZE 8 /* size of eBPF register in bytes */
@@ -254,8 +255,9 @@ struct bpf_reference_state {
254255
* default to pointer reference on zero initialization of a state.
255256
*/
256257
enum ref_state_type {
257-
REF_TYPE_PTR = 0,
258-
REF_TYPE_LOCK,
258+
REF_TYPE_PTR = 1,
259+
REF_TYPE_IRQ = 2,
260+
REF_TYPE_LOCK = 3,
259261
} type;
260262
/* Track each reference created with a unique id, even if the same
261263
* instruction creates the reference multiple times (eg, via CALL).
@@ -315,9 +317,6 @@ struct bpf_func_state {
315317
u32 callback_depth;
316318

317319
/* The following fields should be last. See copy_func_state() */
318-
int acquired_refs;
319-
int active_locks;
320-
struct bpf_reference_state *refs;
321320
/* The state of the stack. Each element of the array describes BPF_REG_SIZE
322321
* (i.e. 8) bytes worth of stack memory.
323322
* stack[0] represents bytes [*(r10-8)..*(r10-1)]
@@ -370,6 +369,8 @@ struct bpf_verifier_state {
370369
/* call stack tracking */
371370
struct bpf_func_state *frame[MAX_CALL_FRAMES];
372371
struct bpf_verifier_state *parent;
372+
/* Acquired reference states */
373+
struct bpf_reference_state *refs;
373374
/*
374375
* 'branches' field is the number of branches left to explore:
375376
* 0 - all possible paths from this state reached bpf_exit or
@@ -419,9 +420,13 @@ struct bpf_verifier_state {
419420
u32 insn_idx;
420421
u32 curframe;
421422

422-
bool speculative;
423+
u32 acquired_refs;
424+
u32 active_locks;
425+
u32 active_preempt_locks;
426+
u32 active_irq_id;
423427
bool active_rcu_lock;
424-
u32 active_preempt_lock;
428+
429+
bool speculative;
425430
/* If this state was ever pointed-to by other state's loop_entry field
426431
* this flag would be set to true. Used to avoid freeing such states
427432
* while they are still in use.
@@ -979,8 +984,9 @@ const char *dynptr_type_str(enum bpf_dynptr_type type);
979984
const char *iter_type_str(const struct btf *btf, u32 btf_id);
980985
const char *iter_state_str(enum bpf_iter_state state);
981986

982-
void print_verifier_state(struct bpf_verifier_env *env,
983-
const struct bpf_func_state *state, bool print_all);
984-
void print_insn_state(struct bpf_verifier_env *env, const struct bpf_func_state *state);
987+
void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_verifier_state *vstate,
988+
u32 frameno, bool print_all);
989+
void print_insn_state(struct bpf_verifier_env *env, const struct bpf_verifier_state *vstate,
990+
u32 frameno);
985991

986992
#endif /* _LINUX_BPF_VERIFIER_H */

kernel/bpf/helpers.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3057,6 +3057,21 @@ __bpf_kfunc int bpf_copy_from_user_str(void *dst, u32 dst__sz, const void __user
30573057
return ret + 1;
30583058
}
30593059

3060+
/* Keep unsinged long in prototype so that kfunc is usable when emitted to
3061+
* vmlinux.h in BPF programs directly, but note that while in BPF prog, the
3062+
* unsigned long always points to 8-byte region on stack, the kernel may only
3063+
* read and write the 4-bytes on 32-bit.
3064+
*/
3065+
__bpf_kfunc void bpf_local_irq_save(unsigned long *flags__irq_flag)
3066+
{
3067+
local_irq_save(*flags__irq_flag);
3068+
}
3069+
3070+
__bpf_kfunc void bpf_local_irq_restore(unsigned long *flags__irq_flag)
3071+
{
3072+
local_irq_restore(*flags__irq_flag);
3073+
}
3074+
30603075
__bpf_kfunc_end_defs();
30613076

30623077
BTF_KFUNCS_START(generic_btf_ids)
@@ -3149,6 +3164,8 @@ BTF_ID_FLAGS(func, bpf_get_kmem_cache)
31493164
BTF_ID_FLAGS(func, bpf_iter_kmem_cache_new, KF_ITER_NEW | KF_SLEEPABLE)
31503165
BTF_ID_FLAGS(func, bpf_iter_kmem_cache_next, KF_ITER_NEXT | KF_RET_NULL | KF_SLEEPABLE)
31513166
BTF_ID_FLAGS(func, bpf_iter_kmem_cache_destroy, KF_ITER_DESTROY | KF_SLEEPABLE)
3167+
BTF_ID_FLAGS(func, bpf_local_irq_save)
3168+
BTF_ID_FLAGS(func, bpf_local_irq_restore)
31523169
BTF_KFUNCS_END(common_btf_ids)
31533170

31543171
static const struct btf_kfunc_id_set common_kfunc_set = {

kernel/bpf/log.c

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,7 @@ static char slot_type_char[] = {
537537
[STACK_ZERO] = '0',
538538
[STACK_DYNPTR] = 'd',
539539
[STACK_ITER] = 'i',
540+
[STACK_IRQ_FLAG] = 'f'
540541
};
541542

542543
static void print_liveness(struct bpf_verifier_env *env,
@@ -753,9 +754,10 @@ static void print_reg_state(struct bpf_verifier_env *env,
753754
verbose(env, ")");
754755
}
755756

756-
void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_func_state *state,
757-
bool print_all)
757+
void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_verifier_state *vstate,
758+
u32 frameno, bool print_all)
758759
{
760+
const struct bpf_func_state *state = vstate->frame[frameno];
759761
const struct bpf_reg_state *reg;
760762
int i;
761763

@@ -843,11 +845,11 @@ void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_func_st
843845
break;
844846
}
845847
}
846-
if (state->acquired_refs && state->refs[0].id) {
847-
verbose(env, " refs=%d", state->refs[0].id);
848-
for (i = 1; i < state->acquired_refs; i++)
849-
if (state->refs[i].id)
850-
verbose(env, ",%d", state->refs[i].id);
848+
if (vstate->acquired_refs && vstate->refs[0].id) {
849+
verbose(env, " refs=%d", vstate->refs[0].id);
850+
for (i = 1; i < vstate->acquired_refs; i++)
851+
if (vstate->refs[i].id)
852+
verbose(env, ",%d", vstate->refs[i].id);
851853
}
852854
if (state->in_callback_fn)
853855
verbose(env, " cb");
@@ -864,7 +866,8 @@ static inline u32 vlog_alignment(u32 pos)
864866
BPF_LOG_MIN_ALIGNMENT) - pos - 1;
865867
}
866868

867-
void print_insn_state(struct bpf_verifier_env *env, const struct bpf_func_state *state)
869+
void print_insn_state(struct bpf_verifier_env *env, const struct bpf_verifier_state *vstate,
870+
u32 frameno)
868871
{
869872
if (env->prev_log_pos && env->prev_log_pos == env->log.end_pos) {
870873
/* remove new line character */
@@ -873,5 +876,5 @@ void print_insn_state(struct bpf_verifier_env *env, const struct bpf_func_state
873876
} else {
874877
verbose(env, "%d:", env->insn_idx);
875878
}
876-
print_verifier_state(env, state, false);
879+
print_verifier_state(env, vstate, frameno, false);
877880
}

0 commit comments

Comments
 (0)