public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
From: Ilya Leoshkevich <iii@linux.ibm.com>
To: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>
Cc: bpf@vger.kernel.org, Heiko Carstens <hca@linux.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Ilya Leoshkevich <iii@linux.ibm.com>
Subject: [PATCH bpf-next 17/24] libbpf: Read usdt arg spec with bpf_probe_read_kernel()
Date: Wed, 25 Jan 2023 22:38:10 +0100	[thread overview]
Message-ID: <20230125213817.1424447-18-iii@linux.ibm.com> (raw)
In-Reply-To: <20230125213817.1424447-1-iii@linux.ibm.com>

Loading programs that use bpf_usdt_arg() on s390x fails with:

    ; switch (arg_spec->arg_type) {
    139: (61) r1 = *(u32 *)(r2 +8)
    R2 unbounded memory access, make sure to bounds check any such access

The bound checks seem to be already in place in the C code, and maybe
it's even possible to add extra bogus checks to placate LLVM or the
verifier. Take a simpler approach and just use a helper.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tools/lib/bpf/usdt.bpf.h | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/tools/lib/bpf/usdt.bpf.h b/tools/lib/bpf/usdt.bpf.h
index fdfd235e52c4..ddfa2521ab67 100644
--- a/tools/lib/bpf/usdt.bpf.h
+++ b/tools/lib/bpf/usdt.bpf.h
@@ -116,7 +116,7 @@ __weak __hidden
 int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res)
 {
 	struct __bpf_usdt_spec *spec;
-	struct __bpf_usdt_arg_spec *arg_spec;
+	struct __bpf_usdt_arg_spec arg_spec;
 	unsigned long val;
 	int err, spec_id;
 
@@ -133,21 +133,24 @@ int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res)
 	if (arg_num >= BPF_USDT_MAX_ARG_CNT || arg_num >= spec->arg_cnt)
 		return -ENOENT;
 
-	arg_spec = &spec->args[arg_num];
-	switch (arg_spec->arg_type) {
+	err = bpf_probe_read_kernel(&arg_spec, sizeof(arg_spec), &spec->args[arg_num]);
+	if (err)
+		return err;
+
+	switch (arg_spec.arg_type) {
 	case BPF_USDT_ARG_CONST:
 		/* Arg is just a constant ("-4@$-9" in USDT arg spec).
-		 * value is recorded in arg_spec->val_off directly.
+		 * value is recorded in arg_spec.val_off directly.
 		 */
-		val = arg_spec->val_off;
+		val = arg_spec.val_off;
 		break;
 	case BPF_USDT_ARG_REG:
 		/* Arg is in a register (e.g, "8@%rax" in USDT arg spec),
 		 * so we read the contents of that register directly from
 		 * struct pt_regs. To keep things simple user-space parts
-		 * record offsetof(struct pt_regs, <regname>) in arg_spec->reg_off.
+		 * record offsetof(struct pt_regs, <regname>) in arg_spec.reg_off.
 		 */
-		err = bpf_probe_read_kernel(&val, sizeof(val), (void *)ctx + arg_spec->reg_off);
+		err = bpf_probe_read_kernel(&val, sizeof(val), (void *)ctx + arg_spec.reg_off);
 		if (err)
 			return err;
 		break;
@@ -155,18 +158,18 @@ int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res)
 		/* Arg is in memory addressed by register, plus some offset
 		 * (e.g., "-4@-1204(%rbp)" in USDT arg spec). Register is
 		 * identified like with BPF_USDT_ARG_REG case, and the offset
-		 * is in arg_spec->val_off. We first fetch register contents
+		 * is in arg_spec.val_off. We first fetch register contents
 		 * from pt_regs, then do another user-space probe read to
 		 * fetch argument value itself.
 		 */
-		err = bpf_probe_read_kernel(&val, sizeof(val), (void *)ctx + arg_spec->reg_off);
+		err = bpf_probe_read_kernel(&val, sizeof(val), (void *)ctx + arg_spec.reg_off);
 		if (err)
 			return err;
-		err = bpf_probe_read_user(&val, sizeof(val), (void *)val + arg_spec->val_off);
+		err = bpf_probe_read_user(&val, sizeof(val), (void *)val + arg_spec.val_off);
 		if (err)
 			return err;
 #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
-		val >>= arg_spec->arg_bitshift;
+		val >>= arg_spec.arg_bitshift;
 #endif
 		break;
 	default:
@@ -177,11 +180,11 @@ int bpf_usdt_arg(struct pt_regs *ctx, __u64 arg_num, long *res)
 	 * necessary upper arg_bitshift bits, with sign extension if argument
 	 * is signed
 	 */
-	val <<= arg_spec->arg_bitshift;
-	if (arg_spec->arg_signed)
-		val = ((long)val) >> arg_spec->arg_bitshift;
+	val <<= arg_spec.arg_bitshift;
+	if (arg_spec.arg_signed)
+		val = ((long)val) >> arg_spec.arg_bitshift;
 	else
-		val = val >> arg_spec->arg_bitshift;
+		val = val >> arg_spec.arg_bitshift;
 	*res = val;
 	return 0;
 }
-- 
2.39.1


  parent reply	other threads:[~2023-01-25 21:40 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-25 21:37 [PATCH bpf-next 00/24] Support bpf trampoline for s390x Ilya Leoshkevich
2023-01-25 21:37 ` [PATCH bpf-next 01/24] selftests/bpf: Fix liburandom_read.so linker error Ilya Leoshkevich
2023-01-26  1:07   ` Andrii Nakryiko
2023-01-26 13:30     ` Ilya Leoshkevich
2023-01-25 21:37 ` [PATCH bpf-next 02/24] selftests/bpf: Fix symlink creation error Ilya Leoshkevich
2023-01-25 21:37 ` [PATCH bpf-next 03/24] selftests/bpf: Fix fexit_stress on s390x Ilya Leoshkevich
2023-01-25 21:37 ` [PATCH bpf-next 04/24] selftests/bpf: Fix trampoline_count " Ilya Leoshkevich
2023-01-25 21:37 ` [PATCH bpf-next 05/24] selftests/bpf: Fix kfree_skb " Ilya Leoshkevich
2023-01-25 21:37 ` [PATCH bpf-next 06/24] selftests/bpf: Set errno when urand_spawn() fails Ilya Leoshkevich
2023-01-25 21:38 ` [PATCH bpf-next 07/24] selftests/bpf: Fix decap_sanity_ns cleanup Ilya Leoshkevich
2023-01-25 21:38 ` [PATCH bpf-next 08/24] selftests/bpf: Fix verify_pkcs7_sig on s390x Ilya Leoshkevich
2023-01-26  1:06   ` Andrii Nakryiko
2023-01-27 12:36     ` Ilya Leoshkevich
2023-01-27 17:26       ` Andrii Nakryiko
2023-01-25 21:38 ` [PATCH bpf-next 09/24] selftests/bpf: Fix xdp_do_redirect " Ilya Leoshkevich
2023-01-25 21:38 ` [PATCH bpf-next 10/24] selftests/bpf: Fix cgrp_local_storage " Ilya Leoshkevich
2023-01-25 21:38 ` [PATCH bpf-next 11/24] selftests/bpf: Check stack_mprotect() return value Ilya Leoshkevich
2023-01-25 21:38 ` [PATCH bpf-next 12/24] selftests/bpf: Increase SIZEOF_BPF_LOCAL_STORAGE_ELEM on s390x Ilya Leoshkevich
2023-01-25 21:38 ` [PATCH bpf-next 13/24] selftests/bpf: Add a sign-extension test for kfuncs Ilya Leoshkevich
2023-01-25 21:38 ` [PATCH bpf-next 14/24] selftests/bpf: Fix test_lsm on s390x Ilya Leoshkevich
2023-01-25 21:38 ` [PATCH bpf-next 15/24] selftests/bpf: Fix test_xdp_adjust_tail_grow2 " Ilya Leoshkevich
2023-01-25 21:38 ` [PATCH bpf-next 16/24] selftests/bpf: Fix vmlinux test " Ilya Leoshkevich
2023-01-25 21:38 ` Ilya Leoshkevich [this message]
2023-01-26  0:26   ` [PATCH bpf-next 17/24] libbpf: Read usdt arg spec with bpf_probe_read_kernel() Andrii Nakryiko
2023-01-26 11:41     ` Ilya Leoshkevich
2023-01-26 19:03       ` Andrii Nakryiko
2023-01-27 11:01         ` Ilya Leoshkevich
2023-01-25 21:38 ` [PATCH bpf-next 18/24] s390/bpf: Fix a typo in a comment Ilya Leoshkevich
2023-01-25 21:38 ` [PATCH bpf-next 19/24] s390/bpf: Add expoline to tail calls Ilya Leoshkevich
2023-01-25 21:38 ` [PATCH bpf-next 20/24] s390/bpf: Implement bpf_arch_text_poke() Ilya Leoshkevich
2023-01-25 21:38 ` [PATCH bpf-next 21/24] bpf: btf: Add BTF_FMODEL_SIGNED_ARG flag Ilya Leoshkevich
2023-01-25 21:38 ` [PATCH bpf-next 22/24] s390/bpf: Implement arch_prepare_bpf_trampoline() Ilya Leoshkevich
2023-01-26  1:15   ` Andrii Nakryiko
2023-01-26 14:30     ` Ilya Leoshkevich
2023-01-26 19:06       ` Andrii Nakryiko
2023-01-27 11:15         ` Ilya Leoshkevich
2023-01-27 17:30           ` Andrii Nakryiko
2023-01-25 21:38 ` [PATCH bpf-next 23/24] s390/bpf: Implement bpf_jit_supports_subprog_tailcalls() Ilya Leoshkevich
2023-01-25 21:38 ` [PATCH bpf-next 24/24] s390/bpf: Implement bpf_jit_supports_kfunc_call() Ilya Leoshkevich
2023-01-26  1:28   ` Alexei Starovoitov
2023-01-27 11:36     ` Ilya Leoshkevich
2023-01-27 16:04       ` Alexei Starovoitov
2023-01-26  0:45 ` [PATCH bpf-next 00/24] Support bpf trampoline for s390x Andrii Nakryiko
2023-01-27 16:51   ` Ilya Leoshkevich
2023-01-27 17:24     ` Andrii Nakryiko
2023-01-27 22:50       ` Ilya Leoshkevich

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=20230125213817.1424447-18-iii@linux.ibm.com \
    --to=iii@linux.ibm.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox