Patch "bpf: propagate precision in ALU/ALU64 operations" has been added to the 5.4-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: propagate precision in ALU/ALU64 operations

to the 5.4-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-propagate-precision-in-alu-alu64-operations.patch
and it can be found in the queue-5.4 subdirectory.

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



commit 22172f33af1ec72c9b8f56c0b95ba2690d18b090
Author: Andrii Nakryiko <andrii@xxxxxxxxxx>
Date:   Fri Nov 4 09:36:44 2022 -0700

    bpf: propagate precision in ALU/ALU64 operations
    
    [ Upstream commit a3b666bfa9c9edc05bca62a87abafe0936bd7f97 ]
    
    When processing ALU/ALU64 operations (apart from BPF_MOV, which is
    handled correctly already; and BPF_NEG and BPF_END are special and don't
    have source register), if destination register is already marked
    precise, this causes problem with potentially missing precision tracking
    for the source register. E.g., when we have r1 >>= r5 and r1 is marked
    precise, but r5 isn't, this will lead to r5 staying as imprecise. This
    is due to the precision backtracking logic stopping early when it sees
    r1 is already marked precise. If r1 wasn't precise, we'd keep
    backtracking and would add r5 to the set of registers that need to be
    marked precise. So there is a discrepancy here which can lead to invalid
    and incompatible states matched due to lack of precision marking on r5.
    If r1 wasn't precise, precision backtracking would correctly mark both
    r1 and r5 as precise.
    
    This is simple to fix, though. During the forward instruction simulation
    pass, for arithmetic operations of `scalar <op>= scalar` form (where
    <op> is ALU or ALU64 operations), if destination register is already
    precise, mark source register as precise. This applies only when both
    involved registers are SCALARs. `ptr += scalar` and `scalar += ptr`
    cases are already handled correctly.
    
    This does have (negative) effect on some selftest programs and few
    Cilium programs.  ~/baseline-tmp-results.csv are veristat results with
    this patch, while ~/baseline-results.csv is without it. See post
    scriptum for instructions on how to make Cilium programs testable with
    veristat. Correctness has a price.
    
    $ ./veristat -C -e file,prog,insns,states ~/baseline-results.csv ~/baseline-tmp-results.csv | grep -v '+0'
    File                     Program               Total insns (A)  Total insns (B)  Total insns (DIFF)  Total states (A)  Total states (B)  Total states (DIFF)
    -----------------------  --------------------  ---------------  ---------------  ------------------  ----------------  ----------------  -------------------
    bpf_cubic.bpf.linked1.o  bpf_cubic_cong_avoid              997             1700      +703 (+70.51%)                62                90        +28 (+45.16%)
    test_l4lb.bpf.linked1.o  balancer_ingress                 4559             5469      +910 (+19.96%)               118               126          +8 (+6.78%)
    -----------------------  --------------------  ---------------  ---------------  ------------------  ----------------  ----------------  -------------------
    
    $ ./veristat -C -e file,prog,verdict,insns,states ~/baseline-results-cilium.csv ~/baseline-tmp-results-cilium.csv | grep -v '+0'
    File           Program                         Total insns (A)  Total insns (B)  Total insns (DIFF)  Total states (A)  Total states (B)  Total states (DIFF)
    -------------  ------------------------------  ---------------  ---------------  ------------------  ----------------  ----------------  -------------------
    bpf_host.o     tail_nodeport_nat_ingress_ipv6             4448             5261      +813 (+18.28%)               234               247         +13 (+5.56%)
    bpf_host.o     tail_nodeport_nat_ipv6_egress              3396             3446        +50 (+1.47%)               201               203          +2 (+1.00%)
    bpf_lxc.o      tail_nodeport_nat_ingress_ipv6             4448             5261      +813 (+18.28%)               234               247         +13 (+5.56%)
    bpf_overlay.o  tail_nodeport_nat_ingress_ipv6             4448             5261      +813 (+18.28%)               234               247         +13 (+5.56%)
    bpf_xdp.o      tail_lb_ipv4                              71736            73442      +1706 (+2.38%)              4295              4370         +75 (+1.75%)
    -------------  ------------------------------  ---------------  ---------------  ------------------  ----------------  ----------------  -------------------
    
    P.S. To make Cilium ([0]) programs libbpf-compatible and thus
    veristat-loadable, apply changes from topmost commit in [1], which does
    minimal changes to Cilium source code, mostly around SEC() annotations
    and BPF map definitions.
    
      [0] https://github.com/cilium/cilium/
      [1] https://github.com/anakryiko/cilium/commits/libbpf-friendliness
    
    Fixes: b5dc0163d8fd ("bpf: precise scalar_value tracking")
    Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
    Link: https://lore.kernel.org/r/20221104163649.121784-2-andrii@xxxxxxxxxx
    Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f705d3752fe0..32b32ecad770 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5140,6 +5140,11 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env,
 				return err;
 			return adjust_ptr_min_max_vals(env, insn,
 						       dst_reg, src_reg);
+		} else if (dst_reg->precise) {
+			/* if dst_reg is precise, src_reg should be precise as well */
+			err = mark_chain_precision(env, insn->src_reg);
+			if (err)
+				return err;
 		}
 	} else {
 		/* Pretend the src is a reg with a known value, since we only



[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