Re: [PATCH RFC v2 net-next 07/16] bpf: add lookup/update/delete/iterate methods to BPF maps

On Wed, Jul 23, 2014 at 1:25 PM, Kees Cook [off-list ref] wrote:

On Wed, Jul 23, 2014 at 12:49 PM, Alexei Starovoitov [off-list ref] wrote:

quoted

On Wed, Jul 23, 2014 at 11:25 AM, Kees Cook [off-list ref] wrote:

quoted

quoted

+
+       /* 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.

quoted

quoted

+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.