Re: [PATCH bpf-next 3/3] bpf: don't require bpf_capable() for GET_INFO_BY_FD

[Daniel Borkmann <daniel@iogearbox.net>]

On 5/25/23 7:20 PM, Andrii Nakryiko wrote:
> On Thu, May 25, 2023 at 6:14 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>> On 5/25/23 12:54 AM, Andrii Nakryiko wrote:
>>> The rest of BPF subsystem follows the rule that if process managed to
>>> get BPF object FD, then it has an ownership of this object, and thus can
>>> query any information about it, or update it. Doing something special in
>>> GET_INFO_BY_FD operation based on bpf_capable() goes against that
>>> philosophy, so drop the check and unify the approach with the rest of
>>> bpf() syscall.
>>>
>>> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>>> ---
>>>    kernel/bpf/syscall.c | 11 -----------
>>>    1 file changed, 11 deletions(-)
>>>
>>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>>> index 1d74c0a8d903..b07453ce10e7 100644
>>> --- a/kernel/bpf/syscall.c
>>> +++ b/kernel/bpf/syscall.c
>>> @@ -4022,17 +4022,6 @@ static int bpf_prog_get_info_by_fd(struct file *file,
>>>
>>>        info.verified_insns = prog->aux->verified_insns;
>>>
>>> -     if (!bpf_capable()) {
>>> -             info.jited_prog_len = 0;
>>> -             info.xlated_prog_len = 0;
>>> -             info.nr_jited_ksyms = 0;
>>> -             info.nr_jited_func_lens = 0;
>>> -             info.nr_func_info = 0;
>>> -             info.nr_line_info = 0;
>>> -             info.nr_jited_line_info = 0;
>>> -             goto done;
>>> -     }
>>
>> Isn't this leaking raw kernel pointers from JIT image this way for unpriv? I think that
>> is the main reason why we guarded this (originally behind !capable(CAP_SYS_ADMIN)) back
>> then..
> 
> Ah, ok, makes sense. We are protecting kernel from unpriv prog/user,
> so the "if you have FD you can get info about object" rule doesn't
> apply here.

Yeah that is correct.