From mboxrd@z Thu Jan 1 00:00:00 1970 From: Song Liu Subject: Re: [PATCH bpf 2/3] bpf: show real jited address in bpf_prog_info->jited_ksyms Date: Fri, 2 Nov 2018 16:07:05 +0000 Message-ID: References: <20181101070058.2760251-1-songliubraving@fb.com> <20181101070058.2760251-3-songliubraving@fb.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: Netdev , Kernel Team , "ast@kernel.org" , "sandipan@linux.vnet.ibm.com" To: Daniel Borkmann Return-path: Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:55398 "EHLO mx0b-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727615AbeKCBP3 (ORCPT ); Fri, 2 Nov 2018 21:15:29 -0400 In-Reply-To: Content-Language: en-US Content-ID: <0B09B2648E87D24A84DE7DD0810AFB75@namprd15.prod.outlook.com> Sender: netdev-owner@vger.kernel.org List-ID: > On Nov 2, 2018, at 3:19 AM, Daniel Borkmann wrote: >=20 > On 11/02/2018 11:09 AM, Daniel Borkmann wrote: >> On 11/01/2018 08:00 AM, Song Liu wrote: >>> Currently, jited_ksyms in bpf_prog_info shows page addresses of jited >>> bpf program. This is not ideal for detailed profiling (find hot >>> instructions from stack traces). This patch replaces the page address >>> with real prog start address. >>>=20 >>> Signed-off-by: Song Liu >>> --- >>> kernel/bpf/syscall.c | 1 - >>> 1 file changed, 1 deletion(-) >>>=20 >>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c >>> index ccb93277aae2..34a9eef5992c 100644 >>> --- a/kernel/bpf/syscall.c >>> +++ b/kernel/bpf/syscall.c >>> @@ -2172,7 +2172,6 @@ static int bpf_prog_get_info_by_fd(struct bpf_pro= g *prog, >>> user_ksyms =3D u64_to_user_ptr(info.jited_ksyms); >>> for (i =3D 0; i < ulen; i++) { >>> ksym_addr =3D (ulong) prog->aux->func[i]->bpf_func; >>> - ksym_addr &=3D PAGE_MASK; >>=20 >> Note that the masking was done on purpose here and in patch 1/3 in order= to >> not expose randomized start address to kallsyms at least. I suppose it's >> okay to change it here and for kallsyms given bpf_prog_get_info_by_fd() = dump >> is for root only, and in each of the two cases we additionally apply >> kallsyms_show_value() logic, so for unpriv this is zeroed out plus only = root >> loaded programs are added under kallsyms (capable(CAP_SYS_ADMIN)) anyway= . >=20 > (Btw, something like above should have been in changelog to provide some = more > historical context of why we used to do it like that and explaining why i= t is > okay to change it this way.) Thanks Daniel! I will send v2 with these fixes.=20 Song=