netdev - Re: [PATCH bpf 2/3] bpf: show real jited 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 for Android: free password hash cracker in your pocket

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

Message-ID: <AB621962-500A-4F06-99CD-7E9E0B99E729@fb.com>
Date:   Fri, 2 Nov 2018 16:07:05 +0000
From:   Song Liu <songliubraving@...com>
To:     Daniel Borkmann <daniel@...earbox.net>
CC:     Netdev <netdev@...r.kernel.org>, Kernel Team <Kernel-team@...com>,
        "ast@...nel.org" <ast@...nel.org>,
        "sandipan@...ux.vnet.ibm.com" <sandipan@...ux.vnet.ibm.com>
Subject: Re: [PATCH bpf 2/3] bpf: show real jited address in
 bpf_prog_info->jited_ksyms



> On Nov 2, 2018, at 3:19 AM, Daniel Borkmann <daniel@...earbox.net> wrote:
> 
> 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.
>>> 
>>> Signed-off-by: Song Liu <songliubraving@...com>
>>> ---
>>> kernel/bpf/syscall.c | 1 -
>>> 1 file changed, 1 deletion(-)
>>> 
>>> 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_prog *prog,
>>> 			user_ksyms = u64_to_user_ptr(info.jited_ksyms);
>>> 			for (i = 0; i < ulen; i++) {
>>> 				ksym_addr = (ulong) prog->aux->func[i]->bpf_func;
>>> -				ksym_addr &= PAGE_MASK;
>> 
>> 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.
> 
> (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 it is
> okay to change it this way.)

Thanks Daniel!

I will send v2 with these fixes. 

Song