Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpf/verifier: range computation improvements #6968

Closed
wants to merge 7 commits into from
Closed

bpf/verifier: range computation improvements #6968

wants to merge 7 commits into from

Conversation

kernel-patches-daemon-bpf[bot]
Copy link

Pull request for series with
subject: bpf/verifier: range computation improvements
version: 5
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=850816

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: a9e7715
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=850816
version: 5

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 8e6d9ae
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=850816
version: 5

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: e549b39
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=850816
version: 5

@cupermir
bpf/verifier: replace calls to mark_reg_unknown.
da28e2d 
In order to further simplify the code in adjust_scalar_min_max_vals all
the calls to mark_reg_unknown are replaced by __mark_reg_unknown.

static void mark_reg_unknown(struct bpf_verifier_env *env,
  			     struct bpf_reg_state *regs, u32 regno)
{
	if (WARN_ON(regno >= MAX_BPF_REG)) {
		... mark all regs not init ...
		return;
    }
	__mark_reg_unknown(env, regs + regno);
}

The 'regno >= MAX_BPF_REG' does not apply to
adjust_scalar_min_max_vals(), because it is only called from the
following stack:
  - check_alu_op
    - adjust_reg_min_max_vals
      - adjust_scalar_min_max_vals

The check_alu_op() does check_reg_arg() which verifies that both src and
dst register numbers are within bounds.

Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: David Faust <david.faust@oracle.com>
Cc: Jose Marchesi <jose.marchesi@oracle.com>
Cc: Elena Zannoni <elena.zannoni@oracle.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
@cupermir
bpf/verifier: refactor checks for range computation
9df2621 
Split range computation checks in its own function, isolating pessimitic
range set for dst_reg and failing return to a single point.

Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: David Faust <david.faust@oracle.com>
Cc: Jose Marchesi <jose.marchesi@oracle.com>
Cc: Elena Zannoni <elena.zannoni@oracle.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>

bpf/verifier: improve code after range computation recent changes.
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 41b307a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=850816
version: 5

@cupermir
bpf/verifier: improve XOR and OR range computation
15cb2ac 
Range for XOR and OR operators would not be attempted unless src_reg
would resolve to a single value, i.e. a known constant value.
This condition is unnecessary, and the following XOR/OR operator
handling could compute a possible better range.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: David Faust <david.faust@oracle.com>
Cc: Jose Marchesi <jose.marchesi@oracle.com>
Cc: Elena Zannoni <elena.zannoni@oracle.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
@cupermir
selftests/bpf: XOR and OR range computation tests.
2abe7a0 
Added a test for bound computation in XOR and OR when non constant
values are used and both registers have bounded ranges.

Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: David Faust <david.faust@oracle.com>
Cc: Jose Marchesi <jose.marchesi@oracle.com>
Cc: Elena Zannoni <elena.zannoni@oracle.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
@cupermir
bpf/verifier: relax MUL range computation check
91832e1 
MUL instruction required that src_reg would be a known value (i.e.
src_reg would be a const value). The condition in this case can be
relaxed, since the range computation algorithm used in current code
already supports a proper range computation for any valid range value on
its operands.

Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Acked-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: David Faust <david.faust@oracle.com>
Cc: Jose Marchesi <jose.marchesi@oracle.com>
Cc: Elena Zannoni <elena.zannoni@oracle.com>
@cupermir
selftests/bpf: MUL range computation tests.
4ad67aa 
Added a test for bound computation in MUL when non constant
values are used and both registers have bounded ranges.

Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Acked-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: David Faust <david.faust@oracle.com>
Cc: Jose Marchesi <jose.marchesi@oracle.com>
Cc: Elena Zannoni <elena.zannoni@oracle.com>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 329a672
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=850816
version: 5

Pull request is NOT updated. Failed to apply https://patchwork.kernel.org/project/netdevbpf/list/?series=850816
error message:

Cmd('git') failed due to: exit code(128)
  cmdline: git am --3way
  stdout: 'Applying: bpf/verifier: replace calls to mark_reg_unknown.
Using index info to reconstruct a base tree...
M	kernel/bpf/verifier.c
Falling back to patching base and 3-way merge...
Auto-merging kernel/bpf/verifier.c
CONFLICT (content): Merge conflict in kernel/bpf/verifier.c
Patch failed at 0001 bpf/verifier: replace calls to mark_reg_unknown.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".'
  stderr: 'error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch'

conflict:

diff --cc kernel/bpf/verifier.c
index 9e3aba08984e,41c66cc6db80..000000000000
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@@ -13988,24 -13989,46 +13988,58 @@@ static int adjust_scalar_min_max_vals(s
  		scalar_min_max_xor(dst_reg, &src_reg);
  		break;
  	case BPF_LSH:
++<<<<<<< HEAD
++=======
+ 		if (umax_val >= insn_bitness) {
+ 			/* Shifts greater than 31 or 63 are undefined.
+ 			 * This includes shifts by a negative number.
+ 			 */
+ 			__mark_reg_unknown(env, dst_reg);
+ 			break;
+ 		}
++>>>>>>> bpf/verifier: replace calls to mark_reg_unknown.
  		if (alu32)
  			scalar32_min_max_lsh(dst_reg, &src_reg);
  		else
  			scalar_min_max_lsh(dst_reg, &src_reg);
  		break;
  	case BPF_RSH:
++<<<<<<< HEAD
++=======
+ 		if (umax_val >= insn_bitness) {
+ 			/* Shifts greater than 31 or 63 are undefined.
+ 			 * This includes shifts by a negative number.
+ 			 */
+ 			__mark_reg_unknown(env, dst_reg);
+ 			break;
+ 		}
++>>>>>>> bpf/verifier: replace calls to mark_reg_unknown.
  		if (alu32)
  			scalar32_min_max_rsh(dst_reg, &src_reg);
  		else
  			scalar_min_max_rsh(dst_reg, &src_reg);
  		break;
  	case BPF_ARSH:
++<<<<<<< HEAD
++=======
+ 		if (umax_val >= insn_bitness) {
+ 			/* Shifts greater than 31 or 63 are undefined.
+ 			 * This includes shifts by a negative number.
+ 			 */
+ 			__mark_reg_unknown(env, dst_reg);
+ 			break;
+ 		}
++>>>>>>> bpf/verifier: replace calls to mark_reg_unknown.
  		if (alu32)
  			scalar32_min_max_arsh(dst_reg, &src_reg);
  		else
  			scalar_min_max_arsh(dst_reg, &src_reg);
  		break;
  	default:
++<<<<<<< HEAD
++=======
+ 		__mark_reg_unknown(env, dst_reg);
++>>>>>>> bpf/verifier: replace calls to mark_reg_unknown.
  		break;
  	}

@kernel-patches-daemon-bpf
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=850816 irrelevant now. Closing PR.

@kernel-patches-daemon-bpf kernel-patches-daemon-bpf bot deleted the series/845427=>bpf-next branch May 7, 2024 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
accepted bpf-next V5-ci-pass V5
Projects
None yet
Milestone
No milestone
1 participant
@cupermir