* [PATCH v4 1/2] bpftool: Align output skeleton ELF code
@ 2023-10-04 22:23 Ian Rogers
2023-10-04 22:23 ` [PATCH v4 2/2] bpftool: Align bpf_load_and_run_opts insns and data Ian Rogers
2023-10-05 15:51 ` [PATCH v4 1/2] bpftool: Align output skeleton ELF code Quentin Monnet
0 siblings, 2 replies; 5+ messages in thread
From: Ian Rogers @ 2023-10-04 22:23 UTC (permalink / raw)
To: Quentin Monnet, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
bpf, linux-kernel
Cc: Ian Rogers, Alan Maguire
libbpf accesses the ELF data requiring at least 8 byte alignment,
however, the data is generated into a C string that doesn't guarantee
alignment. Fix this by assigning to an aligned char array. Use sizeof
on the array, less one for the \0 terminator, rather than generating a
constant.
Fixes: a6cc6b34b93e ("bpftool: Provide a helper method for accessing skeleton's embedded ELF data")
Signed-off-by: Ian Rogers <irogers@google.com>
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
---
tools/bpf/bpftool/gen.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 2883660d6b67..b8ebcee9bc56 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -1209,7 +1209,7 @@ static int do_skeleton(int argc, char **argv)
codegen("\
\n\
\n\
- s->data = (void *)%2$s__elf_bytes(&s->data_sz); \n\
+ s->data = (void *)%1$s__elf_bytes(&s->data_sz); \n\
\n\
obj->skeleton = s; \n\
return 0; \n\
@@ -1218,12 +1218,12 @@ static int do_skeleton(int argc, char **argv)
return err; \n\
} \n\
\n\
- static inline const void *%2$s__elf_bytes(size_t *sz) \n\
+ static inline const void *%1$s__elf_bytes(size_t *sz) \n\
{ \n\
- *sz = %1$d; \n\
- return (const void *)\"\\ \n\
- "
- , file_sz, obj_name);
+ static const char data[] __attribute__((__aligned__(8))) = \"\\\n\
+ ",
+ obj_name
+ );
/* embed contents of BPF object file */
print_hex(obj_data, file_sz);
@@ -1231,6 +1231,9 @@ static int do_skeleton(int argc, char **argv)
codegen("\
\n\
\"; \n\
+ \n\
+ *sz = sizeof(data) - 1; \n\
+ return (const void *)data; \n\
} \n\
\n\
#ifdef __cplusplus \n\
--
2.42.0.609.gbb76f46606-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v4 2/2] bpftool: Align bpf_load_and_run_opts insns and data 2023-10-04 22:23 [PATCH v4 1/2] bpftool: Align output skeleton ELF code Ian Rogers @ 2023-10-04 22:23 ` Ian Rogers 2023-10-05 15:54 ` Quentin Monnet 2023-10-05 15:51 ` [PATCH v4 1/2] bpftool: Align output skeleton ELF code Quentin Monnet 1 sibling, 1 reply; 5+ messages in thread From: Ian Rogers @ 2023-10-04 22:23 UTC (permalink / raw) To: Quentin Monnet, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf, linux-kernel Cc: Ian Rogers A C string lacks alignment so use aligned arrays to avoid potential alignment problems. Switch to using sizeof (less 1 for the \0 terminator) rather than a hardcode size constant. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/bpf/bpftool/gen.c | 47 ++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c index b8ebcee9bc56..7a545dcabe38 100644 --- a/tools/bpf/bpftool/gen.c +++ b/tools/bpf/bpftool/gen.c @@ -408,8 +408,8 @@ static void codegen(const char *template, ...) /* skip baseline indentation tabs */ for (n = skip_tabs; n > 0; n--, src++) { if (*src != '\t') { - p_err("not enough tabs at pos %td in template '%s'", - src - template - 1, template); + p_err("not enough tabs at pos %td in template '%s'\n'%s'", + src - template - 1, template, src); free(s); exit(-1); } @@ -708,17 +708,22 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h codegen("\ \n\ - skel->%1$s = skel_prep_map_data((void *)\"\\ \n\ - ", ident); + { \n\ + static const char data[] __attribute__((__aligned__(8))) = \"\\\n\ + "); mmap_data = bpf_map__initial_value(map, &mmap_size); print_hex(mmap_data, mmap_size); codegen("\ \n\ - \", %1$zd, %2$zd); \n\ - if (!skel->%3$s) \n\ - goto cleanup; \n\ - skel->maps.%3$s.initial_value = (__u64) (long) skel->%3$s;\n\ - ", bpf_map_mmap_sz(map), mmap_size, ident); + \"; \n\ + \n\ + skel->%1$s = skel_prep_map_data((void *)data, %2$zd,\n\ + sizeof(data) - 1);\n\ + if (!skel->%1$s) \n\ + goto cleanup; \n\ + skel->maps.%1$s.initial_value = (__u64) (long) skel->%1$s;\n\ + } \n\ + ", ident, bpf_map_mmap_sz(map)); } codegen("\ \n\ @@ -733,32 +738,30 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h { \n\ struct bpf_load_and_run_opts opts = {}; \n\ int err; \n\ - \n\ - opts.ctx = (struct bpf_loader_ctx *)skel; \n\ - opts.data_sz = %2$d; \n\ - opts.data = (void *)\"\\ \n\ + static const char opts_data[] __attribute__((__aligned__(8))) = \"\\\n\ ", - obj_name, opts.data_sz); + obj_name); print_hex(opts.data, opts.data_sz); codegen("\ \n\ \"; \n\ + static const char opts_insn[] __attribute__((__aligned__(8))) = \"\\\n\ "); - - codegen("\ - \n\ - opts.insns_sz = %d; \n\ - opts.insns = (void *)\"\\ \n\ - ", - opts.insns_sz); print_hex(opts.insns, opts.insns_sz); codegen("\ \n\ \"; \n\ + \n\ + opts.ctx = (struct bpf_loader_ctx *)skel; \n\ + opts.data_sz = sizeof(opts_data) - 1; \n\ + opts.data = (void *)opts_data; \n\ + opts.insns_sz = sizeof(opts_insn) - 1; \n\ + opts.insns = (void *)opts_insn; \n\ + \n\ err = bpf_load_and_run(&opts); \n\ if (err < 0) \n\ return err; \n\ - ", obj_name); + "); bpf_object__for_each_map(map, obj) { const char *mmap_flags; -- 2.42.0.609.gbb76f46606-goog ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4 2/2] bpftool: Align bpf_load_and_run_opts insns and data 2023-10-04 22:23 ` [PATCH v4 2/2] bpftool: Align bpf_load_and_run_opts insns and data Ian Rogers @ 2023-10-05 15:54 ` Quentin Monnet 2023-10-05 22:03 ` Ian Rogers 0 siblings, 1 reply; 5+ messages in thread From: Quentin Monnet @ 2023-10-05 15:54 UTC (permalink / raw) To: Ian Rogers, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf, linux-kernel On 04/10/2023 23:23, Ian Rogers wrote: > A C string lacks alignment so use aligned arrays to avoid potential > alignment problems. Switch to using sizeof (less 1 for the \0 > terminator) rather than a hardcode size constant. > > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/bpf/bpftool/gen.c | 47 ++++++++++++++++++++++------------------- > 1 file changed, 25 insertions(+), 22 deletions(-) > > diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c > index b8ebcee9bc56..7a545dcabe38 100644 > --- a/tools/bpf/bpftool/gen.c > +++ b/tools/bpf/bpftool/gen.c > @@ -408,8 +408,8 @@ static void codegen(const char *template, ...) > /* skip baseline indentation tabs */ > for (n = skip_tabs; n > 0; n--, src++) { > if (*src != '\t') { > - p_err("not enough tabs at pos %td in template '%s'", > - src - template - 1, template); > + p_err("not enough tabs at pos %td in template '%s'\n'%s'", > + src - template - 1, template, src); Nit: This line is no longer aligned with the opening parenthesis. Other than that: Acked-by: Quentin Monnet <quenti@isovalent.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 2/2] bpftool: Align bpf_load_and_run_opts insns and data 2023-10-05 15:54 ` Quentin Monnet @ 2023-10-05 22:03 ` Ian Rogers 0 siblings, 0 replies; 5+ messages in thread From: Ian Rogers @ 2023-10-05 22:03 UTC (permalink / raw) To: Quentin Monnet Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf, linux-kernel On Thu, Oct 5, 2023 at 8:54 AM Quentin Monnet <quentin@isovalent.com> wrote: > > On 04/10/2023 23:23, Ian Rogers wrote: > > A C string lacks alignment so use aligned arrays to avoid potential > > alignment problems. Switch to using sizeof (less 1 for the \0 > > terminator) rather than a hardcode size constant. > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > --- > > tools/bpf/bpftool/gen.c | 47 ++++++++++++++++++++++------------------- > > 1 file changed, 25 insertions(+), 22 deletions(-) > > > > diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c > > index b8ebcee9bc56..7a545dcabe38 100644 > > --- a/tools/bpf/bpftool/gen.c > > +++ b/tools/bpf/bpftool/gen.c > > @@ -408,8 +408,8 @@ static void codegen(const char *template, ...) > > /* skip baseline indentation tabs */ > > for (n = skip_tabs; n > 0; n--, src++) { > > if (*src != '\t') { > > - p_err("not enough tabs at pos %td in template '%s'", > > - src - template - 1, template); > > + p_err("not enough tabs at pos %td in template '%s'\n'%s'", > > + src - template - 1, template, src); > > Nit: This line is no longer aligned with the opening parenthesis. > > Other than that: > > Acked-by: Quentin Monnet <quenti@isovalent.com> Thanks! Nit fixed in v5. Ian ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/2] bpftool: Align output skeleton ELF code 2023-10-04 22:23 [PATCH v4 1/2] bpftool: Align output skeleton ELF code Ian Rogers 2023-10-04 22:23 ` [PATCH v4 2/2] bpftool: Align bpf_load_and_run_opts insns and data Ian Rogers @ 2023-10-05 15:51 ` Quentin Monnet 1 sibling, 0 replies; 5+ messages in thread From: Quentin Monnet @ 2023-10-05 15:51 UTC (permalink / raw) To: Ian Rogers, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf, linux-kernel Cc: Alan Maguire On 04/10/2023 23:23, Ian Rogers wrote: > libbpf accesses the ELF data requiring at least 8 byte alignment, > however, the data is generated into a C string that doesn't guarantee > alignment. Fix this by assigning to an aligned char array. Use sizeof > on the array, less one for the \0 terminator, rather than generating a > constant. > > Fixes: a6cc6b34b93e ("bpftool: Provide a helper method for accessing skeleton's embedded ELF data") > Signed-off-by: Ian Rogers <irogers@google.com> > Reviewed-by: Alan Maguire <alan.maguire@oracle.com> Acked-by: Quentin Monnet <quentin@isovalent.com> Thanks ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-10-05 22:03 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-04 22:23 [PATCH v4 1/2] bpftool: Align output skeleton ELF code Ian Rogers 2023-10-04 22:23 ` [PATCH v4 2/2] bpftool: Align bpf_load_and_run_opts insns and data Ian Rogers 2023-10-05 15:54 ` Quentin Monnet 2023-10-05 22:03 ` Ian Rogers 2023-10-05 15:51 ` [PATCH v4 1/2] bpftool: Align output skeleton ELF code Quentin Monnet
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox