All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: bpf@vger.kernel.org, ast@kernel.org
Cc: andrii@kernel.org, daniel@iogearbox.net, martin.lau@linux.dev,
	kernel-team@fb.com, yonghong.song@linux.dev,
	jose.marchesi@oracle.com, Eduard Zingerman <eddyz87@gmail.com>
Subject: [bpf-next v3 11/12] bpf: do check_nocsr_stack_contract() for ARG_ANYTHING helper params
Date: Mon, 15 Jul 2024 16:02:00 -0700	[thread overview]
Message-ID: <20240715230201.3901423-12-eddyz87@gmail.com> (raw)
In-Reply-To: <20240715230201.3901423-1-eddyz87@gmail.com>

There is a number of BPF helper functions that use ARG_ANYTHING to
mark parameters that are used as memory addresses.
An address of BPF stack slot could be passed as such parameter
for two such helper functions:
- bpf_probe_read_kernel
- bpf_probe_read_kernel_str

This might lead to a surprising behavior in combination with nocsr
rewrites, e.g. consider the program below:

     1: r1 = 1;
        /* nocsr pattern with stack offset -16 */
     2: *(u64 *)(r10 - 16) = r1;
     3: call %[bpf_get_smp_processor_id];
     4: r1 = *(u64 *)(r10 - 16);
     5: r1 = r10;
     6: r1 += -8;
     7: r2 = 1;
     8: r3 = r10;
     9: r3 += -16;
        /* bpf_probe_read_kernel(dst: &fp[-8], size: 1, src: &fp[-16]) */
    10: call %[bpf_probe_read_kernel];
    11: exit;

Here nocsr rewrite logic would remove instructions (2) and (4).
However, (2) writes a value that is later read by a call at (10).

Function check_func_arg() is called from check_helper_call() and is
responsible for memory access checks, when helper argument type is
declared as ARG_PTR_TO_... .

However, for ARG_ANYTHING this function returns early and does not
call check_stack_{read,write}().

This patch opts to add a check_nocsr_stack_contract() to the
ARG_ANYTHING return path if passed parameter is a pointer to stack.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 kernel/bpf/verifier.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 438daf36a694..77affc563a64 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -8684,11 +8684,15 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 		return err;
 
 	if (arg_type == ARG_ANYTHING) {
+		/* return value depends on env->allow_ptr_leaks */
 		if (is_pointer_value(env, regno)) {
 			verbose(env, "R%d leaks addr into helper function\n",
 				regno);
 			return -EACCES;
 		}
+		if (reg->type == PTR_TO_STACK)
+			check_nocsr_stack_contract(env, cur_func(env), insn_idx,
+						   reg->smin_value + reg->off);
 		return 0;
 	}
 
-- 
2.45.2


  parent reply	other threads:[~2024-07-15 23:02 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-15 23:01 [bpf-next v3 00/12] no_caller_saved_registers attribute for helper calls Eduard Zingerman
2024-07-15 23:01 ` [bpf-next v3 01/12] bpf: add a get_helper_proto() utility function Eduard Zingerman
2024-07-15 23:01 ` [bpf-next v3 02/12] bpf: no_caller_saved_registers attribute for helper calls Eduard Zingerman
2024-07-16  1:51   ` Alexei Starovoitov
2024-07-16  5:34     ` Eduard Zingerman
2024-07-20  2:00       ` Alexei Starovoitov
2024-07-22 18:42         ` Eduard Zingerman
2024-07-15 23:01 ` [bpf-next v3 03/12] bpf, x86, riscv, arm: no_caller_saved_registers for bpf_get_smp_processor_id() Eduard Zingerman
2024-07-15 23:01 ` [bpf-next v3 04/12] selftests/bpf: extract utility function for BPF disassembly Eduard Zingerman
2024-07-15 23:01 ` [bpf-next v3 05/12] selftests/bpf: print correct offset for pseudo calls in disasm_insn() Eduard Zingerman
2024-07-15 23:01 ` [bpf-next v3 06/12] selftests/bpf: no need to track next_match_pos in struct test_loader Eduard Zingerman
2024-07-15 23:01 ` [bpf-next v3 07/12] selftests/bpf: extract test_loader->expect_msgs as a data structure Eduard Zingerman
2024-07-15 23:01 ` [bpf-next v3 08/12] selftests/bpf: allow checking xlated programs in verifier_* tests Eduard Zingerman
2024-07-15 23:01 ` [bpf-next v3 09/12] selftests/bpf: __arch_* macro to limit test cases to specific archs Eduard Zingerman
2024-07-15 23:01 ` [bpf-next v3 10/12] selftests/bpf: test no_caller_saved_registers spill/fill removal Eduard Zingerman
2024-07-15 23:02 ` Eduard Zingerman [this message]
2024-07-16  2:00   ` [bpf-next v3 11/12] bpf: do check_nocsr_stack_contract() for ARG_ANYTHING helper params Alexei Starovoitov
2024-07-16 10:03     ` Eduard Zingerman
2024-07-16 18:15       ` Eduard Zingerman
2024-07-20  1:54         ` Alexei Starovoitov
2024-07-20  1:58           ` Eduard Zingerman
2024-07-15 23:02 ` [bpf-next v3 12/12] selftests/bpf: check nocsr contract for bpf_probe_read_kernel() Eduard Zingerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240715230201.3901423-12-eddyz87@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jose.marchesi@oracle.com \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@linux.dev \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.