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

[Kui-Feng Lee <sinquersw@gmail.com>]

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.


> 
>> 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;
>