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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <82bdf2c2-fe37-a693-f8c3-dd5160367533@iogearbox.net>
Date:   Fri, 2 Nov 2018 10:57:28 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Song Liu <songliubraving@...com>, netdev@...r.kernel.org
Cc:     kernel-team@...com, ast@...nel.org, sandipan@...ux.vnet.ibm.com
Subject: Re: [PATCH bpf 3/3] bpf: show main program address in
 bpf_prog_info->jited_ksyms

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