* [PATCH bpf-next 1/2] tools: bpftool: skip type probe if name is not found
2020-07-24 9:06 [PATCH bpf-next 0/2] tools: bpftool: update prog names list and fix segfault Quentin Monnet
@ 2020-07-24 9:06 ` Quentin Monnet
2020-07-27 21:19 ` Song Liu
2020-07-24 9:06 ` [PATCH bpf-next 2/2] tools: bpftool: add LSM type to array of prog names Quentin Monnet
2020-07-28 10:16 ` [PATCH bpf-next 0/2] tools: bpftool: update prog names list and fix segfault Daniel Borkmann
2 siblings, 1 reply; 6+ messages in thread
From: Quentin Monnet @ 2020-07-24 9:06 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: bpf, netdev, Paul Chaignon, Quentin Monnet
For probing program and map types, bpftool loops on type values and uses
the relevant type name in prog_type_name[] or map_type_name[]. To ensure
the name exists, we exit from the loop if we go over the size of the
array.
However, this is not enough in the case where the arrays have "holes" in
them, program or map types for which they have no name, but not at the
end of the list. This is currently the case for BPF_PROG_TYPE_LSM, not
known to bpftool and which name is a null string. When probing for
features, bpftool attempts to strlen() that name and segfaults.
Let's fix it by skipping probes for "unknown" program and map types,
with an informational message giving the numeral value in that case.
Fixes: 93a3545d812a ("tools/bpftool: Add name mappings for SK_LOOKUP prog and attach type")
Reported-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
tools/bpf/bpftool/feature.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index 1cd75807673e..a43a6f10b564 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -504,6 +504,10 @@ probe_prog_type(enum bpf_prog_type prog_type, bool *supported_types,
supported_types[prog_type] |= res;
+ if (!prog_type_name[prog_type]) {
+ p_info("program type name not found (type %d)", prog_type);
+ return;
+ }
maxlen = sizeof(plain_desc) - strlen(plain_comment) - 1;
if (strlen(prog_type_name[prog_type]) > maxlen) {
p_info("program type name too long");
@@ -533,6 +537,10 @@ probe_map_type(enum bpf_map_type map_type, const char *define_prefix,
* check required for unprivileged users
*/
+ if (!map_type_name[map_type]) {
+ p_info("map type name not found (type %d)", map_type);
+ return;
+ }
maxlen = sizeof(plain_desc) - strlen(plain_comment) - 1;
if (strlen(map_type_name[map_type]) > maxlen) {
p_info("map type name too long");
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH bpf-next 1/2] tools: bpftool: skip type probe if name is not found
2020-07-24 9:06 ` [PATCH bpf-next 1/2] tools: bpftool: skip type probe if name is not found Quentin Monnet
@ 2020-07-27 21:19 ` Song Liu
0 siblings, 0 replies; 6+ messages in thread
From: Song Liu @ 2020-07-27 21:19 UTC (permalink / raw)
To: Quentin Monnet
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Networking, Paul Chaignon
On Fri, Jul 24, 2020 at 2:07 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> For probing program and map types, bpftool loops on type values and uses
> the relevant type name in prog_type_name[] or map_type_name[]. To ensure
> the name exists, we exit from the loop if we go over the size of the
> array.
>
> However, this is not enough in the case where the arrays have "holes" in
> them, program or map types for which they have no name, but not at the
> end of the list. This is currently the case for BPF_PROG_TYPE_LSM, not
> known to bpftool and which name is a null string. When probing for
> features, bpftool attempts to strlen() that name and segfaults.
>
> Let's fix it by skipping probes for "unknown" program and map types,
> with an informational message giving the numeral value in that case.
>
> Fixes: 93a3545d812a ("tools/bpftool: Add name mappings for SK_LOOKUP prog and attach type")
> Reported-by: Paul Chaignon <paul@cilium.io>
> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf-next 2/2] tools: bpftool: add LSM type to array of prog names
2020-07-24 9:06 [PATCH bpf-next 0/2] tools: bpftool: update prog names list and fix segfault Quentin Monnet
2020-07-24 9:06 ` [PATCH bpf-next 1/2] tools: bpftool: skip type probe if name is not found Quentin Monnet
@ 2020-07-24 9:06 ` Quentin Monnet
2020-07-27 21:15 ` Song Liu
2020-07-28 10:16 ` [PATCH bpf-next 0/2] tools: bpftool: update prog names list and fix segfault Daniel Borkmann
2 siblings, 1 reply; 6+ messages in thread
From: Quentin Monnet @ 2020-07-24 9:06 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: bpf, netdev, Paul Chaignon, Quentin Monnet
Assign "lsm" as a printed name for BPF_PROG_TYPE_LSM in bpftool, so that
it can use it when listing programs loaded on the system or when probing
features.
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
tools/bpf/bpftool/prog.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 3e6ecc6332e2..158995d853b0 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -59,6 +59,7 @@ const char * const prog_type_name[] = {
[BPF_PROG_TYPE_TRACING] = "tracing",
[BPF_PROG_TYPE_STRUCT_OPS] = "struct_ops",
[BPF_PROG_TYPE_EXT] = "ext",
+ [BPF_PROG_TYPE_LSM] = "lsm",
[BPF_PROG_TYPE_SK_LOOKUP] = "sk_lookup",
};
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH bpf-next 0/2] tools: bpftool: update prog names list and fix segfault
2020-07-24 9:06 [PATCH bpf-next 0/2] tools: bpftool: update prog names list and fix segfault Quentin Monnet
2020-07-24 9:06 ` [PATCH bpf-next 1/2] tools: bpftool: skip type probe if name is not found Quentin Monnet
2020-07-24 9:06 ` [PATCH bpf-next 2/2] tools: bpftool: add LSM type to array of prog names Quentin Monnet
@ 2020-07-28 10:16 ` Daniel Borkmann
2 siblings, 0 replies; 6+ messages in thread
From: Daniel Borkmann @ 2020-07-28 10:16 UTC (permalink / raw)
To: Quentin Monnet, Alexei Starovoitov; +Cc: bpf, netdev, Paul Chaignon
On 7/24/20 11:06 AM, Quentin Monnet wrote:
> Although probing features with bpftool works fine if bpftool's array of
> program and map type names lack the latest kernel additions, it can crash
> if there are some types missing _in the middle_ of the arrays. The case
> recently occurred with the addition of the "sk_lookup" name, which skipped
> the "lsm" in the list.
>
> Let's update the list, and let's make sure it does not crash bpftool again
> if we omit other types again in the future.
>
> Quentin Monnet (2):
> tools: bpftool: skip type probe if name is not found
> tools: bpftool: add LSM type to array of prog names
>
> tools/bpf/bpftool/feature.c | 8 ++++++++
> tools/bpf/bpftool/prog.c | 1 +
> 2 files changed, 9 insertions(+)
>
Applied, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread