All of lore.kernel.org
 help / color / mirror / Atom feed
From: Edward Liaw <edliaw@google.com>
To: stable@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
	 Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	 Martin KaFai Lau <kafai@fb.com>,
	Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
	 John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,  Hao Luo <haoluo@google.com>
Cc: bpf@vger.kernel.org, kernel-team@android.com,
	 Edward Liaw <edliaw@google.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 5.15.y 3/5] bpf: Generally fix helper register offset check
Date: Wed, 17 Apr 2024 23:35:05 +0000	[thread overview]
Message-ID: <20240417233517.3044316-4-edliaw@google.com> (raw)
In-Reply-To: <20240417233517.3044316-1-edliaw@google.com>

From: Daniel Borkmann <daniel@iogearbox.net>

Right now the assertion on check_ptr_off_reg() is only enforced for register
types PTR_TO_CTX (and open coded also for PTR_TO_BTF_ID), however, this is
insufficient since many other PTR_TO_* register types such as PTR_TO_FUNC do
not handle/expect register offsets when passed to helper functions.

Given this can slip-through easily when adding new types, make this an explicit
allow-list and reject all other current and future types by default if this is
encountered.

Also, extend check_ptr_off_reg() to handle PTR_TO_BTF_ID as well instead of
duplicating it. For PTR_TO_BTF_ID, reg->off is used for BTF to match expected
BTF ids if struct offset is used. This part still needs to be allowed, but the
dynamic off from the tnum must be rejected.

Fixes: 69c087ba6225 ("bpf: Add bpf_for_each_map_elem() helper")
Fixes: eaa6bcb71ef6 ("bpf: Introduce bpf_per_cpu_ptr()")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
(cherry picked from commit 6788ab23508bddb0a9d88e104284922cb2c22b77)
Signed-off-by: Edward Liaw <edliaw@google.com>
---
 kernel/bpf/verifier.c | 39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 61b8a9c69b1c..14813fbebc9f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3792,14 +3792,15 @@ static int get_callee_stack_depth(struct bpf_verifier_env *env,
 }
 #endif
 
-int check_ptr_off_reg(struct bpf_verifier_env *env,
-		      const struct bpf_reg_state *reg, int regno)
+static int __check_ptr_off_reg(struct bpf_verifier_env *env,
+			       const struct bpf_reg_state *reg, int regno,
+			       bool fixed_off_ok)
 {
 	/* Access to this pointer-typed register or passing it to a helper
 	 * is only allowed in its original, unmodified form.
 	 */
 
-	if (reg->off) {
+	if (!fixed_off_ok && reg->off) {
 		verbose(env, "dereference of modified %s ptr R%d off=%d disallowed\n",
 			reg_type_str(env, reg->type), regno, reg->off);
 		return -EACCES;
@@ -3817,6 +3818,12 @@ int check_ptr_off_reg(struct bpf_verifier_env *env,
 	return 0;
 }
 
+int check_ptr_off_reg(struct bpf_verifier_env *env,
+		      const struct bpf_reg_state *reg, int regno)
+{
+	return __check_ptr_off_reg(env, reg, regno, false);
+}
+
 static int __check_buffer_access(struct bpf_verifier_env *env,
 				 const char *buf_info,
 				 const struct bpf_reg_state *reg,
@@ -5080,12 +5087,6 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
 				kernel_type_name(btf_vmlinux, *arg_btf_id));
 			return -EACCES;
 		}
-
-		if (!tnum_is_const(reg->var_off) || reg->var_off.value) {
-			verbose(env, "R%d is a pointer to in-kernel struct with non-zero offset\n",
-				regno);
-			return -EACCES;
-		}
 	}
 
 	return 0;
@@ -5140,10 +5141,26 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 	if (err)
 		return err;
 
-	if (type == PTR_TO_CTX) {
-		err = check_ptr_off_reg(env, reg, regno);
+	switch ((u32)type) {
+	case SCALAR_VALUE:
+	/* Pointer types where reg offset is explicitly allowed: */
+	case PTR_TO_PACKET:
+	case PTR_TO_PACKET_META:
+	case PTR_TO_MAP_KEY:
+	case PTR_TO_MAP_VALUE:
+	case PTR_TO_MEM:
+	case PTR_TO_MEM | MEM_RDONLY:
+	case PTR_TO_BUF:
+	case PTR_TO_BUF | MEM_RDONLY:
+	case PTR_TO_STACK:
+		break;
+	/* All the rest must be rejected: */
+	default:
+		err = __check_ptr_off_reg(env, reg, regno,
+					  type == PTR_TO_BTF_ID);
 		if (err < 0)
 			return err;
+		break;
 	}
 
 skip_type_check:
-- 
2.44.0.769.g3c40516874-goog


  parent reply	other threads:[~2024-04-17 23:36 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-24 12:01 FAILED: patch "[PATCH] bpf: Fix out of bounds access for ringbuf helpers" failed to apply to 5.15-stable tree gregkh
2024-04-17 23:35 ` [PATCH 5.15.y 0/5] Backport bounds checks for bpf Edward Liaw
2024-04-17 23:35   ` [PATCH 5.15.y 1/5] bpf: Extend kfunc with PTR_TO_CTX, PTR_TO_MEM argument support Edward Liaw
2024-04-17 23:35   ` [PATCH 5.15.y 2/5] bpf: Generalize check_ctx_reg for reuse with other types Edward Liaw
2024-04-17 23:35   ` Edward Liaw [this message]
2024-04-17 23:35   ` [PATCH 5.15.y 4/5] bpf: Fix out of bounds access for ringbuf helpers Edward Liaw
2024-04-17 23:35   ` [PATCH 5.15.y 5/5] bpf: Fix ringbuf memory type confusion when passing to helpers Edward Liaw
2024-04-18  1:07 ` [PATCH 5.15.y v2 0/5] Backport bounds checks for bpf Edward Liaw
2024-04-18  1:07   ` [PATCH 5.15.y v2 1/5] bpf: Extend kfunc with PTR_TO_CTX, PTR_TO_MEM argument support Edward Liaw
2024-04-18  1:07   ` [PATCH 5.15.y v2 2/5] bpf: Generalize check_ctx_reg for reuse with other types Edward Liaw
2024-04-18  1:07   ` [PATCH 5.15.y v2 3/5] bpf: Generally fix helper register offset check Edward Liaw
2024-04-18  1:07   ` [PATCH 5.15.y v2 4/5] bpf: Fix out of bounds access for ringbuf helpers Edward Liaw
2024-04-18  1:07   ` [PATCH 5.15.y v2 5/5] bpf: Fix ringbuf memory type confusion when passing to helpers Edward Liaw
2024-04-18 23:19 ` [PATCH 5.15.y v3 0/5] Backport bounds checks for bpf Edward Liaw
2024-04-18 23:19   ` [PATCH 5.15.y v3 1/5] bpf: Extend kfunc with PTR_TO_CTX, PTR_TO_MEM argument support Edward Liaw
2024-04-18 23:19   ` [PATCH 5.15.y v3 2/5] bpf: Generalize check_ctx_reg for reuse with other types Edward Liaw
2024-04-18 23:19   ` [PATCH 5.15.y v3 3/5] bpf: Generally fix helper register offset check Edward Liaw
2024-04-18 23:19   ` [PATCH 5.15.y v3 4/5] bpf: Fix out of bounds access for ringbuf helpers Edward Liaw
2024-04-18 23:19   ` [PATCH 5.15.y v3 5/5] bpf: Fix ringbuf memory type confusion when passing to helpers Edward Liaw
2024-04-19 11:09   ` [PATCH 5.15.y v3 0/5] Backport bounds checks for bpf Greg KH

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=20240417233517.3044316-4-edliaw@google.com \
    --to=edliaw@google.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=kernel-team@android.com \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    --cc=stable@vger.kernel.org \
    --cc=yhs@fb.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 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.