Messages in this thread | | | Date | Wed, 23 Jul 2014 12:57:23 -0700 | Subject | Re: [PATCH RFC v2 net-next 08/16] bpf: add hashtable type of BPF maps | From | Alexei Starovoitov <> |
| |
On Wed, Jul 23, 2014 at 11:36 AM, Kees Cook <keescook@chromium.org> wrote: >> +static struct bpf_map *htab_map_alloc(struct nlattr *attr[BPF_MAP_ATTR_MAX + 1]) >> +{ >> + struct bpf_htab *htab; >> + int err, i; >> + >> + htab = kmalloc(sizeof(*htab), GFP_USER); > > I'd prefer kzalloc here.
in this case I agree. will change, since it's not in critical path and we can waste few cycles zeroing memory.
>> + err = -ENOMEM; >> + htab->buckets = kmalloc(htab->n_buckets * sizeof(struct hlist_head), >> + GFP_USER); > > I'd prefer kcalloc here, even though n_buckets can't currently trigger > an integer overflow.
hmm, I would argue that kmalloc_array is a preferred way, but kcalloc ? Few lines below the whole array is inited with INIT_HLIST_HEAD...
>> + for (i = 0; i < htab->n_buckets; i++) >> + INIT_HLIST_HEAD(&htab->buckets[i]);
>> + htab->slab_name = kasprintf(GFP_USER, "bpf_htab_%p", htab); > > This leaks a kernel heap memory pointer to userspace. If a unique name > needed, I think map_id should be used instead.
it leaks, how? slabinfo is only available to root. The same code exists in conntrack: net/netfilter/nf_conntrack_core.c:1767
| |