*** BLURB HERE ***
Daniel Borkmann (6): bpf: Fix bpf_strtol and bpf_strtoul helpers for 32bit bpf: Remove truncation test in bpf_strtol and bpf_strtoul helpers bpf: Fix helper writes to read-only maps bpf: Improve check_raw_mode_ok test for MEM_UNINIT-tagged types selftests/bpf: Fix ARG_PTR_TO_LONG {half-,}uninitialized test selftests/bpf: Rename ARG_PTR_TO_LONG test description
Joanne Koong (1): bpf: Add MEM_UNINIT as a bpf_type_flag
Maxim Mikityanskiy (1): bpf: Allow helpers to accept pointers with a fixed size
Xiaomeng Zhang (1): bpf: Fix kabi breakage in enum bpf_type_flag
include/linux/bpf.h | 39 ++++-- kernel/bpf/helpers.c | 18 ++- kernel/bpf/verifier.c | 114 ++++++++---------- .../testing/selftests/bpf/verifier/int_ptr.c | 15 +-- 4 files changed, 91 insertions(+), 95 deletions(-)
From: Joanne Koong joannelkoong@gmail.com
mainline inclusion from mainline-v5.19-rc1 commit 16d1e00c7e8a4950e914223b3112144289a82913 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAYQOP CVE: CVE-2024-49861
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
Instead of having uninitialized versions of arguments as separate bpf_arg_types (eg ARG_PTR_TO_UNINIT_MEM as the uninitialized version of ARG_PTR_TO_MEM), we can instead use MEM_UNINIT as a bpf_type_flag modifier to denote that the argument is uninitialized.
Doing so cleans up some of the logic in the verifier. We no longer need to do two checks against an argument type (eg "if (base_type(arg_type) == ARG_PTR_TO_MEM || base_type(arg_type) == ARG_PTR_TO_UNINIT_MEM)"), since uninitialized and initialized versions of the same argument type will now share the same base type.
In the near future, MEM_UNINIT will be used by dynptr helper functions as well.
Signed-off-by: Joanne Koong joannelkoong@gmail.com Acked-by: Andrii Nakryiko andrii@kernel.org Acked-by: David Vernet void@manifault.com Link: https://lore.kernel.org/r/20220509224257.3222614-2-joannelkoong@gmail.com Signed-off-by: Alexei Starovoitov ast@kernel.org Conflicts: include/linux/bpf.h kernel/bpf/helpers.c kernel/bpf/verifier.c [The conflicts were due to not merge some bpf_type_flag] Signed-off-by: Xiaomeng Zhang zhangxiaomeng13@huawei.com --- include/linux/bpf.h | 18 ++++++++++-------- kernel/bpf/helpers.c | 4 ++-- kernel/bpf/verifier.c | 28 ++++++++-------------------- 3 files changed, 20 insertions(+), 30 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index bd60c6f5f8e0..558b8f883a5d 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -290,7 +290,10 @@ enum bpf_type_flag { */ MEM_ALLOC = BIT(2 + BPF_BASE_TYPE_BITS),
- __BPF_TYPE_LAST_FLAG = MEM_ALLOC, + MEM_UNINIT = BIT(7 + BPF_BASE_TYPE_BITS), + + __BPF_TYPE_FLAG_MAX, + __BPF_TYPE_LAST_FLAG = __BPF_TYPE_FLAG_MAX - 1, };
/* Max number of base types. */ @@ -309,16 +312,11 @@ enum bpf_arg_type { ARG_CONST_MAP_PTR, /* const argument used as pointer to bpf_map */ ARG_PTR_TO_MAP_KEY, /* pointer to stack used as map key */ ARG_PTR_TO_MAP_VALUE, /* pointer to stack used as map value */ - ARG_PTR_TO_UNINIT_MAP_VALUE, /* pointer to valid memory used to store a map value */
- /* the following constraints used to prototype bpf_memcmp() and other - * functions that access data on eBPF program stack + /* Used to prototype bpf_memcmp() and other functions that access data + * on eBPF program stack */ ARG_PTR_TO_MEM, /* pointer to valid memory (stack, packet, map value) */ - ARG_PTR_TO_UNINIT_MEM, /* pointer to memory does not need to be initialized, - * helper function must fill all bytes or clear - * them in error case. - */
ARG_CONST_SIZE, /* number of bytes accessed from memory */ ARG_CONST_SIZE_OR_ZERO, /* number of bytes accessed from memory or 0 */ @@ -343,6 +341,10 @@ enum bpf_arg_type { ARG_PTR_TO_CTX_OR_NULL = PTR_MAYBE_NULL | ARG_PTR_TO_CTX, ARG_PTR_TO_SOCKET_OR_NULL = PTR_MAYBE_NULL | ARG_PTR_TO_SOCKET, ARG_PTR_TO_ALLOC_MEM_OR_NULL = PTR_MAYBE_NULL | ARG_PTR_TO_ALLOC_MEM, + /* pointer to memory does not need to be initialized, helper function must fill + * all bytes or clear them in error case. + */ + ARG_PTR_TO_UNINIT_MEM = MEM_UNINIT | ARG_PTR_TO_MEM,
/* This must be the last entry. Its purpose is to ensure the enum is * wide enough to hold the higher bits reserved for bpf_type_flag. diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index a13a8c6a69cb..327b9f76df26 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -101,7 +101,7 @@ const struct bpf_func_proto bpf_map_pop_elem_proto = { .gpl_only = false, .ret_type = RET_INTEGER, .arg1_type = ARG_CONST_MAP_PTR, - .arg2_type = ARG_PTR_TO_UNINIT_MAP_VALUE, + .arg2_type = ARG_PTR_TO_MAP_VALUE | MEM_UNINIT, };
BPF_CALL_2(bpf_map_peek_elem, struct bpf_map *, map, void *, value) @@ -114,7 +114,7 @@ const struct bpf_func_proto bpf_map_peek_elem_proto = { .gpl_only = false, .ret_type = RET_INTEGER, .arg1_type = ARG_CONST_MAP_PTR, - .arg2_type = ARG_PTR_TO_UNINIT_MAP_VALUE, + .arg2_type = ARG_PTR_TO_MAP_VALUE | MEM_UNINIT, };
const struct bpf_func_proto bpf_get_prandom_u32_proto = { diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index e1f8e3632880..68220a75d149 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -4334,12 +4334,6 @@ static int process_spin_lock(struct bpf_verifier_env *env, int regno, return 0; }
-static bool arg_type_is_mem_ptr(enum bpf_arg_type type) -{ - return base_type(type) == ARG_PTR_TO_MEM || - base_type(type) == ARG_PTR_TO_UNINIT_MEM; -} - static bool arg_type_is_mem_size(enum bpf_arg_type type) { return type == ARG_CONST_SIZE || @@ -4463,7 +4457,6 @@ static const struct bpf_reg_types percpu_btf_ptr_types = { .types = { PTR_TO_PER static const struct bpf_reg_types *compatible_reg_types[__BPF_ARG_TYPE_MAX] = { [ARG_PTR_TO_MAP_KEY] = &map_key_value_types, [ARG_PTR_TO_MAP_VALUE] = &map_key_value_types, - [ARG_PTR_TO_UNINIT_MAP_VALUE] = &map_key_value_types, [ARG_CONST_SIZE] = &scalar_types, [ARG_CONST_SIZE_OR_ZERO] = &scalar_types, [ARG_CONST_ALLOC_SIZE_OR_ZERO] = &scalar_types, @@ -4477,7 +4470,6 @@ static const struct bpf_reg_types *compatible_reg_types[__BPF_ARG_TYPE_MAX] = { [ARG_PTR_TO_BTF_ID] = &btf_ptr_types, [ARG_PTR_TO_SPIN_LOCK] = &spin_lock_types, [ARG_PTR_TO_MEM] = &mem_types, - [ARG_PTR_TO_UNINIT_MEM] = &mem_types, [ARG_PTR_TO_ALLOC_MEM] = &alloc_mem_types, [ARG_PTR_TO_INT] = &int_ptr_types, [ARG_PTR_TO_LONG] = &int_ptr_types, @@ -4583,8 +4575,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, return -EACCES; }
- if (base_type(arg_type) == ARG_PTR_TO_MAP_VALUE || - base_type(arg_type) == ARG_PTR_TO_UNINIT_MAP_VALUE) { + if (base_type(arg_type) == ARG_PTR_TO_MAP_VALUE) { err = resolve_map_arg_type(env, meta, &arg_type); if (err) return err; @@ -4659,8 +4650,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, err = check_helper_mem_access(env, regno, meta->map_ptr->key_size, false, NULL); - } else if (base_type(arg_type) == ARG_PTR_TO_MAP_VALUE || - base_type(arg_type) == ARG_PTR_TO_UNINIT_MAP_VALUE) { + } else if (base_type(arg_type) == ARG_PTR_TO_MAP_VALUE) { if (type_may_be_null(arg_type) && register_is_null(reg)) return 0;
@@ -4672,7 +4662,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, verbose(env, "invalid map_ptr to access map->value\n"); return -EACCES; } - meta->raw_mode = (arg_type == ARG_PTR_TO_UNINIT_MAP_VALUE); + meta->raw_mode = arg_type & MEM_UNINIT; err = check_helper_mem_access(env, regno, meta->map_ptr->value_size, false, meta); @@ -4693,11 +4683,11 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, verbose(env, "verifier internal error\n"); return -EFAULT; } - } else if (arg_type_is_mem_ptr(arg_type)) { + } else if (base_type(arg_type) == ARG_PTR_TO_MEM) { /* The access to this pointer is only checked when we hit the * next is_mem_size argument below. */ - meta->raw_mode = (arg_type == ARG_PTR_TO_UNINIT_MEM); + meta->raw_mode = arg_type & MEM_UNINIT; } else if (arg_type_is_mem_size(arg_type)) { bool zero_size_allowed = (arg_type == ARG_CONST_SIZE_OR_ZERO);
@@ -5030,10 +5020,8 @@ static bool check_raw_mode_ok(const struct bpf_func_proto *fn) static bool check_args_pair_invalid(enum bpf_arg_type arg_curr, enum bpf_arg_type arg_next) { - return (arg_type_is_mem_ptr(arg_curr) && - !arg_type_is_mem_size(arg_next)) || - (!arg_type_is_mem_ptr(arg_curr) && - arg_type_is_mem_size(arg_next)); + return (base_type(arg_curr) == ARG_PTR_TO_MEM) != + arg_type_is_mem_size(arg_next); }
static bool check_arg_pair_ok(const struct bpf_func_proto *fn) @@ -5044,7 +5032,7 @@ static bool check_arg_pair_ok(const struct bpf_func_proto *fn) * helper function specification. */ if (arg_type_is_mem_size(fn->arg1_type) || - arg_type_is_mem_ptr(fn->arg5_type) || + base_type(fn->arg5_type) == ARG_PTR_TO_MEM || check_args_pair_invalid(fn->arg1_type, fn->arg2_type) || check_args_pair_invalid(fn->arg2_type, fn->arg3_type) || check_args_pair_invalid(fn->arg3_type, fn->arg4_type) ||
From: Maxim Mikityanskiy maximmi@nvidia.com
mainline inclusion from mainline-v6.0-rc1 commit 508362ac66b0478affb4e52cb8da98478312d72d category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAYQOP CVE: CVE-2024-49861
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
Before this commit, the BPF verifier required ARG_PTR_TO_MEM arguments to be followed by ARG_CONST_SIZE holding the size of the memory region. The helpers had to check that size in runtime.
There are cases where the size expected by a helper is a compile-time constant. Checking it in runtime is an unnecessary overhead and waste of BPF registers.
This commit allows helpers to accept pointers to memory without the corresponding ARG_CONST_SIZE, given that they define the memory region size in struct bpf_func_proto and use ARG_PTR_TO_FIXED_SIZE_MEM type.
arg_size is unionized with arg_btf_id to reduce the kernel image size, and it's valid because they are used by different argument types.
Signed-off-by: Maxim Mikityanskiy maximmi@nvidia.com Reviewed-by: Tariq Toukan tariqt@nvidia.com Link: https://lore.kernel.org/r/20220615134847.3753567-3-maximmi@nvidia.com Signed-off-by: Alexei Starovoitov ast@kernel.org Conflicts: include/linux/bpf.h kernel/bpf/verifier.c [The conflicts were due to not merge commit c0a5a21c25f37] Signed-off-by: Xiaomeng Zhang zhangxiaomeng13@huawei.com --- include/linux/bpf.h | 13 +++++++++++++ kernel/bpf/verifier.c | 43 ++++++++++++++++++++++++++++++++----------- 2 files changed, 45 insertions(+), 11 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 558b8f883a5d..6347b7d5b678 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -292,6 +292,9 @@ enum bpf_type_flag {
MEM_UNINIT = BIT(7 + BPF_BASE_TYPE_BITS),
+ /* Size is known at compile time. */ + MEM_FIXED_SIZE = BIT(10 + BPF_BASE_TYPE_BITS), + __BPF_TYPE_FLAG_MAX, __BPF_TYPE_LAST_FLAG = __BPF_TYPE_FLAG_MAX - 1, }; @@ -345,6 +348,8 @@ enum bpf_arg_type { * all bytes or clear them in error case. */ ARG_PTR_TO_UNINIT_MEM = MEM_UNINIT | ARG_PTR_TO_MEM, + /* Pointer to valid memory of size known at compile time. */ + ARG_PTR_TO_FIXED_SIZE_MEM = MEM_FIXED_SIZE | ARG_PTR_TO_MEM,
/* This must be the last entry. Its purpose is to ensure the enum is * wide enough to hold the higher bits reserved for bpf_type_flag. @@ -409,6 +414,14 @@ struct bpf_func_proto { u32 *arg5_btf_id; }; u32 *arg_btf_id[5]; + struct { + size_t arg1_size; + size_t arg2_size; + size_t arg3_size; + size_t arg4_size; + size_t arg5_size; + }; + size_t arg_size[5]; }; int *ret_btf_id; /* return value btf_id */ bool (*allowed)(const struct bpf_prog *prog); diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 68220a75d149..6d6f84956016 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -4551,6 +4551,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, struct bpf_reg_state *regs = cur_regs(env), *reg = ®s[regno]; enum bpf_arg_type arg_type = fn->arg_type[arg]; enum bpf_reg_type type = reg->type; + u32 *arg_btf_id = NULL; int err = 0;
if (arg_type == ARG_DONTCARE) @@ -4587,7 +4588,11 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, */ goto skip_type_check;
- err = check_reg_type(env, regno, arg_type, fn->arg_btf_id[arg]); + /* arg_btf_id and arg_size are in a union. */ + if (base_type(arg_type) == ARG_PTR_TO_BTF_ID) + arg_btf_id = fn->arg_btf_id[arg]; + + err = check_reg_type(env, regno, arg_type, arg_btf_id); if (err) return err;
@@ -4688,6 +4693,11 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, * next is_mem_size argument below. */ meta->raw_mode = arg_type & MEM_UNINIT; + if (arg_type & MEM_FIXED_SIZE) { + err = check_helper_mem_access(env, regno, + fn->arg_size[arg], false, + meta); + } } else if (arg_type_is_mem_size(arg_type)) { bool zero_size_allowed = (arg_type == ARG_CONST_SIZE_OR_ZERO);
@@ -5017,11 +5027,19 @@ static bool check_raw_mode_ok(const struct bpf_func_proto *fn) return count <= 1; }
-static bool check_args_pair_invalid(enum bpf_arg_type arg_curr, - enum bpf_arg_type arg_next) +static bool check_args_pair_invalid(const struct bpf_func_proto *fn, int arg) { - return (base_type(arg_curr) == ARG_PTR_TO_MEM) != - arg_type_is_mem_size(arg_next); + bool is_fixed = fn->arg_type[arg] & MEM_FIXED_SIZE; + bool has_size = fn->arg_size[arg] != 0; + bool is_next_size = false; + + if (arg + 1 < ARRAY_SIZE(fn->arg_type)) + is_next_size = arg_type_is_mem_size(fn->arg_type[arg + 1]); + + if (base_type(fn->arg_type[arg]) != ARG_PTR_TO_MEM) + return is_next_size; + + return has_size == is_next_size || is_next_size == is_fixed; }
static bool check_arg_pair_ok(const struct bpf_func_proto *fn) @@ -5032,11 +5050,11 @@ static bool check_arg_pair_ok(const struct bpf_func_proto *fn) * helper function specification. */ if (arg_type_is_mem_size(fn->arg1_type) || - base_type(fn->arg5_type) == ARG_PTR_TO_MEM || - check_args_pair_invalid(fn->arg1_type, fn->arg2_type) || - check_args_pair_invalid(fn->arg2_type, fn->arg3_type) || - check_args_pair_invalid(fn->arg3_type, fn->arg4_type) || - check_args_pair_invalid(fn->arg4_type, fn->arg5_type)) + check_args_pair_invalid(fn, 0) || + check_args_pair_invalid(fn, 1) || + check_args_pair_invalid(fn, 2) || + check_args_pair_invalid(fn, 3) || + check_args_pair_invalid(fn, 4)) return false;
return true; @@ -5077,7 +5095,10 @@ static bool check_btf_id_ok(const struct bpf_func_proto *fn) if (base_type(fn->arg_type[i]) == ARG_PTR_TO_BTF_ID && !fn->arg_btf_id[i]) return false;
- if (base_type(fn->arg_type[i]) != ARG_PTR_TO_BTF_ID && fn->arg_btf_id[i]) + if (base_type(fn->arg_type[i]) != ARG_PTR_TO_BTF_ID && fn->arg_btf_id[i] && + /* arg_btf_id and arg_size are in a union. */ + (base_type(fn->arg_type[i]) != ARG_PTR_TO_MEM || + !(fn->arg_type[i] & MEM_FIXED_SIZE))) return false; }
From: Daniel Borkmann daniel@iogearbox.net
mainline inclusion from mainline-v6.12-rc1 commit cfe69c50b05510b24e26ccb427c7cc70beafd6c1 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAYQOP CVE: CVE-2024-49861
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
The bpf_strtol() and bpf_strtoul() helpers are currently broken on 32bit:
The argument type ARG_PTR_TO_LONG is BPF-side "long", not kernel-side "long" and therefore always considered fixed 64bit no matter if 64 or 32bit underlying architecture.
This contract breaks in case of the two mentioned helpers since their BPF_CALL definition for the helpers was added with {unsigned,}long *res. Meaning, the transition from BPF-side "long" (BPF program) to kernel-side "long" (BPF helper) breaks here.
Both helpers call __bpf_strtoll() with "long long" correctly, but later assigning the result into 32-bit "*(long *)" on 32bit architectures. From a BPF program point of view, this means upper bits will be seen as uninitialised.
Therefore, fix both BPF_CALL signatures to {s,u}64 types to fix this situation.
Now, changing also uapi/bpf.h helper documentation which generates bpf_helper_defs.h for BPF programs is tricky: Changing signatures there to __{s,u}64 would trigger compiler warnings (incompatible pointer types passing 'long *' to parameter of type '__s64 *' (aka 'long long *')) for existing BPF programs.
Leaving the signatures as-is would be fine as from BPF program point of view it is still BPF-side "long" and thus equivalent to __{s,u}64 on 64 or 32bit underlying architectures.
Note that bpf_strtol() and bpf_strtoul() are the only helpers with this issue.
Fixes: d7a4cb9b6705 ("bpf: Introduce bpf_strtol and bpf_strtoul helpers") Reported-by: Alexei Starovoitov ast@kernel.org Signed-off-by: Daniel Borkmann daniel@iogearbox.net Acked-by: Andrii Nakryiko andrii@kernel.org Link: https://lore.kernel.org/bpf/481fcec8-c12c-9abb-8ecb-76c71c009959@iogearbox.n... Link: https://lore.kernel.org/r/20240913191754.13290-1-daniel@iogearbox.net Signed-off-by: Alexei Starovoitov ast@kernel.org Signed-off-by: Xiaomeng Zhang zhangxiaomeng13@huawei.com --- kernel/bpf/helpers.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 327b9f76df26..8216eb2dbb41 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -484,7 +484,7 @@ static int __bpf_strtoll(const char *buf, size_t buf_len, u64 flags, }
BPF_CALL_4(bpf_strtol, const char *, buf, size_t, buf_len, u64, flags, - long *, res) + s64 *, res) { long long _res; int err; @@ -510,7 +510,7 @@ const struct bpf_func_proto bpf_strtol_proto = { };
BPF_CALL_4(bpf_strtoul, const char *, buf, size_t, buf_len, u64, flags, - unsigned long *, res) + u64 *, res) { unsigned long long _res; bool is_negative;
From: Daniel Borkmann daniel@iogearbox.net
mainline inclusion from mainline-v6.12-rc1 commit 7d71f59e028028f1160602121f40f45e89b3664e category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAYQOP CVE: CVE-2024-49861
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
Both bpf_strtol() and bpf_strtoul() helpers passed a temporary "long long" respectively "unsigned long long" to __bpf_strtoll() / __bpf_strtoull().
Later, the result was checked for truncation via _res != ({unsigned,} long)_res as the destination buffer for the BPF helpers was of type {unsigned,} long which is 32bit on 32bit architectures.
Given the latter was a bug in the helper signatures where the destination buffer got adjusted to {s,u}64, the truncation check can now be removed.
Signed-off-by: Daniel Borkmann daniel@iogearbox.net Acked-by: Andrii Nakryiko andrii@kernel.org Link: https://lore.kernel.org/r/20240913191754.13290-2-daniel@iogearbox.net Signed-off-by: Alexei Starovoitov ast@kernel.org Signed-off-by: Xiaomeng Zhang zhangxiaomeng13@huawei.com --- kernel/bpf/helpers.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 8216eb2dbb41..25b15ab75e84 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -493,8 +493,6 @@ BPF_CALL_4(bpf_strtol, const char *, buf, size_t, buf_len, u64, flags, err = __bpf_strtoll(buf, buf_len, flags, &_res); if (err < 0) return err; - if (_res != (long)_res) - return -ERANGE; *res = _res; return err; } @@ -522,8 +520,6 @@ BPF_CALL_4(bpf_strtoul, const char *, buf, size_t, buf_len, u64, flags, return err; if (is_negative) return -EINVAL; - if (_res != (unsigned long)_res) - return -ERANGE; *res = _res; return err; }
From: Daniel Borkmann daniel@iogearbox.net
mainline inclusion from mainline-v6.12-rc1 commit 32556ce93bc45c730829083cb60f95a2728ea48b category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAYQOP CVE: CVE-2024-49861
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
Lonial found an issue that despite user- and BPF-side frozen BPF map (like in case of .rodata), it was still possible to write into it from a BPF program side through specific helpers having ARG_PTR_TO_{LONG,INT} as arguments.
In check_func_arg() when the argument is as mentioned, the meta->raw_mode is never set. Later, check_helper_mem_access(), under the case of PTR_TO_MAP_VALUE as register base type, it assumes BPF_READ for the subsequent call to check_map_access_type() and given the BPF map is read-only it succeeds.
The helpers really need to be annotated as ARG_PTR_TO_{LONG,INT} | MEM_UNINIT when results are written into them as opposed to read out of them. The latter indicates that it's okay to pass a pointer to uninitialized memory as the memory is written to anyway.
However, ARG_PTR_TO_{LONG,INT} is a special case of ARG_PTR_TO_FIXED_SIZE_MEM just with additional alignment requirement. So it is better to just get rid of the ARG_PTR_TO_{LONG,INT} special cases altogether and reuse the fixed size memory types. For this, add MEM_ALIGNED to additionally ensure alignment given these helpers write directly into the args via *<ptr> = val. The .arg*_size has been initialized reflecting the actual sizeof(*<ptr>).
MEM_ALIGNED can only be used in combination with MEM_FIXED_SIZE annotated argument types, since in !MEM_FIXED_SIZE cases the verifier does not know the buffer size a priori and therefore cannot blindly write *<ptr> = val.
Fixes: 57c3bb725a3d ("bpf: Introduce ARG_PTR_TO_{INT,LONG} arg types") Reported-by: Lonial Con kongln9170@gmail.com Signed-off-by: Daniel Borkmann daniel@iogearbox.net Acked-by: Andrii Nakryiko andrii@kernel.org Acked-by: Shung-Hsi Yu shung-hsi.yu@suse.com Link: https://lore.kernel.org/r/20240913191754.13290-3-daniel@iogearbox.net Signed-off-by: Alexei Starovoitov ast@kernel.org Conflicts: net/core/filter.c include/linux/bpf.h kernel/bpf/helpers.c kernel/bpf/syscall.c kernel/bpf/verifier.c kernel/trace/bpf_trace.c [The conflicts were due to not merge some bpf_type_flag and bpf helpers] Signed-off-by: Xiaomeng Zhang zhangxiaomeng13@huawei.com --- include/linux/bpf.h | 7 +++++-- kernel/bpf/helpers.c | 6 ++++-- kernel/bpf/verifier.c | 41 +++++------------------------------------ 3 files changed, 14 insertions(+), 40 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 6347b7d5b678..4c169f0ff6ab 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -295,6 +295,11 @@ enum bpf_type_flag { /* Size is known at compile time. */ MEM_FIXED_SIZE = BIT(10 + BPF_BASE_TYPE_BITS),
+ /* Memory must be aligned on some architectures, used in combination with + * MEM_FIXED_SIZE. + */ + MEM_ALIGNED = BIT(17 + BPF_BASE_TYPE_BITS), + __BPF_TYPE_FLAG_MAX, __BPF_TYPE_LAST_FLAG = __BPF_TYPE_FLAG_MAX - 1, }; @@ -328,8 +333,6 @@ enum bpf_arg_type { ARG_ANYTHING, /* any (initialized) argument is ok */ ARG_PTR_TO_SPIN_LOCK, /* pointer to bpf_spin_lock */ ARG_PTR_TO_SOCK_COMMON, /* pointer to sock_common */ - ARG_PTR_TO_INT, /* pointer to int */ - ARG_PTR_TO_LONG, /* pointer to long */ ARG_PTR_TO_SOCKET, /* pointer to bpf_sock (fullsock) */ ARG_PTR_TO_BTF_ID, /* pointer to in-kernel struct */ ARG_PTR_TO_ALLOC_MEM, /* pointer to dynamically allocated memory */ diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 25b15ab75e84..1fbda5c87a29 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -504,7 +504,8 @@ const struct bpf_func_proto bpf_strtol_proto = { .arg1_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg2_type = ARG_CONST_SIZE, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_LONG, + .arg4_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_UNINIT | MEM_ALIGNED, + .arg4_size = sizeof(s64), };
BPF_CALL_4(bpf_strtoul, const char *, buf, size_t, buf_len, u64, flags, @@ -531,7 +532,8 @@ const struct bpf_func_proto bpf_strtoul_proto = { .arg1_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg2_type = ARG_CONST_SIZE, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_LONG, + .arg4_type = ARG_PTR_TO_FIXED_SIZE_MEM | MEM_UNINIT | MEM_ALIGNED, + .arg4_size = sizeof(u64), };
BPF_CALL_4(bpf_get_ns_current_pid_tgid, u64, dev, u64, ino, diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 6d6f84956016..79628bd1efbe 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -4345,22 +4345,6 @@ static bool arg_type_is_alloc_size(enum bpf_arg_type type) return type == ARG_CONST_ALLOC_SIZE_OR_ZERO; }
-static bool arg_type_is_int_ptr(enum bpf_arg_type type) -{ - return type == ARG_PTR_TO_INT || - type == ARG_PTR_TO_LONG; -} - -static int int_ptr_type_to_size(enum bpf_arg_type type) -{ - if (type == ARG_PTR_TO_INT) - return sizeof(u32); - else if (type == ARG_PTR_TO_LONG) - return sizeof(u64); - - return -EINVAL; -} - static int resolve_map_arg_type(struct bpf_verifier_env *env, const struct bpf_call_arg_meta *meta, enum bpf_arg_type *arg_type) @@ -4436,14 +4420,6 @@ static const struct bpf_reg_types mem_types = { }, };
-static const struct bpf_reg_types int_ptr_types = { - .types = { - PTR_TO_STACK, - PTR_TO_PACKET, - PTR_TO_PACKET_META, - PTR_TO_MAP_VALUE, - }, -};
static const struct bpf_reg_types fullsock_types = { .types = { PTR_TO_SOCKET } }; static const struct bpf_reg_types scalar_types = { .types = { SCALAR_VALUE } }; @@ -4471,8 +4447,6 @@ static const struct bpf_reg_types *compatible_reg_types[__BPF_ARG_TYPE_MAX] = { [ARG_PTR_TO_SPIN_LOCK] = &spin_lock_types, [ARG_PTR_TO_MEM] = &mem_types, [ARG_PTR_TO_ALLOC_MEM] = &alloc_mem_types, - [ARG_PTR_TO_INT] = &int_ptr_types, - [ARG_PTR_TO_LONG] = &int_ptr_types, [ARG_PTR_TO_PERCPU_BTF_ID] = &percpu_btf_ptr_types, };
@@ -4694,9 +4668,11 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, */ meta->raw_mode = arg_type & MEM_UNINIT; if (arg_type & MEM_FIXED_SIZE) { - err = check_helper_mem_access(env, regno, - fn->arg_size[arg], false, - meta); + err = check_helper_mem_access(env, regno, fn->arg_size[arg], false, meta); + if (err) + return err; + if (arg_type & MEM_ALIGNED) + err = check_ptr_alignment(env, reg, 0, fn->arg_size[arg], true); } } else if (arg_type_is_mem_size(arg_type)) { bool zero_size_allowed = (arg_type == ARG_CONST_SIZE_OR_ZERO); @@ -4753,13 +4729,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, return -EACCES; } meta->mem_size = reg->var_off.value; - } else if (arg_type_is_int_ptr(arg_type)) { - int size = int_ptr_type_to_size(arg_type); - - err = check_helper_mem_access(env, regno, size, false, meta); - if (err) - return err; - err = check_ptr_alignment(env, reg, 0, size, true); }
return err;
From: Daniel Borkmann daniel@iogearbox.net
mainline inclusion from mainline-v6.12-rc1 commit 18752d73c1898fd001569195ba4b0b8c43255f4a category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAYQOP CVE: CVE-2024-49861
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
When checking malformed helper function signatures, also take other argument types into account aside from just ARG_PTR_TO_UNINIT_MEM.
This concerns (formerly) ARG_PTR_TO_{INT,LONG} given uninitialized memory can be passed there, too.
The func proto sanity check goes back to commit 435faee1aae9 ("bpf, verifier: add ARG_PTR_TO_RAW_STACK type"), and its purpose was to detect wrong func protos which had more than just one MEM_UNINIT-tagged type as arguments.
The reason more than one is currently not supported is as we mark stack slots with STACK_MISC in check_helper_call() in case of raw mode based on meta.access_size to allow uninitialized stack memory to be passed to helpers when they just write into the buffer.
Probing for base type as well as MEM_UNINIT tagging ensures that other types do not get missed (as it used to be the case for ARG_PTR_TO_{INT,LONG}).
Fixes: 57c3bb725a3d ("bpf: Introduce ARG_PTR_TO_{INT,LONG} arg types") Reported-by: Shung-Hsi Yu shung-hsi.yu@suse.com Signed-off-by: Daniel Borkmann daniel@iogearbox.net Acked-by: Andrii Nakryiko andrii@kernel.org Acked-by: Shung-Hsi Yu shung-hsi.yu@suse.com Link: https://lore.kernel.org/r/20240913191754.13290-4-daniel@iogearbox.net Signed-off-by: Alexei Starovoitov ast@kernel.org Conflicts: kernel/bpf/verifier.c [The conflicts were due to some minor issue] Signed-off-by: Xiaomeng Zhang zhangxiaomeng13@huawei.com --- kernel/bpf/verifier.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 79628bd1efbe..c1a03ab632bb 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -4340,6 +4340,12 @@ static bool arg_type_is_mem_size(enum bpf_arg_type type) type == ARG_CONST_SIZE_OR_ZERO; }
+static bool arg_type_is_raw_mem(enum bpf_arg_type type) +{ + return base_type(type) == ARG_PTR_TO_MEM && + type & MEM_UNINIT; +} + static bool arg_type_is_alloc_size(enum bpf_arg_type type) { return type == ARG_CONST_ALLOC_SIZE_OR_ZERO; @@ -4978,15 +4984,15 @@ static bool check_raw_mode_ok(const struct bpf_func_proto *fn) { int count = 0;
- if (fn->arg1_type == ARG_PTR_TO_UNINIT_MEM) + if (arg_type_is_raw_mem(fn->arg1_type)) count++; - if (fn->arg2_type == ARG_PTR_TO_UNINIT_MEM) + if (arg_type_is_raw_mem(fn->arg2_type)) count++; - if (fn->arg3_type == ARG_PTR_TO_UNINIT_MEM) + if (arg_type_is_raw_mem(fn->arg3_type)) count++; - if (fn->arg4_type == ARG_PTR_TO_UNINIT_MEM) + if (arg_type_is_raw_mem(fn->arg4_type)) count++; - if (fn->arg5_type == ARG_PTR_TO_UNINIT_MEM) + if (arg_type_is_raw_mem(fn->arg5_type)) count++;
/* We only support one arg being in raw mode at the moment,
From: Daniel Borkmann daniel@iogearbox.net
mainline inclusion from mainline-v6.12-rc1 commit b8e188f023e07a733b47d5865311ade51878fe40 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAYQOP CVE: CVE-2024-49861
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
The assumption of 'in privileged mode reads from uninitialized stack locations are permitted' is not quite correct since the verifier was probing for read access rather than write access. Both tests need to be annotated as __success for privileged and unprivileged.
Signed-off-by: Daniel Borkmann daniel@iogearbox.net Acked-by: Andrii Nakryiko andrii@kernel.org Link: https://lore.kernel.org/r/20240913191754.13290-6-daniel@iogearbox.net Signed-off-by: Alexei Starovoitov ast@kernel.org Conflicts: tools/testing/selftests/bpf/verifier/int_ptr.c tools/testing/selftests/bpf/progs/verifier_int_ptr.c [The conflicts were due to test files changed] Signed-off-by: Xiaomeng Zhang zhangxiaomeng13@huawei.com --- tools/testing/selftests/bpf/verifier/int_ptr.c | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/verifier/int_ptr.c b/tools/testing/selftests/bpf/verifier/int_ptr.c index c28cd2b8f1da..fd7bf5d4a35b 100644 --- a/tools/testing/selftests/bpf/verifier/int_ptr.c +++ b/tools/testing/selftests/bpf/verifier/int_ptr.c @@ -27,8 +27,6 @@ }, .result = ACCEPT, .retval = POINTER_VALUE, - .errstr_unpriv = "invalid indirect read from stack R4 off -16+0 size 8", - .result_unpriv = REJECT, }, { "ARG_PTR_TO_LONG half-uninitialized", @@ -58,9 +56,6 @@ BPF_MOV64_IMM(BPF_REG_0, 0), BPF_EXIT_INSN(), }, - .result_unpriv = REJECT, - .errstr_unpriv = "invalid indirect read from stack R4 off -16+4 size 8", - /* in privileged mode reads from uninitialized stack locations are permitted */ .result = ACCEPT, }, {
From: Daniel Borkmann daniel@iogearbox.net
mainline inclusion from mainline-v6.12-rc1 commit b073b82d4d3cee703d17e099c7e739082f15bd2e category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAYQOP CVE: CVE-2024-49861
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
Given we got rid of ARG_PTR_TO_LONG, change the test case description to avoid potential confusion:
# ./vmtest.sh -- ./test_progs -t verifier_int_ptr [...] ./test_progs -t verifier_int_ptr [ 1.610563] bpf_testmod: loading out-of-tree module taints kernel. [ 1.611049] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel #489/1 verifier_int_ptr/arg pointer to long uninitialized:OK #489/2 verifier_int_ptr/arg pointer to long half-uninitialized:OK #489/3 verifier_int_ptr/arg pointer to long misaligned:OK #489/4 verifier_int_ptr/arg pointer to long size < sizeof(long):OK #489/5 verifier_int_ptr/arg pointer to long initialized:OK #489 verifier_int_ptr:OK Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED
Signed-off-by: Daniel Borkmann daniel@iogearbox.net Link: https://lore.kernel.org/r/20240913191754.13290-7-daniel@iogearbox.net Signed-off-by: Alexei Starovoitov ast@kernel.org Conflicts: tools/testing/selftests/bpf/verifier/int_ptr.c tools/testing/selftests/bpf/progs/verifier_int_ptr.c [The conflicts were due to test files changed] Signed-off-by: Xiaomeng Zhang zhangxiaomeng13@huawei.com --- tools/testing/selftests/bpf/verifier/int_ptr.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/verifier/int_ptr.c b/tools/testing/selftests/bpf/verifier/int_ptr.c index fd7bf5d4a35b..cd881ec2688f 100644 --- a/tools/testing/selftests/bpf/verifier/int_ptr.c +++ b/tools/testing/selftests/bpf/verifier/int_ptr.c @@ -1,5 +1,5 @@ { - "ARG_PTR_TO_LONG uninitialized", + "arg pointer to long uninitialized", .insns = { /* bpf_strtoul arg1 (buf) */ BPF_MOV64_REG(BPF_REG_7, BPF_REG_10), @@ -29,7 +29,7 @@ .retval = POINTER_VALUE, }, { - "ARG_PTR_TO_LONG half-uninitialized", + "arg pointer to long half-uninitialized", .insns = { /* bpf_strtoul arg1 (buf) */ BPF_MOV64_REG(BPF_REG_7, BPF_REG_10), @@ -59,7 +59,7 @@ .result = ACCEPT, }, { - "ARG_PTR_TO_LONG misaligned", + "arg pointer to long misaligned", .insns = { /* bpf_strtoul arg1 (buf) */ BPF_MOV64_REG(BPF_REG_7, BPF_REG_10), @@ -93,7 +93,7 @@ .errstr = "misaligned stack access off (0x0; 0x0)+-20+0 size 8", }, { - "ARG_PTR_TO_LONG size < sizeof(long)", + "arg pointer to long size < sizeof(long)", .insns = { /* bpf_strtoul arg1 (buf) */ BPF_MOV64_REG(BPF_REG_7, BPF_REG_10), @@ -125,7 +125,7 @@ .errstr = "invalid indirect access to stack R4 off=-4 size=8", }, { - "ARG_PTR_TO_LONG initialized", + "arg pointer to long initialized", .insns = { /* bpf_strtoul arg1 (buf) */ BPF_MOV64_REG(BPF_REG_7, BPF_REG_10),
Offering: HULK hulk inclusion category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAYQOP CVE: CVE-2024-49861
--------------------------------
enum bpf_type_flag is used only in kernel. Let's fix this kabi breakage by adding KABI_BROKEN_INSERT_ENUM and KABI_BROKEN_INSERT_ENUM macro.
Fixes: 656830f06678 ("[Backport] bpf: Add MEM_UNINIT as a bpf_type_flag") Fixes: 834a586818e5 ("[Backport] bpf: Allow helpers to accept pointers with a fixed size") Fixes: ec15a06019fe ("[Backport] bpf: Fix helper writes to read-only maps") Signed-off-by: Xiaomeng Zhang zhangxiaomeng13@huawei.com --- include/linux/bpf.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 4c169f0ff6ab..d709e32e841b 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -290,18 +290,19 @@ enum bpf_type_flag { */ MEM_ALLOC = BIT(2 + BPF_BASE_TYPE_BITS),
- MEM_UNINIT = BIT(7 + BPF_BASE_TYPE_BITS), + KABI_BROKEN_INSERT_ENUM(MEM_UNINIT = BIT(7 + BPF_BASE_TYPE_BITS))
/* Size is known at compile time. */ - MEM_FIXED_SIZE = BIT(10 + BPF_BASE_TYPE_BITS), + KABI_BROKEN_INSERT_ENUM(MEM_FIXED_SIZE = BIT(10 + BPF_BASE_TYPE_BITS))
/* Memory must be aligned on some architectures, used in combination with * MEM_FIXED_SIZE. */ - MEM_ALIGNED = BIT(17 + BPF_BASE_TYPE_BITS), + KABI_BROKEN_INSERT_ENUM(MEM_ALIGNED = BIT(17 + BPF_BASE_TYPE_BITS))
- __BPF_TYPE_FLAG_MAX, - __BPF_TYPE_LAST_FLAG = __BPF_TYPE_FLAG_MAX - 1, + KABI_BROKEN_INSERT_ENUM(__BPF_TYPE_FLAG_MAX) + KABI_BROKEN_REMOVE_ENUM(__BPF_TYPE_LAST_FLAG = MEM_ALLOC) + KABI_BROKEN_INSERT_ENUM(__BPF_TYPE_LAST_FLAG = __BPF_TYPE_FLAG_MAX - 1) };
/* Max number of base types. */
-
Xiaomeng Zhang