netdev - Re: [PATCH bpf-next v2 1/7] perf/core: add perf_get_event() to return perf

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] [day] [month] [year] [list]

Message-ID: <cf3c38d5-f5b9-61ae-4b53-259c97dc84ab@fb.com>
Date:   Fri, 18 May 2018 09:32:09 -0700
From:   Yonghong Song <yhs@...com>
To:     Peter Zijlstra <peterz@...radead.org>
CC:     <ast@...com>, <daniel@...earbox.net>, <netdev@...r.kernel.org>,
        <kernel-team@...com>
Subject: Re: [PATCH bpf-next v2 1/7] perf/core: add perf_get_event() to return
 perf_event given a struct file

On 5/18/18 12:18 AM, Peter Zijlstra wrote:
> On Thu, May 17, 2018 at 10:32:53PM -0700, Yonghong Song wrote:
>> A new extern function, perf_get_event(), is added to return a perf event
>> given a struct file. This function will be used in later patches.
> 
> Can't you do a narrower interface? Like return the prog. I'm not too
> keen on random !perf code frobbing around inside the event.

Hi, Peter,

My initial implementation (not upstreamed) actually have the whole
function bpf_get_perf_event_info() in the events/core.c. In that
case, the "struct file *" pointer is passed. This way, the event pointer
does not need to go to kernel/bpf/syscall.c or kernel/trace/bpf_trace.c.

I dropped this mechanism since it added more codes in the events/core.c
file, and I felt that such query code might clutter events/core.c.
The function bpf_get_perf_event_info() is now placed in 
kernel/trace/bpf_trace.c.

Just getting bpf prog pointer is not enough as it does not provide
enough attachment information. Getting such information requires
poking into event/tp_event etc.

Currently we have this extern function exposed by events/core.c:
    extern struct perf_event *perf_get_event(struct file *file);
We could make the result value "const" like
    extern const struct perf_event *perf_get_event(struct file *file);
This will make it clear that we do not change "event" fields, and
merely poking at it.

Please let me know your preference.

Thanks!
Yonghong