* [PATCH bpf] tools, bpftool: check return value of function codegen
@ 2020-06-10 13:08 Tobias Klauser
2020-06-10 18:50 ` Andrii Nakryiko
2020-06-11 10:33 ` [PATCH] tools, bpftool: Exit on error in " Tobias Klauser
0 siblings, 2 replies; 6+ messages in thread
From: Tobias Klauser @ 2020-06-10 13:08 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: bpf
The codegen function might fail an return an error. Check its return
value in all call sites and handle it properly.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
tools/bpf/bpftool/gen.c | 32 +++++++++++++++++++++++---------
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index ecbae47e66b8..b5fa3060dce3 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -325,7 +325,7 @@ static int do_skeleton(int argc, char **argv)
}
get_header_guard(header_guard, obj_name);
- codegen("\
+ err = codegen("\
\n\
/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */ \n\
\n\
@@ -342,6 +342,8 @@ static int do_skeleton(int argc, char **argv)
",
obj_name, header_guard
);
+ if (err)
+ goto out;
if (map_cnt) {
printf("\tstruct {\n");
@@ -376,7 +378,7 @@ static int do_skeleton(int argc, char **argv)
goto out;
}
- codegen("\
+ err = codegen("\
\n\
}; \n\
\n\
@@ -453,8 +455,10 @@ static int do_skeleton(int argc, char **argv)
",
obj_name
);
+ if (err)
+ goto out;
- codegen("\
+ err = codegen("\
\n\
\n\
static inline int \n\
@@ -474,7 +478,7 @@ static int do_skeleton(int argc, char **argv)
obj_name
);
if (map_cnt) {
- codegen("\
+ err = codegen("\
\n\
\n\
/* maps */ \n\
@@ -486,6 +490,9 @@ static int do_skeleton(int argc, char **argv)
",
map_cnt
);
+ if (err)
+ goto out;
+
i = 0;
bpf_object__for_each_map(map, obj) {
ident = get_map_ident(map);
@@ -493,13 +500,16 @@ static int do_skeleton(int argc, char **argv)
if (!ident)
continue;
- codegen("\
+ err = codegen("\
\n\
\n\
s->maps[%zu].name = \"%s\"; \n\
s->maps[%zu].map = &obj->maps.%s; \n\
",
i, bpf_map__name(map), i, ident);
+ if (err)
+ goto out;
+
/* memory-mapped internal maps */
if (bpf_map__is_internal(map) &&
(bpf_map__def(map)->map_flags & BPF_F_MMAPABLE)) {
@@ -510,7 +520,7 @@ static int do_skeleton(int argc, char **argv)
}
}
if (prog_cnt) {
- codegen("\
+ err = codegen("\
\n\
\n\
/* programs */ \n\
@@ -522,6 +532,9 @@ static int do_skeleton(int argc, char **argv)
",
prog_cnt
);
+ if (err)
+ goto out;
+
i = 0;
bpf_object__for_each_program(prog, obj) {
codegen("\
@@ -535,13 +548,15 @@ static int do_skeleton(int argc, char **argv)
i++;
}
}
- codegen("\
+ err = codegen("\
\n\
\n\
s->data_sz = %d; \n\
s->data = (void *)\"\\ \n\
",
file_sz);
+ if (err)
+ goto out;
/* embed contents of BPF object file */
for (i = 0, len = 0; i < file_sz; i++) {
@@ -558,7 +573,7 @@ static int do_skeleton(int argc, char **argv)
printf("\\x%02x", (unsigned char)obj_data[i]);
}
- codegen("\
+ err = codegen("\
\n\
\"; \n\
\n\
@@ -571,7 +586,6 @@ static int do_skeleton(int argc, char **argv)
#endif /* %s */ \n\
",
header_guard);
- err = 0;
out:
bpf_object__close(obj);
if (obj_data)
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] tools, bpftool: check return value of function codegen
2020-06-10 13:08 [PATCH bpf] tools, bpftool: check return value of function codegen Tobias Klauser
@ 2020-06-10 18:50 ` Andrii Nakryiko
2020-06-11 9:05 ` Tobias Klauser
2020-06-11 10:33 ` [PATCH] tools, bpftool: Exit on error in " Tobias Klauser
1 sibling, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2020-06-10 18:50 UTC (permalink / raw)
To: Tobias Klauser; +Cc: Alexei Starovoitov, Daniel Borkmann, bpf
On Wed, Jun 10, 2020 at 6:09 AM Tobias Klauser <tklauser@distanz.ch> wrote:
>
> The codegen function might fail an return an error. Check its return
> value in all call sites and handle it properly.
>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> ---
codegen() can fail only if the system ran out of memory or the static
template is malformed. Both are highly unlikely. I wonder if the
better approach would be to just exit(1) on such an unlikely error
inside codegen() and make the function itself void-returning.
We'll probably expand codegen to other languages soon, so not having
to do those annoying error checks everywhere is a good thing.
What do you think?
[...]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] tools, bpftool: check return value of function codegen
2020-06-10 18:50 ` Andrii Nakryiko
@ 2020-06-11 9:05 ` Tobias Klauser
0 siblings, 0 replies; 6+ messages in thread
From: Tobias Klauser @ 2020-06-11 9:05 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: Alexei Starovoitov, Daniel Borkmann, bpf
On 2020-06-10 at 20:50:06 +0200, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> On Wed, Jun 10, 2020 at 6:09 AM Tobias Klauser <tklauser@distanz.ch> wrote:
> >
> > The codegen function might fail an return an error. Check its return
> > value in all call sites and handle it properly.
> >
> > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > ---
>
> codegen() can fail only if the system ran out of memory or the static
> template is malformed. Both are highly unlikely. I wonder if the
> better approach would be to just exit(1) on such an unlikely error
> inside codegen() and make the function itself void-returning.
>
> We'll probably expand codegen to other languages soon, so not having
> to do those annoying error checks everywhere is a good thing.
>
> What do you think?
Sounds good to me, thanks. I'll send a v2 implementing your suggestion.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] tools, bpftool: Exit on error in function codegen
2020-06-10 13:08 [PATCH bpf] tools, bpftool: check return value of function codegen Tobias Klauser
2020-06-10 18:50 ` Andrii Nakryiko
@ 2020-06-11 10:33 ` Tobias Klauser
2020-06-11 18:02 ` Andrii Nakryiko
1 sibling, 1 reply; 6+ messages in thread
From: Tobias Klauser @ 2020-06-11 10:33 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko; +Cc: bpf
Currently, the codegen function might fail and return an error. But its
callers continue without checking its return value. Since codegen can
fail only in the ounlikely case of the system running out of memory or
the static template being malformed, just exit(-1) directly from codegen
and make it void-returning.
Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
Replaces https://lore.kernel.org/bpf/20200610130807.21497-1-tklauser@distanz.ch/
and to be applied on top of
https://lore.kernel.org/bpf/20200610130804.21423-1-tklauser@distanz.ch/
tools/bpf/bpftool/gen.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index ecbae47e66b8..7443879e87af 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -200,7 +200,7 @@ static int codegen_datasecs(struct bpf_object *obj, const char *obj_name)
return err;
}
-static int codegen(const char *template, ...)
+static void codegen(const char *template, ...)
{
const char *src, *end;
int skip_tabs = 0, n;
@@ -211,7 +211,7 @@ static int codegen(const char *template, ...)
n = strlen(template);
s = malloc(n + 1);
if (!s)
- return -ENOMEM;
+ exit(-1);
src = template;
dst = s;
@@ -225,7 +225,7 @@ static int codegen(const char *template, ...)
p_err("unrecognized character at pos %td in template '%s'",
src - template - 1, template);
free(s);
- return -EINVAL;
+ exit(-1);
}
}
@@ -236,7 +236,7 @@ static int codegen(const char *template, ...)
p_err("not enough tabs at pos %td in template '%s'",
src - template - 1, template);
free(s);
- return -EINVAL;
+ exit(-1);
}
}
/* trim trailing whitespace */
@@ -257,7 +257,8 @@ static int codegen(const char *template, ...)
va_end(args);
free(s);
- return n;
+ if (n)
+ exit(-1);
}
static int do_skeleton(int argc, char **argv)
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] tools, bpftool: Exit on error in function codegen
2020-06-11 10:33 ` [PATCH] tools, bpftool: Exit on error in " Tobias Klauser
@ 2020-06-11 18:02 ` Andrii Nakryiko
2020-06-11 21:55 ` Daniel Borkmann
0 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2020-06-11 18:02 UTC (permalink / raw)
To: Tobias Klauser; +Cc: Alexei Starovoitov, Daniel Borkmann, bpf
On Thu, Jun 11, 2020 at 3:33 AM Tobias Klauser <tklauser@distanz.ch> wrote:
>
> Currently, the codegen function might fail and return an error. But its
> callers continue without checking its return value. Since codegen can
> fail only in the ounlikely case of the system running out of memory or
> the static template being malformed, just exit(-1) directly from codegen
> and make it void-returning.
>
> Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> ---
LGTM. Thanks!
Acked-by: Andrii Nakryiko <andriin@fb.com>
[...]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tools, bpftool: Exit on error in function codegen
2020-06-11 18:02 ` Andrii Nakryiko
@ 2020-06-11 21:55 ` Daniel Borkmann
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Borkmann @ 2020-06-11 21:55 UTC (permalink / raw)
To: Andrii Nakryiko, Tobias Klauser; +Cc: Alexei Starovoitov, bpf
On 6/11/20 8:02 PM, Andrii Nakryiko wrote:
> On Thu, Jun 11, 2020 at 3:33 AM Tobias Klauser <tklauser@distanz.ch> wrote:
>>
>> Currently, the codegen function might fail and return an error. But its
>> callers continue without checking its return value. Since codegen can
>> fail only in the ounlikely case of the system running out of memory or
>> the static template being malformed, just exit(-1) directly from codegen
>> and make it void-returning.
>>
>> Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
>> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
>> ---
>
> LGTM. Thanks!
>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
Applied, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-11 21:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-10 13:08 [PATCH bpf] tools, bpftool: check return value of function codegen Tobias Klauser
2020-06-10 18:50 ` Andrii Nakryiko
2020-06-11 9:05 ` Tobias Klauser
2020-06-11 10:33 ` [PATCH] tools, bpftool: Exit on error in " Tobias Klauser
2020-06-11 18:02 ` Andrii Nakryiko
2020-06-11 21:55 ` Daniel Borkmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).