lkml.org 
[lkml]   [2014]   [Jul]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH RFC v2 net-next 07/16] bpf: add lookup/update/delete/iterate methods to BPF maps
From
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.


\
 
 \ /
  Last update: 2014-07-24 01:41    [W:0.113 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site