Patch "bpf: Ensure proper register state printing for cond jumps" has been added to the 6.1-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    bpf: Ensure proper register state printing for cond jumps

to the 6.1-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     bpf-ensure-proper-register-state-printing-for-cond-j.patch
and it can be found in the queue-6.1 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 5ec7f6fd77d019ecafbcc6d35c504cde70c01656
Author: Andrii Nakryiko <andrii@xxxxxxxxxx>
Date:   Wed Oct 11 15:37:28 2023 -0700

    bpf: Ensure proper register state printing for cond jumps
    
    [ Upstream commit 1a8a315f008a58f54fecb012b928aa6a494435b3 ]
    
    Verifier emits relevant register state involved in any given instruction
    next to it after `;` to the right, if possible. Or, worst case, on the
    separate line repeating instruction index.
    
    E.g., a nice and simple case would be:
    
      2: (d5) if r0 s<= 0x0 goto pc+1       ; R0_w=0
    
    But if there is some intervening extra output (e.g., precision
    backtracking log) involved, we are supposed to see the state after the
    precision backtrack log:
    
      4: (75) if r0 s>= 0x0 goto pc+1
      mark_precise: frame0: last_idx 4 first_idx 0 subseq_idx -1
      mark_precise: frame0: regs=r0 stack= before 2: (d5) if r0 s<= 0x0 goto pc+1
      mark_precise: frame0: regs=r0 stack= before 1: (b7) r0 = 0
      6: R0_w=0
    
    First off, note that in `6: R0_w=0` instruction index corresponds to the
    next instruction, not to the conditional jump instruction itself, which
    is wrong and we'll get to that.
    
    But besides that, the above is a happy case that does work today. Yet,
    if it so happens that precision backtracking had to traverse some of the
    parent states, this `6: R0_w=0` state output would be missing.
    
    This is due to a quirk of print_verifier_state() routine, which performs
    mark_verifier_state_clean(env) at the end. This marks all registers as
    "non-scratched", which means that subsequent logic to print *relevant*
    registers (that is, "scratched ones") fails and doesn't see anything
    relevant to print and skips the output altogether.
    
    print_verifier_state() is used both to print instruction context, but
    also to print an **entire** verifier state indiscriminately, e.g.,
    during precision backtracking (and in a few other situations, like
    during entering or exiting subprogram).  Which means if we have to print
    entire parent state before getting to printing instruction context
    state, instruction context is marked as clean and is omitted.
    
    Long story short, this is definitely not intentional. So we fix this
    behavior in this patch by teaching print_verifier_state() to clear
    scratch state only if it was used to print instruction state, not the
    parent/callback state. This is determined by print_all option, so if
    it's not set, we don't clear scratch state. This fixes missing
    instruction state for these cases.
    
    As for the mismatched instruction index, we fix that by making sure we
    call print_insn_state() early inside check_cond_jmp_op() before we
    adjusted insn_idx based on jump branch taken logic. And with that we get
    desired correct information:
    
      9: (16) if w4 == 0x1 goto pc+9
      mark_precise: frame0: last_idx 9 first_idx 9 subseq_idx -1
      mark_precise: frame0: parent state regs=r4 stack=: R2_w=1944 R4_rw=P1 R10=fp0
      mark_precise: frame0: last_idx 8 first_idx 0 subseq_idx 9
      mark_precise: frame0: regs=r4 stack= before 8: (66) if w4 s> 0x3 goto pc+5
      mark_precise: frame0: regs=r4 stack= before 7: (b7) r4 = 1
      9: R4=1
    
    Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
    Signed-off-by: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
    Acked-by: John Fastabend <john.fastabend@xxxxxxxxx>
    Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
    Link: https://lore.kernel.org/bpf/20231011223728.3188086-6-andrii@xxxxxxxxxx
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index eb3f52be115d6..7fbc6492fe7b4 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -978,7 +978,8 @@ static void print_verifier_state(struct bpf_verifier_env *env,
 	if (state->in_async_callback_fn)
 		verbose(env, " async_cb");
 	verbose(env, "\n");
-	mark_verifier_state_clean(env);
+	if (!print_all)
+		mark_verifier_state_clean(env);
 }
 
 static inline u32 vlog_alignment(u32 pos)
@@ -10476,6 +10477,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 		    !sanitize_speculative_path(env, insn, *insn_idx + 1,
 					       *insn_idx))
 			return -EFAULT;
+		if (env->log.level & BPF_LOG_LEVEL)
+			print_insn_state(env, this_branch->frame[this_branch->curframe]);
 		*insn_idx += insn->off;
 		return 0;
 	} else if (pred == 0) {
@@ -10488,6 +10491,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 					       *insn_idx + insn->off + 1,
 					       *insn_idx))
 			return -EFAULT;
+		if (env->log.level & BPF_LOG_LEVEL)
+			print_insn_state(env, this_branch->frame[this_branch->curframe]);
 		return 0;
 	}
 



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux