Re: [PATCH bpf 3/3] bpf: show main program address in bpf_prog_info->jited_ksyms

[Daniel Borkmann <daniel@iogearbox.net>]

On 11/01/2018 08:00 AM, Song Liu wrote:
> Currently, when there is not subprog (prog->aux->func_cnt == 0),
> bpf_prog_info does not return any jited_ksyms. This patch adds
> main program address (prog->bpf_func) to jited_ksyms.
> 
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
>  kernel/bpf/syscall.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 34a9eef5992c..7293b17ca62a 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2158,7 +2158,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
>  	}
>  
>  	ulen = info.nr_jited_ksyms;
> -	info.nr_jited_ksyms = prog->aux->func_cnt;
> +	info.nr_jited_ksyms = prog->aux->func_cnt ? : 1;
>  	if (info.nr_jited_ksyms && ulen) {
>  		if (bpf_dump_raw_ok()) {
>  			u64 __user *user_ksyms;
> @@ -2170,9 +2170,17 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
>  			 */
>  			ulen = min_t(u32, info.nr_jited_ksyms, ulen);
>  			user_ksyms = u64_to_user_ptr(info.jited_ksyms);
> -			for (i = 0; i < ulen; i++) {
> -				ksym_addr = (ulong) prog->aux->func[i]->bpf_func;
> -				if (put_user((u64) ksym_addr, &user_ksyms[i]))
> +			if (prog->aux->func_cnt) {
> +				for (i = 0; i < ulen; i++) {
> +					ksym_addr = (ulong)

Small nit: can we change ksym_addr, the above and below cast to kernel-style
'unsigned long' while at it?

> +						prog->aux->func[i]->bpf_func;
> +					if (put_user((u64) ksym_addr,
> +						     &user_ksyms[i]))
> +						return -EFAULT;
> +				}
> +			} else {
> +				ksym_addr = (ulong) prog->bpf_func;
> +				if (put_user((u64) ksym_addr, &user_ksyms[0]))
>  					return -EFAULT;

If we do this here, I think we should also update nr_jited_func_lens to copy
prog->jited_len to user space to be consistent with this change here. In case
of multi-func, the latter copies the len of the main program, and the lens of
the subprogs. Given we push the address for it to user space, we should then
also push the main prog len if it's only main prog there so this case doesn't
need any special handling by user space.

>  			}
>  		} else {
>