Re: [PATCH bpf-next v5 4/8] libbpf: Create a bpf_link in bpf_map__attach_struct_ops().

Kui-Feng Lee
2023-03-09T18:20:06Z

On 3/9/23 10:16, Kui-Feng Lee wrote:
>
>
> On 3/9/23 09:09, Martin KaFai Lau wrote:
>> On 3/8/23 4:22 PM, Kui-Feng Lee wrote:
>>>
>>>
>>> On 3/8/23 13:42, Martin KaFai Lau wrote:
>>>> On 3/7/23 4:50 PM, Kui-Feng Lee wrote:
>>>>> @@ -11566,22 +11591,34 @@ struct bpf_link
>>>>> *bpf_program__attach(const struct bpf_program *prog)
>>>>>       return link;
>>>>>   }
>>>>> +struct bpf_link_struct_ops {
>>>>> +    struct bpf_link link;
>>>>> +    int map_fd;
>>>>> +};
>>>>> +
>>>>>   static int bpf_link__detach_struct_ops(struct bpf_link *link)
>>>>>   {
>>>>> +    struct bpf_link_struct_ops *st_link;
>>>>>       __u32 zero = 0;
>>>>> -    if (bpf_map_delete_elem(link->fd, &zero))
>>>>> -        return -errno;
>>>>> +    st_link = container_of(link, struct bpf_link_struct_ops, link);
>>>>> -    return 0;
>>>>> +    if (st_link->map_fd < 0) {
>>>>
>>>> map_fd < 0 should always be true?
>>>
>>> If the user pass a wrong link, it can fail.
>>
>> I may have missed something. How can user directly pass a link to this
>> static function?
>
> Ouch! You are right. This check is not necessary. I mixed it with the
> old detach feature.

By the way, I will keep this test here since this function will handle
the case w/o a link as well.

>
>
>>
>>> I check it here explicitly even the kernel returns
>>> an error for deleting an element of a struct_ops w/ link.
>> Yep, the kernel should have stopped the delete if the user somehow
>> corrupted the map_fd to -1.
>>
>>>
>>>>
>>>>> +        /* Fake bpf_link */
>>>>> +        if (bpf_map_delete_elem(link->fd, &zero))
>>>>> +            return -errno;
>>>>> +        return 0;
>>>>> +    }
>>>>> +
>>>>> +    /* Doesn't support detaching. */
>>>>> +    return -EOPNOTSUPP;
>>