netdev - Re: [PATCH bpf-next 01/10] bpf: btf: Introduce BPF Type Format (BTF)

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180330233240.ks63h53vorbsxa7f@kafai-mbp.dhcp.thefacebook.com>
Date:   Fri, 30 Mar 2018 16:32:42 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     Daniel Borkmann <daniel@...earbox.net>
CC:     <netdev@...r.kernel.org>, Alexei Starovoitov <ast@...com>,
        <kernel-team@...com>
Subject: Re: [PATCH bpf-next 01/10] bpf: btf: Introduce BPF Type Format (BTF)

On Sat, Mar 31, 2018 at 01:22:53AM +0200, Daniel Borkmann wrote:
> 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.
Thanks. I will make the change in the next spin.

> 
> > +		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);
> > +}