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