| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 21 May 2018 14:04:51 -0700
From: Yonghong Song <yhs@...com>
To: Martin KaFai Lau <kafai@...com>, <netdev@...r.kernel.org>
CC: Alexei Starovoitov <ast@...com>,
Daniel Borkmann <daniel@...earbox.net>, <kernel-team@...com>
Subject: Re: [PATCH bpf-next 3/7] bpf: btf: Check array->index_type
On 5/18/18 5:16 PM, Martin KaFai Lau wrote:
> Instead of ingoring the array->index_type field. Enforce that
> it must be an unsigned BTF_KIND_INT.
>
> Signed-off-by: Martin KaFai Lau <kafai@...com>
> ---
> kernel/bpf/btf.c | 83 ++++++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 59 insertions(+), 24 deletions(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 536e5981ad8c..b4e48dae2240 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -444,6 +444,28 @@ static const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id)
> return btf->types[type_id];
> }
>
> +/*
> + * Regular int is not a bit field and it must be either
> + * u8/u16/u32/u64.
> + */
> +static bool btf_type_int_is_regular(const struct btf_type *t)
> +{
> + u16 nr_bits, nr_bytes;
> + u32 int_data;
> +
> + int_data = btf_type_int(t);
> + nr_bits = BTF_INT_BITS(int_data);
> + nr_bytes = BITS_ROUNDUP_BYTES(nr_bits);
> + if (BITS_PER_BYTE_MASKED(nr_bits) ||
> + BTF_INT_OFFSET(int_data) ||
> + (nr_bytes != sizeof(u8) && nr_bytes != sizeof(u16) &&
> + nr_bytes != sizeof(u32) && nr_bytes != sizeof(u64))) {
> + return false;
> + }
> +
> + return true;
> +}
> +
> __printf(2, 3) static void __btf_verifier_log(struct bpf_verifier_log *log,
> const char *fmt, ...)
> {
> @@ -1309,14 +1331,16 @@ static s32 btf_array_check_meta(struct btf_verifier_env *env,
> return -EINVAL;
> }
>
> - /* We are a little forgiving on array->index_type since
> - * the kernel is not using it.
> - */
> - /* Array elem cannot be in type void,
> - * so !array->type is not allowed.
> + /* Array elem type and index type cannot be in type void,
> + * so !array->type and !array->index_type are not allowed.
> */
> if (!array->type || BTF_TYPE_PARENT(array->type)) {
> - btf_verifier_log_type(env, t, "Invalid type_id");
> + btf_verifier_log_type(env, t, "Invalid elem");
> + return -EINVAL;
> + }
> +
> + if (!array->index_type || BTF_TYPE_PARENT(array->index_type)) {
> + btf_verifier_log_type(env, t, "Invalid index");
> return -EINVAL;
> }
>
> @@ -1329,11 +1353,35 @@ static int btf_array_resolve(struct btf_verifier_env *env,
> const struct resolve_vertex *v)
> {
> const struct btf_array *array = btf_type_array(v->t);
> - const struct btf_type *elem_type;
> - u32 elem_type_id = array->type;
> + const struct btf_type *elem_type, *index_type;
> + u32 elem_type_id, index_type_id;
> struct btf *btf = env->btf;
> u32 elem_size;
>
> + /* Check array->index_type */
> + index_type_id = array->index_type;
> + index_type = btf_type_by_id(btf, index_type_id);
> + if (btf_type_is_void_or_null(index_type)) {
> + btf_verifier_log_type(env, v->t, "Invalid index");
> + return -EINVAL;
> + }
> +
> + if (!env_type_is_resolve_sink(env, index_type) &&
> + !env_type_is_resolved(env, index_type_id))
> + return env_stack_push(env, index_type, index_type_id);
> +
> + index_type = btf_type_id_size(btf, &index_type_id, NULL);
> + if (!index_type || !btf_type_is_int(index_type) ||
> + /* bit field int is not allowed */
> + !btf_type_int_is_regular(index_type) ||
> + /* unsigned only */
> + BTF_INT_ENCODING(btf_type_int(index_type))) {
> + btf_verifier_log_type(env, v->t, "Invalid index");
> + return -EINVAL;
> + }
Currently, in uapi/linux/btf.h, we have
/* Attributes stored in the BTF_INT_ENCODING */
#define BTF_INT_SIGNED 0x1
#define BTF_INT_CHAR 0x2
#define BTF_INT_BOOL 0x4
#define BTF_INT_VARARGS 0x8
The BPF_INT_ENCODING value 0 stands for UNSIGNED.
Do we want to explicitly document this in uapi/linux/bpf.h?
> +
> + /* Check array->type */
> + elem_type_id = array->type;
> elem_type = btf_type_by_id(btf, elem_type_id);
> if (btf_type_is_void_or_null(elem_type)) {
> btf_verifier_log_type(env, v->t,
> @@ -1351,22 +1399,9 @@ static int btf_array_resolve(struct btf_verifier_env *env,
> return -EINVAL;
> }
>
> - if (btf_type_is_int(elem_type)) {
> - int int_type_data = btf_type_int(elem_type);
> - u16 nr_bits = BTF_INT_BITS(int_type_data);
> - u16 nr_bytes = BITS_ROUNDUP_BYTES(nr_bits);
> -
> - /* Put more restriction on array of int. The int cannot
> - * be a bit field and it must be either u8/u16/u32/u64.
> - */
> - if (BITS_PER_BYTE_MASKED(nr_bits) ||
> - BTF_INT_OFFSET(int_type_data) ||
> - (nr_bytes != sizeof(u8) && nr_bytes != sizeof(u16) &&
> - nr_bytes != sizeof(u32) && nr_bytes != sizeof(u64))) {
> - btf_verifier_log_type(env, v->t,
> - "Invalid array of int");
> - return -EINVAL;
> - }
> + if (btf_type_is_int(elem_type) && !btf_type_int_is_regular(elem_type)) {
> + btf_verifier_log_type(env, v->t, "Invalid array of int");
> + return -EINVAL;
> }
>
> if (array->nelems && elem_size > U32_MAX / array->nelems) {
>
Powered by blists - more mailing lists