Commit 8c4d3c42 authored by Daniel Borkmann's avatar Daniel Borkmann Committed by Xiaomeng Zhang
Browse files

bpf: Fix helper writes to read-only maps

stable inclusion
from stable-v6.6.54
commit a2c8dc7e21803257e762b0bf067fd13e9c995da0
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/stable/linux.git/commit/?id=a2c8dc7e2180



--------------------------------

[ Upstream commit 32556ce93bc45c730829083cb60f95a2728ea48b ]

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: 57c3bb72 ("bpf: Introduce ARG_PTR_TO_{INT,LONG} arg types")
Reported-by: default avatarLonial Con <kongln9170@gmail.com>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Acked-by: default avatarAndrii Nakryiko <andrii@kernel.org>
Acked-by: default avatarShung-Hsi Yu <shung-hsi.yu@suse.com>
Link: https://lore.kernel.org/r/20240913191754.13290-3-daniel@iogearbox.net


Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
Signed-off-by: default avatarXiaomeng Zhang <zhangxiaomeng13@huawei.com>
parent 95bc6947
Loading
Loading
Loading
Loading
+5 −2
Original line number Diff line number Diff line
@@ -685,6 +685,11 @@ enum bpf_type_flag {
	/* DYNPTR points to xdp_buff */
	DYNPTR_TYPE_XDP		= BIT(16 + 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,
};
@@ -721,8 +726,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_RINGBUF_MEM,	/* pointer to dynamically reserved ringbuf memory */
+4 −2
Original line number Diff line number Diff line
@@ -538,7 +538,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,
@@ -567,7 +568,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_3(bpf_strncmp, const char *, s1, u32, s1_sz, const char *, s2)
+2 −1
Original line number Diff line number Diff line
@@ -5713,7 +5713,8 @@ static const struct bpf_func_proto bpf_kallsyms_lookup_name_proto = {
	.arg1_type	= ARG_PTR_TO_MEM,
	.arg2_type	= ARG_CONST_SIZE_OR_ZERO,
	.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),
};

static const struct bpf_func_proto *
+5 −36
Original line number Diff line number Diff line
@@ -8086,16 +8086,6 @@ static bool arg_type_is_dynptr(enum bpf_arg_type type)
	return base_type(type) == ARG_PTR_TO_DYNPTR;
}
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)
@@ -8168,16 +8158,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_KEY,
		PTR_TO_MAP_VALUE,
	},
};
static const struct bpf_reg_types spin_lock_types = {
	.types = {
		PTR_TO_MAP_VALUE,
@@ -8232,8 +8212,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_RINGBUF_MEM]	= &ringbuf_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,
	[ARG_PTR_TO_FUNC]		= &func_ptr_types,
	[ARG_PTR_TO_STACK]		= &stack_ptr_types,
@@ -8740,9 +8718,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);
		}
		break;
	case ARG_CONST_SIZE:
@@ -8767,17 +8747,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
		if (err)
			return err;
		break;
	case ARG_PTR_TO_INT:
	case ARG_PTR_TO_LONG:
	{
		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);
		break;
	}
	case ARG_PTR_TO_CONST_STR:
	{
		struct bpf_map *map = reg->map_ptr;
+4 −2
Original line number Diff line number Diff line
@@ -1220,7 +1220,8 @@ static const struct bpf_func_proto bpf_get_func_arg_proto = {
	.ret_type	= RET_INTEGER,
	.arg1_type	= ARG_PTR_TO_CTX,
	.arg2_type	= ARG_ANYTHING,
	.arg3_type	= ARG_PTR_TO_LONG,
	.arg3_type	= ARG_PTR_TO_FIXED_SIZE_MEM | MEM_UNINIT | MEM_ALIGNED,
	.arg3_size	= sizeof(u64),
};

BPF_CALL_2(get_func_ret, void *, ctx, u64 *, value)
@@ -1236,7 +1237,8 @@ static const struct bpf_func_proto bpf_get_func_ret_proto = {
	.func		= get_func_ret,
	.ret_type	= RET_INTEGER,
	.arg1_type	= ARG_PTR_TO_CTX,
	.arg2_type	= ARG_PTR_TO_LONG,
	.arg2_type	= ARG_PTR_TO_FIXED_SIZE_MEM | MEM_UNINIT | MEM_ALIGNED,
	.arg2_size	= sizeof(u64),
};

BPF_CALL_1(get_func_arg_cnt, void *, ctx)
Loading