A BPF_EXIT of a subprogram returning a value larger than 8 bytes (a struct/union or an __int128) reads R2 as well as R0, since the second half of the return value is passed back in R2. compute_insn_live_regs() only marked R0 used at exit, so a callee's R2 could be considered dead and cleaned from checkpointed states, which would allow unsound state pruning. Mark R2 as read at the BPF_EXIT of a subprogram that does return a register pair. bpf_compute_live_registers() walks the instructions in order and env->subprog_info[] is sorted by subprogram start, so the containing subprogram is tracked with a running index and its return convention is queried once per subprogram through bpf_ret_reg_pair(). Marking R2 at every exit instead would be simpler, but R2 would then stay live backwards across any call that is not followed by a write to R2, which is nearly every program, and would needlessly hurt state pruning. Signed-off-by: Yonghong Song --- kernel/bpf/liveness.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c index ef9a5a922887..451edd74fa6f 100644 --- a/kernel/bpf/liveness.c +++ b/kernel/bpf/liveness.c @@ -2062,10 +2062,15 @@ static inline u32 mask_widen(u32 m) { return m | (m << 16); } static inline u16 mask_lo(u32 m) { return (u16)m; } static inline u16 mask_hi(u32 m) { return (u16)(m >> 16); } -/* Compute info->{use,def} fields for the instruction */ +/* + * Compute info->{use,def} fields for the instruction. @ret_reg_pair tells + * whether the subprogram containing @insn returns its value in the R0:R2 + * register pair, which matters for BPF_EXIT. + */ static void compute_insn_live_regs(struct bpf_verifier_env *env, struct bpf_insn *insn, - struct insn_live_regs *info) + struct insn_live_regs *info, + bool ret_reg_pair) { struct bpf_call_summary cs; const u8 class = BPF_CLASS(insn->code); @@ -2196,7 +2201,7 @@ static void compute_insn_live_regs(struct bpf_verifier_env *env, break; case BPF_EXIT: def = 0; - use = r0; + use = ret_reg_pair ? (r0 | reg64_mask(BPF_REG_2)) : r0; break; case BPF_CALL: def = ALL_CALLER_SAVED_REGS; @@ -2233,8 +2238,8 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env) struct insn_live_regs *state; int insn_cnt = env->prog->len; u64 pos, insn_pos; - int err = 0, i, j; - bool changed; + int err = 0, i, j, subprog, start, end; + bool changed, ret_reg_pair; /* Use the following algorithm: * - define the following: @@ -2261,8 +2266,14 @@ int bpf_compute_live_registers(struct bpf_verifier_env *env) goto out; } - for (i = 0; i < insn_cnt; ++i) - compute_insn_live_regs(env, &insns[i], &state[i]); + for (subprog = 0; subprog < env->subprog_cnt; subprog++) { + start = env->subprog_info[subprog].start; + end = env->subprog_info[subprog + 1].start; + ret_reg_pair = bpf_ret_reg_pair(env, subprog); + + for (i = start; i < end; ++i) + compute_insn_live_regs(env, &insns[i], &state[i], ret_reg_pair); + } /* Forward pass: resolve stack access through FP-derived pointers */ err = bpf_compute_subprog_arg_access(env); -- 2.53.0-Meta