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

Improvements for BPF_ST tracking by verifier #4597

Closed
wants to merge 5 commits into from
Closed

Improvements for BPF_ST tracking by verifier #4597

wants to merge 5 commits into from

Conversation

kernel-patches-bot
Copy link

Pull request for series with
subject: Improvements for BPF_ST tracking by verifier
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=721887

@kernel-patches-bot
Copy link
Author

Upstream branch: 1f5dfcc
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=721887
version: 2

@kernel-patches-bot
Copy link
Author

Upstream branch: 213aacb
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=721887
version: 2

@kernel-patches-bot
Copy link
Author

Upstream branch: 5e53e5c
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=721887
version: 2

@eddyz87
bpf: track immediate values written to stack by BPF_ST instruction
8f3bb7e 
For aligned stack writes using BPF_ST instruction track stored values
in a same way BPF_STX is handled, e.g. make sure that the following
commands produce similar verifier knowledge:

  fp[-8] = 42;             r1 = 42;
                       fp[-8] = r1;

This covers two cases:
 - non-null values written to stack are stored as spill of fake
   registers;
 - null values written to stack are stored as STACK_ZERO marks.

Previously both cases above used STACK_MISC marks instead.

Some verifier test cases relied on the old logic to obtain STACK_MISC
marks for some stack values. These test cases are updated in the same
commit to avoid failures during bisect.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
selftests/bpf: check if verifier tracks constants spilled by BPF_ST_MEM
f1c17c6 
Check that verifier tracks the value of 'imm' spilled to stack by
BPF_ST_MEM instruction. Cover the following cases:
- write of non-zero constant to stack;
- write of a zero constant to stack.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
bpf: BPF_ST with variable offset should preserve STACK_ZERO marks
9b1580e 
BPF_STX instruction preserves STACK_ZERO marks for variable offset
writes in situations like below:

  *(u64*)(r10 - 8) = 0   ; STACK_ZERO marks for fp[-8]
  r0 = random(-7, -1)    ; some random number in range of [-7, -1]
  r0 += r10              ; r0 is now a variable offset pointer to stack
  r1 = 0
  *(u8*)(r0) = r1        ; BPF_STX writing zero, STACK_ZERO mark for
                         ; fp[-8] is preserved

This commit updates verifier.c:check_stack_write_var_off() to process
BPF_ST in a similar manner, e.g. the following example:

  *(u64*)(r10 - 8) = 0   ; STACK_ZERO marks for fp[-8]
  r0 = random(-7, -1)    ; some random number in range of [-7, -1]
  r0 += r10              ; r0 is now variable offset pointer to stack
  *(u8*)(r0) = 0         ; BPF_ST writing zero, STACK_ZERO mark for
                         ; fp[-8] is preserved

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@eddyz87
selftests/bpf: check if BPF_ST with variable offset preserves STACK_ZERO
c654bd5 
A test case to verify that variable offset BPF_ST instruction
preserves STACK_ZERO marks when writes zeros, e.g. in the following
situation:

  *(u64*)(r10 - 8) = 0   ; STACK_ZERO marks for fp[-8]
  r0 = random(-7, -1)    ; some random number in range of [-7, -1]
  r0 += r10              ; r0 is now variable offset pointer to stack
  *(u8*)(r0) = 0         ; BPF_ST writing zero, STACK_ZERO mark for
                         ; fp[-8] should be preserved.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
@kernel-patches-bot
Copy link
Author

Upstream branch: 62d101d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=721887
version: 2

@kernel-patches-bot
Copy link
Author

Upstream branch: b2d9002
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=721887
version: 2

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

Cmd('git') failed due to: exit code(128)
  cmdline: git am --3way
  stdout: 'Applying: bpf: track immediate values written to stack by BPF_ST instruction
Using index info to reconstruct a base tree...
M	kernel/bpf/verifier.c
M	tools/testing/selftests/bpf/verifier/bounds_mix_sign_unsign.c
Falling back to patching base and 3-way merge...
Auto-merging kernel/bpf/verifier.c
No changes -- Patch already applied.
Applying: selftests/bpf: check if verifier tracks constants spilled by BPF_ST_MEM
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
CONFLICT (add/add): Merge conflict in tools/testing/selftests/bpf/verifier/bpf_st_mem.c
Auto-merging tools/testing/selftests/bpf/verifier/bpf_st_mem.c
Patch failed at 0002 selftests/bpf: check if verifier tracks constants spilled by BPF_ST_MEM
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 tools/testing/selftests/bpf/verifier/bpf_st_mem.c
index 3af2501082b2,932903f9e585..000000000000
--- a/tools/testing/selftests/bpf/verifier/bpf_st_mem.c
+++ b/tools/testing/selftests/bpf/verifier/bpf_st_mem.c
@@@ -35,33 -35,3 +35,36 @@@
  	.expected_attach_type = BPF_SK_LOOKUP,
  	.runs = -1,
  },
++<<<<<<< HEAD
 +{
 +	"BPF_ST_MEM stack imm zero, variable offset",
 +	.insns = {
 +	/* set fp[-16], fp[-24] to zeros */
 +	BPF_ST_MEM(BPF_DW, BPF_REG_10, -16, 0),
 +	BPF_ST_MEM(BPF_DW, BPF_REG_10, -24, 0),
 +	/* r0 = random value in range [-32, -15] */
 +	BPF_EMIT_CALL(BPF_FUNC_get_prandom_u32),
 +	BPF_JMP_IMM(BPF_JLE, BPF_REG_0, 16, 2),
 +	BPF_MOV64_IMM(BPF_REG_0, 0),
 +	BPF_EXIT_INSN(),
 +	BPF_ALU64_IMM(BPF_SUB, BPF_REG_0, 32),
 +	/* fp[r0] = 0, make a variable offset write of zero,
 +	 *             this should preserve zero marks on stack.
 +	 */
 +	BPF_ALU64_REG(BPF_ADD, BPF_REG_0, BPF_REG_10),
 +	BPF_ST_MEM(BPF_B, BPF_REG_0, 0, 0),
 +	/* r0 = fp[-20], if variable offset write was tracked correctly
 +	 *               r0 would be a known zero.
 +	 */
 +	BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_10, -20),
 +	/* Would fail return code verification if r0 range is not tracked correctly. */
 +	BPF_EXIT_INSN(),
 +	},
 +	.result = ACCEPT,
 +	/* Use prog type that requires return value in range [0, 1] */
 +	.prog_type = BPF_PROG_TYPE_SK_LOOKUP,
 +	.expected_attach_type = BPF_SK_LOOKUP,
 +	.runs = -1,
 +},
++=======
++>>>>>>> selftests/bpf: check if verifier tracks constants spilled by BPF_ST_MEM

@kernel-patches-bot
Copy link
Author

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

@kernel-patches-bot kernel-patches-bot deleted the series/721887=>bpf-next branch February 15, 2023 20:05
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 V2
Projects
None yet
Milestone
No milestone
2 participants
@kernel-patches-bot @eddyz87