Skip to content

Commit f628456

Browse files
author
Alexei Starovoitov
committed
Merge branch 'bpf-add-gen_epilogue-to-bpf_verifier_ops'
Martin KaFai Lau says: ==================== bpf: Add gen_epilogue to bpf_verifier_ops From: Martin KaFai Lau <[email protected]> This set allows the subsystem to patch codes before BPF_EXIT. The verifier ops, .gen_epilogue, is added for this purpose. One of the use case will be in the bpf qdisc, the bpf qdisc subsystem can ensure the skb->dev is in the correct value. The bpf qdisc subsystem can either inline fixing it in the epilogue or call another kernel function to handle it (e.g. drop) in the epilogue. Another use case could be in bpf_tcp_ca.c to enforce snd_cwnd has valid value (e.g. positive value). v5: * Removed the skip_cnt argument from adjust_jmp_off() in patch 2. Instead, reuse the delta argument and skip the [tgt_idx, tgt_idx + delta) instructions. * Added a BPF_JMP32_A macro in patch 3. * Removed pro_epilogue_subprog.c in patch 6. The pro_epilogue_kfunc.c has covered the subprog case. Renamed the file pro_epilogue_kfunc.c to pro_epilogue.c. Some of the SEC names and function names are changed accordingly (mainly shorten them by removing the _kfunc suffix). * Added comments to explain the tail_call result in patch 7. * Fixed the following bpf CI breakages. I ran it in CI manually to confirm: https://github.com/kernel-patches/bpf/actions/runs/10590714532 * s390 zext added "w3 = w3". Adjusted the test to use all ALU64 and BPF_DW to avoid zext. Also changed the "int a" in the "struct st_ops_args" to "u64 a". * llvm17 does not take: *(u64 *)(r1 +0) = 0; so it is changed to: r3 = 0; *(u64 *)(r1 +0) = r3; v4: * Fixed a bug in the memcpy in patch 3 The size in the memcpy should be epilogue_cnt * sizeof(*epilogue_buf) v3: * Moved epilogue_buf[16] to env. Patch 1 is added to move the existing insn_buf[16] to env. * Fixed a case that the bpf prog has a BPF_JMP that goes back to the first instruction of the main prog. The jump back to 1st insn case also applies to the prologue. Patch 2 is added to handle it. * If the bpf main prog has multiple BPF_EXIT, use a BPF_JA to goto the earlier patched epilogue. Note that there are (BPF_JMP32 | BPF_JA) vs (BPF_JMP | BPF_JA) details in the patch 3 commit message. * There are subtle changes in patch 3, so I reset the Reviewed-by. * Added patch 8 and patch 9 to cover the changes in patch 2 and patch 3. * Dropped the kfunc call from pro/epilogue and its selftests. v2: * Remove the RFC tag. Keep the ordering at where .gen_epilogue is called in the verifier relative to the check_max_stack_depth(). This will be consistent with the other extra stack_depth usage like optimize_bpf_loop(). * Use __xlated check provided by the test_loader to check the patched instructions after gen_pro/epilogue (Eduard). * Added Patch 3 by Eduard (Thanks!). ==================== Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
2 parents c6d9daf + cada0bd commit f628456

File tree

14 files changed

+813
-8
lines changed

14 files changed

+813
-8
lines changed

include/linux/bpf.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -974,6 +974,8 @@ struct bpf_verifier_ops {
974974
struct bpf_insn_access_aux *info);
975975
int (*gen_prologue)(struct bpf_insn *insn, bool direct_write,
976976
const struct bpf_prog *prog);
977+
int (*gen_epilogue)(struct bpf_insn *insn, const struct bpf_prog *prog,
978+
s16 ctx_stack_off);
977979
int (*gen_ld_abs)(const struct bpf_insn *orig,
978980
struct bpf_insn *insn_buf);
979981
u32 (*convert_ctx_access)(enum bpf_access_type type,

include/linux/bpf_verifier.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
* (in the "-8,-16,...,-512" form)
2424
*/
2525
#define TMP_STR_BUF_LEN 320
26+
/* Patch buffer size */
27+
#define INSN_BUF_SIZE 16
2628

2729
/* Liveness marks, used for registers and spilled-regs (in stack slots).
2830
* Read marks propagate upwards until they find a write mark; they record that
@@ -780,6 +782,8 @@ struct bpf_verifier_env {
780782
* e.g., in reg_type_str() to generate reg_type string
781783
*/
782784
char tmp_str_buf[TMP_STR_BUF_LEN];
785+
struct bpf_insn insn_buf[INSN_BUF_SIZE];
786+
struct bpf_insn epilogue_buf[INSN_BUF_SIZE];
783787
};
784788

785789
static inline struct bpf_func_info_aux *subprog_aux(struct bpf_verifier_env *env, int subprog)

include/linux/filter.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,16 @@ static inline bool insn_is_cast_user(const struct bpf_insn *insn)
437437
.off = OFF, \
438438
.imm = 0 })
439439

440+
/* Unconditional jumps, gotol pc + imm32 */
441+
442+
#define BPF_JMP32_A(IMM) \
443+
((struct bpf_insn) { \
444+
.code = BPF_JMP32 | BPF_JA, \
445+
.dst_reg = 0, \
446+
.src_reg = 0, \
447+
.off = 0, \
448+
.imm = IMM })
449+
440450
/* Relative call */
441451

442452
#define BPF_CALL_REL(TGT) \

kernel/bpf/helpers.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2034,6 +2034,7 @@ bpf_base_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
20342034
return NULL;
20352035
}
20362036
}
2037+
EXPORT_SYMBOL_GPL(bpf_base_func_proto);
20372038

20382039
void bpf_list_head_free(const struct btf_field *field, void *list_head,
20392040
struct bpf_spin_lock *spin_lock)

kernel/bpf/verifier.c

Lines changed: 59 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19286,6 +19286,9 @@ static int adjust_jmp_off(struct bpf_prog *prog, u32 tgt_idx, u32 delta)
1928619286
for (i = 0; i < insn_cnt; i++, insn++) {
1928719287
u8 code = insn->code;
1928819288

19289+
if (tgt_idx <= i && i < tgt_idx + delta)
19290+
continue;
19291+
1928919292
if ((BPF_CLASS(code) != BPF_JMP && BPF_CLASS(code) != BPF_JMP32) ||
1929019293
BPF_OP(code) == BPF_CALL || BPF_OP(code) == BPF_EXIT)
1929119294
continue;
@@ -19674,14 +19677,39 @@ static int opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env,
1967419677
*/
1967519678
static int convert_ctx_accesses(struct bpf_verifier_env *env)
1967619679
{
19680+
struct bpf_subprog_info *subprogs = env->subprog_info;
1967719681
const struct bpf_verifier_ops *ops = env->ops;
19678-
int i, cnt, size, ctx_field_size, delta = 0;
19682+
int i, cnt, size, ctx_field_size, delta = 0, epilogue_cnt = 0;
1967919683
const int insn_cnt = env->prog->len;
19680-
struct bpf_insn insn_buf[16], *insn;
19684+
struct bpf_insn *epilogue_buf = env->epilogue_buf;
19685+
struct bpf_insn *insn_buf = env->insn_buf;
19686+
struct bpf_insn *insn;
1968119687
u32 target_size, size_default, off;
1968219688
struct bpf_prog *new_prog;
1968319689
enum bpf_access_type type;
1968419690
bool is_narrower_load;
19691+
int epilogue_idx = 0;
19692+
19693+
if (ops->gen_epilogue) {
19694+
epilogue_cnt = ops->gen_epilogue(epilogue_buf, env->prog,
19695+
-(subprogs[0].stack_depth + 8));
19696+
if (epilogue_cnt >= INSN_BUF_SIZE) {
19697+
verbose(env, "bpf verifier is misconfigured\n");
19698+
return -EINVAL;
19699+
} else if (epilogue_cnt) {
19700+
/* Save the ARG_PTR_TO_CTX for the epilogue to use */
19701+
cnt = 0;
19702+
subprogs[0].stack_depth += 8;
19703+
insn_buf[cnt++] = BPF_STX_MEM(BPF_DW, BPF_REG_FP, BPF_REG_1,
19704+
-subprogs[0].stack_depth);
19705+
insn_buf[cnt++] = env->prog->insnsi[0];
19706+
new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt);
19707+
if (!new_prog)
19708+
return -ENOMEM;
19709+
env->prog = new_prog;
19710+
delta += cnt - 1;
19711+
}
19712+
}
1968519713

1968619714
if (ops->gen_prologue || env->seen_direct_write) {
1968719715
if (!ops->gen_prologue) {
@@ -19690,7 +19718,7 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
1969019718
}
1969119719
cnt = ops->gen_prologue(insn_buf, env->seen_direct_write,
1969219720
env->prog);
19693-
if (cnt >= ARRAY_SIZE(insn_buf)) {
19721+
if (cnt >= INSN_BUF_SIZE) {
1969419722
verbose(env, "bpf verifier is misconfigured\n");
1969519723
return -EINVAL;
1969619724
} else if (cnt) {
@@ -19703,6 +19731,9 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
1970319731
}
1970419732
}
1970519733

19734+
if (delta)
19735+
WARN_ON(adjust_jmp_off(env->prog, 0, delta));
19736+
1970619737
if (bpf_prog_is_offloaded(env->prog->aux))
1970719738
return 0;
1970819739

@@ -19735,6 +19766,25 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
1973519766
insn->code = BPF_STX | BPF_PROBE_ATOMIC | BPF_SIZE(insn->code);
1973619767
env->prog->aux->num_exentries++;
1973719768
continue;
19769+
} else if (insn->code == (BPF_JMP | BPF_EXIT) &&
19770+
epilogue_cnt &&
19771+
i + delta < subprogs[1].start) {
19772+
/* Generate epilogue for the main prog */
19773+
if (epilogue_idx) {
19774+
/* jump back to the earlier generated epilogue */
19775+
insn_buf[0] = BPF_JMP32_A(epilogue_idx - i - delta - 1);
19776+
cnt = 1;
19777+
} else {
19778+
memcpy(insn_buf, epilogue_buf,
19779+
epilogue_cnt * sizeof(*epilogue_buf));
19780+
cnt = epilogue_cnt;
19781+
/* epilogue_idx cannot be 0. It must have at
19782+
* least one ctx ptr saving insn before the
19783+
* epilogue.
19784+
*/
19785+
epilogue_idx = i + delta;
19786+
}
19787+
goto patch_insn_buf;
1973819788
} else {
1973919789
continue;
1974019790
}
@@ -19837,7 +19887,7 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
1983719887
target_size = 0;
1983819888
cnt = convert_ctx_access(type, insn, insn_buf, env->prog,
1983919889
&target_size);
19840-
if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf) ||
19890+
if (cnt == 0 || cnt >= INSN_BUF_SIZE ||
1984119891
(ctx_field_size && !target_size)) {
1984219892
verbose(env, "bpf verifier is misconfigured\n");
1984319893
return -EINVAL;
@@ -19846,7 +19896,7 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
1984619896
if (is_narrower_load && size < target_size) {
1984719897
u8 shift = bpf_ctx_narrow_access_offset(
1984819898
off, size, size_default) * 8;
19849-
if (shift && cnt + 1 >= ARRAY_SIZE(insn_buf)) {
19899+
if (shift && cnt + 1 >= INSN_BUF_SIZE) {
1985019900
verbose(env, "bpf verifier narrow ctx load misconfigured\n");
1985119901
return -EINVAL;
1985219902
}
@@ -19871,6 +19921,7 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
1987119921
insn->dst_reg, insn->dst_reg,
1987219922
size * 8, 0);
1987319923

19924+
patch_insn_buf:
1987419925
new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
1987519926
if (!new_prog)
1987619927
return -ENOMEM;
@@ -20391,7 +20442,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
2039120442
const int insn_cnt = prog->len;
2039220443
const struct bpf_map_ops *ops;
2039320444
struct bpf_insn_aux_data *aux;
20394-
struct bpf_insn insn_buf[16];
20445+
struct bpf_insn *insn_buf = env->insn_buf;
2039520446
struct bpf_prog *new_prog;
2039620447
struct bpf_map *map_ptr;
2039720448
int i, ret, cnt, delta = 0, cur_subprog = 0;
@@ -20510,7 +20561,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
2051020561
(BPF_MODE(insn->code) == BPF_ABS ||
2051120562
BPF_MODE(insn->code) == BPF_IND)) {
2051220563
cnt = env->ops->gen_ld_abs(insn, insn_buf);
20513-
if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf)) {
20564+
if (cnt == 0 || cnt >= INSN_BUF_SIZE) {
2051420565
verbose(env, "bpf verifier is misconfigured\n");
2051520566
return -EINVAL;
2051620567
}
@@ -20803,7 +20854,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
2080320854
cnt = ops->map_gen_lookup(map_ptr, insn_buf);
2080420855
if (cnt == -EOPNOTSUPP)
2080520856
goto patch_map_ops_generic;
20806-
if (cnt <= 0 || cnt >= ARRAY_SIZE(insn_buf)) {
20857+
if (cnt <= 0 || cnt >= INSN_BUF_SIZE) {
2080720858
verbose(env, "bpf verifier is misconfigured\n");
2080820859
return -EINVAL;
2080920860
}

0 commit comments

Comments
 (0)