In the verifier, PTR_TO_BTF_ID is a bit special, as it can be NULL at
runtime. Helpers and kfuncs are expected to handle this case by NULL
checking even if not taking the OR_NULL variant of PTR_TO_BTF_ID, as it
can be obtained using pointer walking (btf_struct_walk) and be NULL.
The first job is to fix this assumption for all helpers, which
subsequent commits will do.
But a more serious issue occurs when the BTF ID that a helper or kfunc
is expecting may be an embedded object in another object whose
PTR_TO_BTF_ID program has access to. In that case, that parent pointer
may be NULL, and the pointer to member embedded in it would be formed by
adding member offset to the pointer.
Ultimately, verifier does a check using btf_struct_ids_match which takes
into account the non-zero reg->off and verifier the pointer to member
formed by pointer increment is same as the one helper is expecting.
However, it may have been formed using a parent pointer that is NULL.
This case would subvert the NULL check that (some) helpers correctly do,
hence some adjustment is needed, because the resulting pointer is NULL +
offset.
First, since all architectures fault in the first page size bytes for
NULL pointer dereference reliably (and consider any other faulting
address to be bad page fault), limit the offset that can be added and
passed to the helper function to PAGE_SIZE bytes, considering higher
address may be actually be valid.
This is a backwards incompatible change, as it restricts to forming
member pointers for structs whose size < PAGE_SIZE, or more precisely
member offset lies in first page, when passing to BPF helpers. The
kfuncs are already unstable so no concerns there. It could be possible
to bump it to higher limit as long as it doesn't overlap with valid
address range.
A test case in later commit is included which crashes the kernel by
forming a struct path * from a NULL struct file *, and then passes it to
bpf_d_path which uses struct path * without a NULL check, which causes
kernel crash without this fix.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
---
include/linux/bpf.h | 19 +++++++++++++++++++
kernel/bpf/btf.c | 7 +++++++
kernel/bpf/verifier.c | 10 ++++++++++
3 files changed, 36 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index d0ad379d1e62..17d4bbf69cb6 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -494,6 +494,23 @@ enum bpf_reg_type {
* assumptions about null or non-null when doing branch analysis.
* Further, when passed into helpers the helpers can not, without
* additional context, assume the value is non-null.
+ *
+ * All BPF helpers must use IS_PTR_TO_BTF_ID_ERR_OR_NULL when accepting
+ * a PTR_TO_BTF_ID or PTR_TO_BTF_ID_OR_NULL from a BPF program.
+ * Likewise, unstable kfuncs must do the same. This is because while
+ * PTR_TO_BTF_ID may be NULL at runtime, a pointer to the embedded
+ * object can be formed by adding the offset to the member, and then
+ * passing verifier checks because verifier thinks that:
+ *
+ * (struct file *)ptr + offsetof(struct file, f_path) == (struct path *)
+ *
+ * This logic in BTF ID check is needed to pass pointer to objects
+ * embedded in other objects user has pointer to, but in the NULL
+ * pointer case for PTR_TO_BTF_ID reg state, it will allow passing
+ * NULL + offset, which won't be rejected because it is not NULL.
+ *
+ * Hence, the IS_PTR_TO_BTF_ID_ERR_OR_NULL macro is needed to provide
+ * safety for both NULL and this 'non-NULL but invalid pointer case'.
*/
PTR_TO_BTF_ID,
/* PTR_TO_BTF_ID_OR_NULL points to a kernel struct that has not
@@ -520,6 +537,8 @@ enum bpf_reg_type {
};
static_assert(__BPF_REG_TYPE_MAX <= BPF_BASE_TYPE_LIMIT);
+#define IS_PTR_TO_BTF_ID_ERR_OR_NULL(p) ((unsigned long)(p) < PAGE_SIZE)
+
/* The information passed from prog-specific *_is_valid_access
* back to the verifier.
*/
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 9c8c429aa4dc..9db92b60ea9e 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5714,6 +5714,13 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
®_ref_id);
reg_ref_tname = btf_name_by_offset(reg_btf,
reg_ref_t->name_off);
+
+ if (reg->type == PTR_TO_BTF_ID && (reg->off < 0 || reg->off >= PAGE_SIZE)) {
+ bpf_log(log,
+ "R%d type=ptr_%s off=%d must be in range [0, %lu) when passing into kfunc\n",
+ regno, reg_ref_tname, reg->off, PAGE_SIZE);
+ }
+
/* In case of PTR_TO_SOCKET, PTR_TO_SOCK_COMMON,
* PTR_TO_TCP_SOCK, we do type check using BTF IDs of
* in-kernel types they point to, but
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 732dcba85ce5..90972bff1c54 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5257,6 +5257,16 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
arg_btf_id = compatible->btf_id;
}
+ /* reg->off may be < 0 here, as check_func_arg_reg_off is
+ * called later.
+ */
+ if (reg->off < 0 || reg->off >= PAGE_SIZE) {
+ verbose(env,
+ "R%d type=%s%s off=%d must be in range [0, %lu) when passing into helper\n",
+ regno, reg_type_str(env, reg->type), kernel_type_name(reg->btf, reg->btf_id),
+ reg->off, PAGE_SIZE);
+ }
+
if (!btf_struct_ids_match(&env->log, reg->btf, reg->btf_id, reg->off,
btf_vmlinux, *arg_btf_id)) {
verbose(env, "R%d is of type %s but %s is expected\n",
--
2.35.1
