From mboxrd@z Thu Jan  1 00:00:00 1970
From: Daniel Borkmann <daniel@iogearbox.net>
Subject: Re: [PATCH bpf-next 01/10] bpf: btf: Introduce BPF Type Format (BTF)
Date: Sat, 31 Mar 2018 01:22:53 +0200
Message-ID: <0dc44dd6-132d-6005-da38-7bcfd22bf040@iogearbox.net>
References: <20180330182643.3539371-1-kafai@fb.com>
 <20180330182643.3539371-2-kafai@fb.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Cc: Alexei Starovoitov <ast@fb.com>, kernel-team@fb.com
To: Martin KaFai Lau <kafai@fb.com>, netdev@vger.kernel.org
Return-path: <netdev-owner@vger.kernel.org>
Received: from www62.your-server.de ([213.133.104.62]:35820 "EHLO
        www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752357AbeC3XWz (ORCPT
        <rfc822;netdev@vger.kernel.org>); Fri, 30 Mar 2018 19:22:55 -0400
In-Reply-To: <20180330182643.3539371-2-kafai@fb.com>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On 03/30/2018 08:26 PM, Martin KaFai Lau wrote:
[...]
> +static int btf_add_type(struct btf_verifier_env *env, struct btf_type *t)
> +{
> +	struct btf *btf = env->btf;
> +
> +	/* < 2 because +1 for btf_void which is always in btf->types[0].
> +	 * btf_void is not accounted in btf->nr_types because btf_void
> +	 * does not come from the BTF file.
> +	 */
> +	if (btf->types_size - btf->nr_types < 2) {
> +		/* Expand 'types' array */
> +
> +		struct btf_type **new_types;
> +		u32 expand_by, new_size;
> +
> +		if (btf->types_size == BTF_MAX_NR_TYPES) {
> +			btf_verifier_log(env, "Exceeded max num of types");
> +			return -E2BIG;
> +		}
> +
> +		expand_by = max_t(u32, btf->types_size >> 2, 16);
> +		new_size = min_t(u32, BTF_MAX_NR_TYPES,
> +				 btf->types_size + expand_by);
> +
> +		new_types = kvzalloc(new_size * sizeof(*new_types),
> +				     GFP_KERNEL | __GFP_NOWARN);
> +		if (!new_types)
> +			return -ENOMEM;
> +
> +		if (btf->nr_types == 0)
> +			new_types[0] = &btf_void;
> +		else
> +			memcpy(new_types, btf->types,
> +			       sizeof(*btf->types) * (btf->nr_types + 1));
> +
> +		kfree(btf->types);
> +		btf->types = new_types;

Haven't read through the whole series yet, but this type of pattern pops up
immediately in several locations throughout multiple patches in this series.

Here, you'll free kv*alloc() backed memory into the wrong backend allocator,
thus if it's vmalloc() backed, it cannot go into kmalloc() backed memory via
kfree(), thus please audit the whole series on this.

> +		btf->types_size = new_size;
> +	}
> +
> +	btf->types[++(btf->nr_types)] = t;
> +
> +	return 0;
> +}
> +
> +static void btf_free(struct btf *btf)
> +{
> +	kfree(btf->types);
> +	kfree(btf->data);
> +	kfree(btf);
> +}
> +
> +static void btf_verifier_env_free(struct btf_verifier_env *env)
> +{
> +	kfree(env);
> +}
> +
[...]
> +	data = kvmalloc(btf_data_size, GFP_KERNEL | __GFP_NOWARN);
> +	if (!data) {
> +		err = -ENOMEM;
> +		goto errout;
> +	}
> +
> +	btf->data = data;
> +	btf->data_size = btf_data_size;
> +
> +	if (copy_from_user(data, btf_data, btf_data_size)) {
> +		err = -EFAULT;
> +		goto errout;
> +	}
> +
> +	env->btf = btf;
> +
> +	err = btf_parse_hdr(env);
> +	if (err)
> +		goto errout;
> +
> +	err = btf_parse_str_sec(env);
> +	if (err)
> +		goto errout;
> +
> +	err = btf_parse_type_sec(env);
> +	if (err)
> +		goto errout;
> +
> +	if (!err && log->level && bpf_verifier_log_full(log)) {
> +		err = -ENOSPC;
> +		goto errout;
> +	}
> +
> +	if (!err) {
> +		btf_verifier_env_free(env);
> +		return btf;
> +	}
> +
> +errout:
> +	btf_verifier_env_free(env);
> +	if (btf)
> +		btf_free(btf);
> +	return ERR_PTR(err);
> +}