All of lore.kernel.org
 help / color / mirror / Atom feed
From: KP Singh <kpsingh@kernel.org>
To: bpf@vger.kernel.org, linux-security-module@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Cc: KP Singh <kpsingh@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Benjamin Tissoires <benjamin.tissoires@redhat.com>,
	Yosry Ahmed <yosryahmed@google.com>
Subject: [PATCH v5 bpf-next 2/5] bpf: kfunc support for ARG_PTR_TO_CONST_STR
Date: Tue, 28 Jun 2022 16:19:45 +0000	[thread overview]
Message-ID: <20220628161948.475097-3-kpsingh@kernel.org> (raw)
In-Reply-To: <20220628161948.475097-1-kpsingh@kernel.org>

kfuncs can handle pointers to memory when the next argument is
the size of the memory that can be read and verify these as
ARG_CONST_SIZE_OR_ZERO

Similarly add support for string constants (const char *) and
verify it similar to ARG_PTR_TO_CONST_STR.

Signed-off-by: KP Singh <kpsingh@kernel.org>
---
 include/linux/bpf_verifier.h |  2 +
 kernel/bpf/btf.c             | 30 ++++++++++++
 kernel/bpf/verifier.c        | 89 +++++++++++++++++++++---------------
 3 files changed, 83 insertions(+), 38 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 81b19669efba..f6d8898270d5 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -560,6 +560,8 @@ int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state
 			     u32 regno);
 int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
 		   u32 regno, u32 mem_size);
+int check_const_str(struct bpf_verifier_env *env,
+		    const struct bpf_reg_state *reg, int regno);
 
 /* this lives here instead of in bpf.h because it needs to dereference tgt_prog */
 static inline u64 bpf_trampoline_compute_key(const struct bpf_prog *tgt_prog,
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 37bc77b3b499..9b9d6117deae 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6162,6 +6162,27 @@ static bool is_kfunc_arg_mem_size(const struct btf *btf,
 	return true;
 }
 
+static bool btf_param_is_const_str_ptr(const struct btf *btf,
+				       const struct btf_param *param)
+{
+	const struct btf_type *t;
+	bool is_const = false;
+
+	t = btf_type_by_id(btf, param->type);
+	if (!btf_type_is_ptr(t))
+		return false;
+
+	t = btf_type_by_id(btf, t->type);
+	while (btf_type_is_modifier(t)) {
+		if (BTF_INFO_KIND(t->info) == BTF_KIND_CONST)
+			is_const = true;
+		t = btf_type_by_id(btf, t->type);
+	}
+
+	return (is_const &&
+		!strcmp(btf_name_by_offset(btf, t->name_off), "char"));
+}
+
 static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 				    const struct btf *btf, u32 func_id,
 				    struct bpf_reg_state *regs,
@@ -6344,10 +6365,19 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 		} else if (ptr_to_mem_ok) {
 			const struct btf_type *resolve_ret;
 			u32 type_size;
+			int err;
 
 			if (is_kfunc) {
 				bool arg_mem_size = i + 1 < nargs && is_kfunc_arg_mem_size(btf, &args[i + 1], &regs[regno + 1]);
 
+
+				if (btf_param_is_const_str_ptr(btf, &args[i])) {
+					err = check_const_str(env, reg, regno);
+					if (err < 0)
+						return err;
+					continue;
+				}
+
 				/* Permit pointer to mem, but only when argument
 				 * type is pointer to scalar, or struct composed
 				 * (recursively) of scalars.
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 4938477912cd..8ce5d2f86c1e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5840,6 +5840,56 @@ static u32 stack_slot_get_id(struct bpf_verifier_env *env, struct bpf_reg_state
 	return state->stack[spi].spilled_ptr.id;
 }
 
+int check_const_str(struct bpf_verifier_env *env,
+		    const struct bpf_reg_state *reg, int regno)
+{
+	struct bpf_map *map;
+	int map_off;
+	u64 map_addr;
+	char *str_ptr;
+	int err;
+
+	if (reg->type != PTR_TO_MAP_VALUE)
+		return -EACCES;
+
+	map = reg->map_ptr;
+	if (!bpf_map_is_rdonly(map)) {
+		verbose(env, "R%d does not point to a readonly map'\n", regno);
+		return -EACCES;
+	}
+
+	if (!tnum_is_const(reg->var_off)) {
+		verbose(env, "R%d is not a constant address'\n", regno);
+		return -EACCES;
+	}
+
+	if (!map->ops->map_direct_value_addr) {
+		verbose(env,
+			"no direct value access support for this map type\n");
+		return -EACCES;
+	}
+
+	err = check_map_access(env, regno, reg->off, map->value_size - reg->off,
+			       false, ACCESS_HELPER);
+	if (err)
+		return err;
+
+	map_off = reg->off + reg->var_off.value;
+	err = map->ops->map_direct_value_addr(map, &map_addr, map_off);
+	if (err) {
+		verbose(env, "direct value access on string failed\n");
+		return err;
+	}
+
+	str_ptr = (char *)(long)(map_addr);
+	if (!strnchr(str_ptr + map_off, map->value_size - map_off, 0)) {
+		verbose(env, "string is not zero-terminated\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 			  struct bpf_call_arg_meta *meta,
 			  const struct bpf_func_proto *fn)
@@ -6074,44 +6124,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 			return err;
 		err = check_ptr_alignment(env, reg, 0, size, true);
 	} else if (arg_type == ARG_PTR_TO_CONST_STR) {
-		struct bpf_map *map = reg->map_ptr;
-		int map_off;
-		u64 map_addr;
-		char *str_ptr;
-
-		if (!bpf_map_is_rdonly(map)) {
-			verbose(env, "R%d does not point to a readonly map'\n", regno);
-			return -EACCES;
-		}
-
-		if (!tnum_is_const(reg->var_off)) {
-			verbose(env, "R%d is not a constant address'\n", regno);
-			return -EACCES;
-		}
-
-		if (!map->ops->map_direct_value_addr) {
-			verbose(env, "no direct value access support for this map type\n");
-			return -EACCES;
-		}
-
-		err = check_map_access(env, regno, reg->off,
-				       map->value_size - reg->off, false,
-				       ACCESS_HELPER);
-		if (err)
-			return err;
-
-		map_off = reg->off + reg->var_off.value;
-		err = map->ops->map_direct_value_addr(map, &map_addr, map_off);
-		if (err) {
-			verbose(env, "direct value access on string failed\n");
-			return err;
-		}
-
-		str_ptr = (char *)(long)(map_addr);
-		if (!strnchr(str_ptr + map_off, map->value_size - map_off, 0)) {
-			verbose(env, "string is not zero-terminated\n");
-			return -EINVAL;
-		}
+		err = check_const_str(env, reg, regno);
 	} else if (arg_type == ARG_PTR_TO_KPTR) {
 		if (process_kptr_func(env, regno, meta))
 			return -EACCES;
-- 
2.37.0.rc0.161.g10f37bed90-goog


  parent reply	other threads:[~2022-06-28 16:30 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-28 16:19 [PATCH v5 bpf-next 0/5] Add bpf_getxattr KP Singh
2022-06-28 16:19 ` [PATCH v5 bpf-next 1/5] btf: Add a new kfunc set which allows to mark a function to be sleepable KP Singh
2022-06-28 16:19 ` KP Singh [this message]
2022-06-28 16:19 ` [PATCH v5 bpf-next 3/5] bpf: Allow kfuncs to be used in LSM programs KP Singh
2022-06-28 16:19 ` [PATCH v5 bpf-next 4/5] bpf: Add a bpf_getxattr kfunc KP Singh
2022-06-28 17:22   ` Christian Brauner
2022-06-28 17:23   ` Al Viro
2022-06-28 17:29     ` KP Singh
2022-06-28 16:19 ` [PATCH v5 bpf-next 5/5] bpf/selftests: Add a selftest for bpf_getxattr KP Singh
2022-06-28 17:33   ` Christian Brauner
2022-06-28 17:52     ` KP Singh
2022-06-28 22:28       ` Alexei Starovoitov
2022-06-29  8:11         ` Christian Brauner
2022-06-29  9:55           ` Christian Brauner
2022-06-30  3:02             ` Alexei Starovoitov
2022-06-30 11:45               ` Christian Brauner
2022-06-30 12:21                 ` KP Singh
2022-06-30 12:23                   ` KP Singh
2022-06-30 13:26                   ` Christian Brauner
2022-06-30 13:29                     ` KP Singh
2022-06-30 13:47                       ` Christian Brauner
2022-06-30 14:37                         ` Christian Brauner
2022-06-30 16:10                         ` Casey Schaufler
2022-06-30 22:23                           ` KP Singh
2022-06-30 23:23                             ` Casey Schaufler
2022-07-01  8:32                               ` Amir Goldstein
2022-07-01  8:58                                 ` Christian Brauner
2022-07-01  9:24                                   ` Amir Goldstein
2022-06-30 16:28                   ` Amir Goldstein
2022-06-30 22:25                     ` KP Singh
2022-06-28 17:13 ` [PATCH v5 bpf-next 0/5] Add bpf_getxattr Christian Brauner
2022-06-28 17:20   ` KP Singh
2022-06-28 17:21     ` KP Singh
2022-06-29  1:36       ` Dave Chinner
2022-06-29  2:00         ` KP Singh
2022-06-29  2:05           ` KP Singh

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=20220628161948.475097-3-kpsingh@kernel.org \
    --to=kpsingh@kernel.org \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=benjamin.tissoires@redhat.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=yosryahmed@google.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.