Messages in this thread | | | Date | Wed, 23 Jul 2014 14:22:24 -0700 | Subject | Re: [PATCH RFC v2 net-next 07/16] bpf: add lookup/update/delete/iterate methods to BPF maps | From | Alexei Starovoitov <> |
| |
On Wed, Jul 23, 2014 at 1:25 PM, Kees Cook <keescook@chromium.org> wrote: > On Wed, Jul 23, 2014 at 12:49 PM, Alexei Starovoitov <ast@plumgrid.com> wrote: >> On Wed, Jul 23, 2014 at 11:25 AM, Kees Cook <keescook@chromium.org> wrote: >>>> + >>>> + /* lookup key in a given map referenced by map_id >>>> + * err = bpf_map_lookup_elem(int map_id, void *key, void *value) >>> >>> This needs map_id documentation updates too? >> >> yes. will grep for it just to make sure. >> >>>> +static int get_map_id(struct fd f) >>>> +{ >>>> + struct bpf_map *map; >>>> + >>>> + if (!f.file) >>>> + return -EBADF; >>>> + >>>> + if (f.file->f_op != &bpf_map_fops) { >>>> + fdput(f); >>> >>> It feels weird to me to do the fdput inside this function. Instead, >>> should map_lookup_elem get a "err_put" label, instead? >> >> I don't think it will work, since I'm not sure that fd.flags will be zero >> when fd.file == NULL. It looks so by analyzing return code path >> in fs/file.c, but I wasn't sure that I followed all code paths, >> so I just picked this style from fs/timerfd.c assuming it was >> done this away on purpose and there can be the case where >> fd.file == null and fd.flags !=0. In such case we cannot call fdput(). > > Yeah, hm, looking around, this does seem to be the case. I guess the > thought is that when get_map_id fails, struct fd has been handled.
correct.
> Maybe add a comment above that function as a reminder?
yes. will do.
| |