* [PATCH bpf] tools/bpftool: fix PID fetching with a lot of results
@ 2020-12-04 23:20 Andrii Nakryiko
2020-12-05 19:11 ` Yonghong Song
2020-12-08 15:50 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2020-12-04 23:20 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team
In case of having so many PID results that they don't fit into a singe page
(4096) bytes, bpftool will erroneously conclude that it got corrupted data due
to 4096 not being a multiple of struct pid_iter_entry, so the last entry will
be partially truncated. Fix this by sizing the buffer to fit exactly N entries
with no truncation in the middle of record.
Fixes: d53dee3fe013 ("tools/bpftool: Show info for processes holding BPF map/prog/link/btf FDs")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/bpf/bpftool/pids.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/bpf/bpftool/pids.c b/tools/bpf/bpftool/pids.c
index df7d8ec76036..477e55d59c34 100644
--- a/tools/bpf/bpftool/pids.c
+++ b/tools/bpf/bpftool/pids.c
@@ -89,9 +89,9 @@ libbpf_print_none(__maybe_unused enum libbpf_print_level level,
int build_obj_refs_table(struct obj_refs_table *table, enum bpf_obj_type type)
{
- char buf[4096];
- struct pid_iter_bpf *skel;
struct pid_iter_entry *e;
+ char buf[4096 / sizeof(*e) * sizeof(*e)];
+ struct pid_iter_bpf *skel;
int err, ret, fd = -1, i;
libbpf_print_fn_t default_print;
--
2.24.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf] tools/bpftool: fix PID fetching with a lot of results
2020-12-04 23:20 [PATCH bpf] tools/bpftool: fix PID fetching with a lot of results Andrii Nakryiko
@ 2020-12-05 19:11 ` Yonghong Song
2020-12-08 2:55 ` Andrii Nakryiko
2020-12-08 15:50 ` patchwork-bot+netdevbpf
1 sibling, 1 reply; 5+ messages in thread
From: Yonghong Song @ 2020-12-05 19:11 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, netdev, ast, daniel; +Cc: kernel-team
On 12/4/20 3:20 PM, Andrii Nakryiko wrote:
> In case of having so many PID results that they don't fit into a singe page
> (4096) bytes, bpftool will erroneously conclude that it got corrupted data due
> to 4096 not being a multiple of struct pid_iter_entry, so the last entry will
> be partially truncated. Fix this by sizing the buffer to fit exactly N entries
> with no truncation in the middle of record.
>
> Fixes: d53dee3fe013 ("tools/bpftool: Show info for processes holding BPF map/prog/link/btf FDs")
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Ack with one nit below.
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> tools/bpf/bpftool/pids.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/bpf/bpftool/pids.c b/tools/bpf/bpftool/pids.c
> index df7d8ec76036..477e55d59c34 100644
> --- a/tools/bpf/bpftool/pids.c
> +++ b/tools/bpf/bpftool/pids.c
> @@ -89,9 +89,9 @@ libbpf_print_none(__maybe_unused enum libbpf_print_level level,
>
> int build_obj_refs_table(struct obj_refs_table *table, enum bpf_obj_type type)
> {
> - char buf[4096];
> - struct pid_iter_bpf *skel;
> struct pid_iter_entry *e;
> + char buf[4096 / sizeof(*e) * sizeof(*e)];
> + struct pid_iter_bpf *skel;
No need to move "struct pid_iter_bpf *skel", right?
> int err, ret, fd = -1, i;
> libbpf_print_fn_t default_print;
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf] tools/bpftool: fix PID fetching with a lot of results
2020-12-05 19:11 ` Yonghong Song
@ 2020-12-08 2:55 ` Andrii Nakryiko
2020-12-08 3:35 ` Yonghong Song
0 siblings, 1 reply; 5+ messages in thread
From: Andrii Nakryiko @ 2020-12-08 2:55 UTC (permalink / raw)
To: Yonghong Song
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
On Sat, Dec 5, 2020 at 11:11 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 12/4/20 3:20 PM, Andrii Nakryiko wrote:
> > In case of having so many PID results that they don't fit into a singe page
> > (4096) bytes, bpftool will erroneously conclude that it got corrupted data due
> > to 4096 not being a multiple of struct pid_iter_entry, so the last entry will
> > be partially truncated. Fix this by sizing the buffer to fit exactly N entries
> > with no truncation in the middle of record.
> >
> > Fixes: d53dee3fe013 ("tools/bpftool: Show info for processes holding BPF map/prog/link/btf FDs")
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>
> Ack with one nit below.
>
> Acked-by: Yonghong Song <yhs@fb.com>
>
> > ---
> > tools/bpf/bpftool/pids.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/pids.c b/tools/bpf/bpftool/pids.c
> > index df7d8ec76036..477e55d59c34 100644
> > --- a/tools/bpf/bpftool/pids.c
> > +++ b/tools/bpf/bpftool/pids.c
> > @@ -89,9 +89,9 @@ libbpf_print_none(__maybe_unused enum libbpf_print_level level,
> >
> > int build_obj_refs_table(struct obj_refs_table *table, enum bpf_obj_type type)
> > {
> > - char buf[4096];
> > - struct pid_iter_bpf *skel;
> > struct pid_iter_entry *e;
> > + char buf[4096 / sizeof(*e) * sizeof(*e)];
> > + struct pid_iter_bpf *skel;
>
> No need to move "struct pid_iter_bpf *skel", right?
It's actually a move of `struct pid_iter_entry *e;` in from of char
buf[], to be able to use sizeof(*e) instead of sizeof(struct
pid_iter_bpf). It's just that diff tool didn't catch this properly :)
>
> > int err, ret, fd = -1, i;
> > libbpf_print_fn_t default_print;
> >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf] tools/bpftool: fix PID fetching with a lot of results
2020-12-08 2:55 ` Andrii Nakryiko
@ 2020-12-08 3:35 ` Yonghong Song
0 siblings, 0 replies; 5+ messages in thread
From: Yonghong Song @ 2020-12-08 3:35 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
On 12/7/20 6:55 PM, Andrii Nakryiko wrote:
> On Sat, Dec 5, 2020 at 11:11 AM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 12/4/20 3:20 PM, Andrii Nakryiko wrote:
>>> In case of having so many PID results that they don't fit into a singe page
>>> (4096) bytes, bpftool will erroneously conclude that it got corrupted data due
>>> to 4096 not being a multiple of struct pid_iter_entry, so the last entry will
>>> be partially truncated. Fix this by sizing the buffer to fit exactly N entries
>>> with no truncation in the middle of record.
>>>
>>> Fixes: d53dee3fe013 ("tools/bpftool: Show info for processes holding BPF map/prog/link/btf FDs")
>>> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>>
>> Ack with one nit below.
>>
>> Acked-by: Yonghong Song <yhs@fb.com>
>>
>>> ---
>>> tools/bpf/bpftool/pids.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/bpf/bpftool/pids.c b/tools/bpf/bpftool/pids.c
>>> index df7d8ec76036..477e55d59c34 100644
>>> --- a/tools/bpf/bpftool/pids.c
>>> +++ b/tools/bpf/bpftool/pids.c
>>> @@ -89,9 +89,9 @@ libbpf_print_none(__maybe_unused enum libbpf_print_level level,
>>>
>>> int build_obj_refs_table(struct obj_refs_table *table, enum bpf_obj_type type)
>>> {
>>> - char buf[4096];
>>> - struct pid_iter_bpf *skel;
>>> struct pid_iter_entry *e;
>>> + char buf[4096 / sizeof(*e) * sizeof(*e)];
>>> + struct pid_iter_bpf *skel;
>>
>> No need to move "struct pid_iter_bpf *skel", right?
>
> It's actually a move of `struct pid_iter_entry *e;` in from of char
> buf[], to be able to use sizeof(*e) instead of sizeof(struct
> pid_iter_bpf). It's just that diff tool didn't catch this properly :)
Indeed. Looking at the final code, no unnecessary code churn.
>
>>
>>> int err, ret, fd = -1, i;
>>> libbpf_print_fn_t default_print;
>>>
>>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf] tools/bpftool: fix PID fetching with a lot of results
2020-12-04 23:20 [PATCH bpf] tools/bpftool: fix PID fetching with a lot of results Andrii Nakryiko
2020-12-05 19:11 ` Yonghong Song
@ 2020-12-08 15:50 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2020-12-08 15:50 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, netdev, ast, daniel, kernel-team
Hello:
This patch was applied to bpf/bpf.git (refs/heads/master):
On Fri, 4 Dec 2020 15:20:01 -0800 you wrote:
> In case of having so many PID results that they don't fit into a singe page
> (4096) bytes, bpftool will erroneously conclude that it got corrupted data due
> to 4096 not being a multiple of struct pid_iter_entry, so the last entry will
> be partially truncated. Fix this by sizing the buffer to fit exactly N entries
> with no truncation in the middle of record.
>
> Fixes: d53dee3fe013 ("tools/bpftool: Show info for processes holding BPF map/prog/link/btf FDs")
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>
> [...]
Here is the summary with links:
- [bpf] tools/bpftool: fix PID fetching with a lot of results
https://git.kernel.org/bpf/bpf/c/932c60558109
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-08 15:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-04 23:20 [PATCH bpf] tools/bpftool: fix PID fetching with a lot of results Andrii Nakryiko
2020-12-05 19:11 ` Yonghong Song
2020-12-08 2:55 ` Andrii Nakryiko
2020-12-08 3:35 ` Yonghong Song
2020-12-08 15:50 ` patchwork-bot+netdevbpf
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.