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

>   	ulen = info.xlated_prog_len;
>   	info.xlated_prog_len = bpf_prog_insn_size(prog);
>   	if (info.xlated_prog_len && ulen) {
>