From: Lorenz Bauer <lmb@cloudflare.com>
To: ast@kernel.org, yhs@fb.com, daniel@iogearbox.net, kafai@fb.com
Cc: bpf@vger.kernel.org, kernel-team@cloudflare.com,
Lorenz Bauer <lmb@cloudflare.com>
Subject: [PATCH bpf-next 10/11] bpf: hoist type checking for nullable arg types
Date: Fri, 4 Sep 2020 12:24:00 +0100 [thread overview]
Message-ID: <20200904112401.667645-11-lmb@cloudflare.com> (raw)
In-Reply-To: <20200904112401.667645-1-lmb@cloudflare.com>
check_func_arg has a plethora of weird if statements with empty branches.
They work around the fact that *_OR_NULL argument types should accept a
SCALAR_VALUE register, as long as it's value is 0. These statements make
it difficult to reason about the type checking logic.
Instead, skip more detailed type checking logic iff the register is 0,
and the function expects a nullable type. This allows simplifying the type
checking itself.
Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
kernel/bpf/verifier.c | 66 ++++++++++++++++++++-----------------------
1 file changed, 31 insertions(+), 35 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8d060da0b068..f124551c316a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -435,6 +435,15 @@ static bool arg_type_may_be_refcounted(enum bpf_arg_type type)
return type == ARG_PTR_TO_SOCK_COMMON;
}
+static bool arg_type_may_be_null(enum bpf_arg_type type)
+{
+ return type == ARG_PTR_TO_MAP_VALUE_OR_NULL ||
+ type == ARG_PTR_TO_MEM_OR_NULL ||
+ type == ARG_PTR_TO_CTX_OR_NULL ||
+ type == ARG_PTR_TO_SOCKET_OR_NULL ||
+ type == ARG_PTR_TO_ALLOC_MEM_OR_NULL;
+}
+
/* Determine whether the function releases some resources allocated by another
* function call. The first reference type argument will be assumed to be
* released by release_reference().
@@ -3946,17 +3955,20 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
return err;
}
+ if (register_is_null(reg) && arg_type_may_be_null(arg_type))
+ /* A NULL register has a SCALAR_VALUE type, so skip
+ * type checking.
+ */
+ goto skip_type_check;
+
if (arg_type == ARG_PTR_TO_MAP_KEY ||
arg_type == ARG_PTR_TO_MAP_VALUE ||
arg_type == ARG_PTR_TO_UNINIT_MAP_VALUE ||
arg_type == ARG_PTR_TO_MAP_VALUE_OR_NULL) {
expected_type = PTR_TO_STACK;
- if (register_is_null(reg) &&
- arg_type == ARG_PTR_TO_MAP_VALUE_OR_NULL)
- /* final test in check_stack_boundary() */;
- else if (!type_is_pkt_pointer(type) &&
- type != PTR_TO_MAP_VALUE &&
- type != expected_type)
+ if (!type_is_pkt_pointer(type) &&
+ type != PTR_TO_MAP_VALUE &&
+ type != expected_type)
goto err_type;
} else if (arg_type == ARG_CONST_SIZE ||
arg_type == ARG_CONST_SIZE_OR_ZERO ||
@@ -3971,11 +3983,8 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
} else if (arg_type == ARG_PTR_TO_CTX ||
arg_type == ARG_PTR_TO_CTX_OR_NULL) {
expected_type = PTR_TO_CTX;
- if (!(register_is_null(reg) &&
- arg_type == ARG_PTR_TO_CTX_OR_NULL)) {
- if (type != expected_type)
- goto err_type;
- }
+ if (type != expected_type)
+ goto err_type;
} else if (arg_type == ARG_PTR_TO_SOCK_COMMON) {
expected_type = PTR_TO_SOCK_COMMON;
/* Any sk pointer can be ARG_PTR_TO_SOCK_COMMON */
@@ -3984,12 +3993,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
} else if (arg_type == ARG_PTR_TO_SOCKET ||
arg_type == ARG_PTR_TO_SOCKET_OR_NULL) {
expected_type = PTR_TO_SOCKET;
- if (!(register_is_null(reg) &&
- arg_type == ARG_PTR_TO_SOCKET_OR_NULL)) {
- if (type != expected_type &&
- type != PTR_TO_BTF_ID)
- goto err_type;
- }
+ if (type != expected_type &&
+ type != PTR_TO_BTF_ID)
+ goto err_type;
btf_ids = &btf_fullsock_ids;
} else if (arg_type == ARG_PTR_TO_BTF_ID) {
expected_type = PTR_TO_BTF_ID;
@@ -4001,27 +4007,16 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
goto err_type;
} else if (arg_type_is_mem_ptr(arg_type)) {
expected_type = PTR_TO_STACK;
- /* One exception here. In case function allows for NULL to be
- * passed in as argument, it's a SCALAR_VALUE type. Final test
- * happens during stack boundary checking.
- */
- if (register_is_null(reg) &&
- (arg_type == ARG_PTR_TO_MEM_OR_NULL ||
- arg_type == ARG_PTR_TO_ALLOC_MEM_OR_NULL))
- /* final test in check_stack_boundary() */;
- else if (!type_is_pkt_pointer(type) &&
- type != PTR_TO_MAP_VALUE &&
- type != PTR_TO_MEM &&
- type != PTR_TO_RDONLY_BUF &&
- type != PTR_TO_RDWR_BUF &&
- type != expected_type)
+ if (!type_is_pkt_pointer(type) &&
+ type != PTR_TO_MAP_VALUE &&
+ type != PTR_TO_MEM &&
+ type != PTR_TO_RDONLY_BUF &&
+ type != PTR_TO_RDWR_BUF &&
+ type != expected_type)
goto err_type;
} else if (arg_type_is_alloc_mem_ptr(arg_type)) {
expected_type = PTR_TO_MEM;
- if (register_is_null(reg) &&
- arg_type == ARG_PTR_TO_ALLOC_MEM_OR_NULL)
- /* final test in check_stack_boundary() */;
- else if (type != expected_type)
+ if (type != expected_type)
goto err_type;
} else if (arg_type_is_int_ptr(arg_type)) {
expected_type = PTR_TO_STACK;
@@ -4056,6 +4051,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
}
}
+skip_type_check:
if (reg->ref_obj_id) {
if (meta->ref_obj_id) {
verbose(env, "verifier internal error: more than one arg with ref_obj_id R%d %u %u\n",
--
2.25.1
next prev parent reply other threads:[~2020-09-04 11:27 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-04 11:23 [PATCH bpf-next 00/11] RFC: Make check_func_arg table driven Lorenz Bauer
2020-09-04 11:23 ` [PATCH bpf-next 01/11] btf: Fix BTF_SET_START_GLOBAL macro Lorenz Bauer
2020-09-09 4:04 ` Andrii Nakryiko
2020-09-09 9:51 ` Lorenz Bauer
2020-09-04 11:23 ` [PATCH bpf-next 02/11] btf: add a global set of valid BTF socket ids Lorenz Bauer
2020-09-09 4:12 ` Andrii Nakryiko
2020-09-04 11:23 ` [PATCH bpf-next 03/11] btf: make btf_set_contains take a const pointer Lorenz Bauer
2020-09-09 4:13 ` Andrii Nakryiko
2020-09-04 11:23 ` [PATCH bpf-next 04/11] bpf: check scalar or invalid register in check_helper_mem_access Lorenz Bauer
2020-09-09 4:22 ` Andrii Nakryiko
2020-09-09 10:02 ` Lorenz Bauer
2020-09-04 11:23 ` [PATCH bpf-next 05/11] bpf: allow specifying a set of BTF IDs for helper arguments Lorenz Bauer
2020-09-06 23:04 ` Martin KaFai Lau
2020-09-07 9:15 ` Lorenz Bauer
2020-09-09 4:47 ` Andrii Nakryiko
2020-09-09 5:56 ` Martin KaFai Lau
2020-09-09 10:06 ` Lorenz Bauer
2020-09-09 10:11 ` Lorenz Bauer
2020-09-04 11:23 ` [PATCH bpf-next 06/11] bpf: make reference tracking in check_func_arg generic Lorenz Bauer
2020-09-09 4:50 ` Andrii Nakryiko
2020-09-04 11:23 ` [PATCH bpf-next 07/11] bpf: always check access to PTR_TO_CTX regardless of arg_type Lorenz Bauer
2020-09-04 11:23 ` [PATCH bpf-next 08/11] bpf: set meta->raw_mode for pointers to memory closer to it's use Lorenz Bauer
2020-09-04 11:23 ` [PATCH bpf-next 09/11] bpf: check ARG_PTR_TO_SPINLOCK register type in check_func_arg Lorenz Bauer
2020-09-04 11:24 ` Lorenz Bauer [this message]
2020-09-09 5:07 ` [PATCH bpf-next 10/11] bpf: hoist type checking for nullable arg types Andrii Nakryiko
2020-09-04 11:24 ` [PATCH bpf-next 11/11] bpf: use a table to drive helper arg type checks Lorenz Bauer
2020-09-09 6:17 ` [PATCH bpf-next 00/11] RFC: Make check_func_arg table driven Martin KaFai Lau
2020-09-09 10:15 ` Lorenz Bauer
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=20200904112401.667645-11-lmb@cloudflare.com \
--to=lmb@cloudflare.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kafai@fb.com \
--cc=kernel-team@cloudflare.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox