* [PATCH bpf 0/3] show more accurrate bpf program address
@ 2018-11-01 7:00 Song Liu
2018-11-01 7:00 ` [PATCH bpf 1/3] bpf: show real jited prog address in /proc/kallsyms Song Liu
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Song Liu @ 2018-11-01 7:00 UTC (permalink / raw)
To: netdev; +Cc: kernel-team, Song Liu, ast, daniel, sandipan
This set improves bpf program address showed in /proc/kallsyms and in
bpf_prog_info. First, real program address is showed instead of page
address. Second, when there is no subprogram, bpf_prog_info->jited_ksyms
returns the main prog address.
Song Liu (3):
bpf: show real jited prog address in /proc/kallsyms
bpf: show real jited address in bpf_prog_info->jited_ksyms
bpf: show main program address in bpf_prog_info->jited_ksyms
kernel/bpf/core.c | 4 +---
kernel/bpf/syscall.c | 17 ++++++++++++-----
2 files changed, 13 insertions(+), 8 deletions(-)
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf 1/3] bpf: show real jited prog address in /proc/kallsyms
2018-11-01 7:00 [PATCH bpf 0/3] show more accurrate bpf program address Song Liu
@ 2018-11-01 7:00 ` Song Liu
2018-11-01 7:00 ` [PATCH bpf 2/3] bpf: show real jited address in bpf_prog_info->jited_ksyms Song Liu
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Song Liu @ 2018-11-01 7:00 UTC (permalink / raw)
To: netdev; +Cc: kernel-team, Song Liu, ast, daniel, sandipan
Currently, /proc/kallsyms shows page address 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@fb.com>
---
kernel/bpf/core.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 6377225b2082..1a796e0799ec 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -553,7 +553,6 @@ bool is_bpf_text_address(unsigned long addr)
int bpf_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
char *sym)
{
- unsigned long symbol_start, symbol_end;
struct bpf_prog_aux *aux;
unsigned int it = 0;
int ret = -ERANGE;
@@ -566,10 +565,9 @@ int bpf_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
if (it++ != symnum)
continue;
- bpf_get_prog_addr_region(aux->prog, &symbol_start, &symbol_end);
bpf_get_prog_name(aux->prog, sym);
- *value = symbol_start;
+ *value = (unsigned long)aux->prog->bpf_func;
*type = BPF_SYM_ELF_TYPE;
ret = 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf 2/3] bpf: show real jited address in bpf_prog_info->jited_ksyms
2018-11-01 7:00 [PATCH bpf 0/3] show more accurrate bpf program address Song Liu
2018-11-01 7:00 ` [PATCH bpf 1/3] bpf: show real jited prog address in /proc/kallsyms Song Liu
@ 2018-11-01 7:00 ` Song Liu
2018-11-02 10:09 ` Daniel Borkmann
2018-11-01 7:00 ` [PATCH bpf 3/3] bpf: show main program " Song Liu
2018-11-01 23:15 ` [PATCH bpf 0/3] show more accurrate bpf program address Alexei Starovoitov
3 siblings, 1 reply; 9+ messages in thread
From: Song Liu @ 2018-11-01 7:00 UTC (permalink / raw)
To: netdev; +Cc: kernel-team, Song Liu, ast, daniel, sandipan
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@fb.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;
if (put_user((u64) ksym_addr, &user_ksyms[i]))
return -EFAULT;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf 3/3] bpf: show main program address in bpf_prog_info->jited_ksyms
2018-11-01 7:00 [PATCH bpf 0/3] show more accurrate bpf program address Song Liu
2018-11-01 7:00 ` [PATCH bpf 1/3] bpf: show real jited prog address in /proc/kallsyms Song Liu
2018-11-01 7:00 ` [PATCH bpf 2/3] bpf: show real jited address in bpf_prog_info->jited_ksyms Song Liu
@ 2018-11-01 7:00 ` Song Liu
2018-11-02 9:57 ` Daniel Borkmann
2018-11-01 23:15 ` [PATCH bpf 0/3] show more accurrate bpf program address Alexei Starovoitov
3 siblings, 1 reply; 9+ messages in thread
From: Song Liu @ 2018-11-01 7:00 UTC (permalink / raw)
To: netdev; +Cc: kernel-team, Song Liu, ast, daniel, sandipan
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)
+ 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;
}
} else {
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf 0/3] show more accurrate bpf program address
2018-11-01 7:00 [PATCH bpf 0/3] show more accurrate bpf program address Song Liu
` (2 preceding siblings ...)
2018-11-01 7:00 ` [PATCH bpf 3/3] bpf: show main program " Song Liu
@ 2018-11-01 23:15 ` Alexei Starovoitov
3 siblings, 0 replies; 9+ messages in thread
From: Alexei Starovoitov @ 2018-11-01 23:15 UTC (permalink / raw)
To: Song Liu; +Cc: netdev, kernel-team, ast, daniel, sandipan
On Thu, Nov 01, 2018 at 12:00:55AM -0700, Song Liu wrote:
> This set improves bpf program address showed in /proc/kallsyms and in
> bpf_prog_info. First, real program address is showed instead of page
> address. Second, when there is no subprogram, bpf_prog_info->jited_ksyms
> returns the main prog address.
For the set:
Acked-by: Alexei Starovoitov <ast@kernel.org>
I think we have to make the whole thing consistent like this set does
and send it to stable.
The only other alternative is to invent new cmd and prog_info fields to return
proper jited_ksyms and keep this one buggy forever.
My preference is to fix it.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf 3/3] bpf: show main program address in bpf_prog_info->jited_ksyms
2018-11-01 7:00 ` [PATCH bpf 3/3] bpf: show main program " Song Liu
@ 2018-11-02 9:57 ` Daniel Borkmann
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Borkmann @ 2018-11-02 9:57 UTC (permalink / raw)
To: Song Liu, netdev; +Cc: kernel-team, ast, sandipan
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 {
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf 2/3] bpf: show real jited address in bpf_prog_info->jited_ksyms
2018-11-01 7:00 ` [PATCH bpf 2/3] bpf: show real jited address in bpf_prog_info->jited_ksyms Song Liu
@ 2018-11-02 10:09 ` Daniel Borkmann
2018-11-02 10:19 ` Daniel Borkmann
0 siblings, 1 reply; 9+ messages in thread
From: Daniel Borkmann @ 2018-11-02 10:09 UTC (permalink / raw)
To: Song Liu, netdev; +Cc: kernel-team, ast, sandipan
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@fb.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.
> if (put_user((u64) ksym_addr, &user_ksyms[i]))
> return -EFAULT;
> }
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf 2/3] bpf: show real jited address in bpf_prog_info->jited_ksyms
2018-11-02 10:09 ` Daniel Borkmann
@ 2018-11-02 10:19 ` Daniel Borkmann
2018-11-02 16:07 ` Song Liu
0 siblings, 1 reply; 9+ messages in thread
From: Daniel Borkmann @ 2018-11-02 10:19 UTC (permalink / raw)
To: Song Liu, netdev; +Cc: kernel-team, ast, sandipan
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@fb.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.)
>> if (put_user((u64) ksym_addr, &user_ksyms[i]))
>> return -EFAULT;
>> }
>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf 2/3] bpf: show real jited address in bpf_prog_info->jited_ksyms
2018-11-02 10:19 ` Daniel Borkmann
@ 2018-11-02 16:07 ` Song Liu
0 siblings, 0 replies; 9+ messages in thread
From: Song Liu @ 2018-11-02 16:07 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Netdev, Kernel Team, ast, sandipan
> On Nov 2, 2018, at 3:19 AM, Daniel Borkmann <daniel@iogearbox.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@fb.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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-11-03 1:15 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-01 7:00 [PATCH bpf 0/3] show more accurrate bpf program address Song Liu
2018-11-01 7:00 ` [PATCH bpf 1/3] bpf: show real jited prog address in /proc/kallsyms Song Liu
2018-11-01 7:00 ` [PATCH bpf 2/3] bpf: show real jited address in bpf_prog_info->jited_ksyms Song Liu
2018-11-02 10:09 ` Daniel Borkmann
2018-11-02 10:19 ` Daniel Borkmann
2018-11-02 16:07 ` Song Liu
2018-11-01 7:00 ` [PATCH bpf 3/3] bpf: show main program " Song Liu
2018-11-02 9:57 ` Daniel Borkmann
2018-11-01 23:15 ` [PATCH bpf 0/3] show more accurrate bpf program address Alexei Starovoitov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.