[bpf-next,v1,2/3] bpf: btf: Ensure natural alignment of .BTF_ids section

Commit Message

Tony Ambardar Sept. 20, 2024, 7:49 a.m. UTC

While building of vmlinux employs a linker script to align the .BTF_ids
section to 4 bytes, other usage leaves .BTF_ids unaligned and may lead to
problems (e.g. [1]). Post-processing and libelf-based endian translation by
resolve_btfids may also potentially suffer from misalignment.

Update encoding macros in btf_ids.h to always align BTF ID data to 4 bytes.

[1]: 3effc06a4dde ("selftests/bpf: Fix alignment of .BTF_ids")

Signed-off-by: Tony Ambardar <[e-mail]>
---
 include/linux/btf_ids.h | 1 +
 1 file changed, 1 insertion(+)

Comments

Eduard Zingerman Sept. 21, 2024, 9:46 a.m. UTC | #1

On Fri, 2024-09-20 at 00:49 -0700, Tony Ambardar wrote:
> While building of vmlinux employs a linker script to align the .BTF_ids
> section to 4 bytes, other usage leaves .BTF_ids unaligned and may lead to
> problems (e.g. [1]). Post-processing and libelf-based endian translation by
> resolve_btfids may also potentially suffer from misalignment.
> 
> Update encoding macros in btf_ids.h to always align BTF ID data to 4 bytes.
> 
> [1]: 3effc06a4dde ("selftests/bpf: Fix alignment of .BTF_ids")
> 
> Signed-off-by: Tony Ambardar <[e-mail]>
> ---
>  include/linux/btf_ids.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> index c0e3e1426a82..c10b163dc340 100644
> --- a/include/linux/btf_ids.h
> +++ b/include/linux/btf_ids.h
> @@ -89,6 +89,7 @@ word							\
>  #define __BTF_ID_LIST(name, scope)			\
>  asm(							\
>  ".pushsection " BTF_IDS_SECTION ",\"a\";       \n"	\
> +".balign 4, 0;                                 \n"	\
>  "." #scope " " #name ";                        \n"	\
>  #name ":;                                      \n"	\
>  ".popsection;                                  \n");

This forces all id list symbols to be aligned on 4 bytes.
Should the same be done for __BTF_SET_START?

Also, is it guaranteed that all btf ids are organized in lists and sets?
Grepping through the code it seems they are, but it looks like resolve_btfids
does not really enforce this, simply looking for symbols matching a special name
__BTF_ID__<type>__<symbol>[__<id>] .

Jiri Olsa Sept. 23, 2024, 10:54 a.m. UTC | #2

On Sat, Sep 21, 2024 at 02:46:17AM -0700, Eduard Zingerman wrote:
> On Fri, 2024-09-20 at 00:49 -0700, Tony Ambardar wrote:
> > While building of vmlinux employs a linker script to align the .BTF_ids
> > section to 4 bytes, other usage leaves .BTF_ids unaligned and may lead to
> > problems (e.g. [1]). Post-processing and libelf-based endian translation by
> > resolve_btfids may also potentially suffer from misalignment.
> > 
> > Update encoding macros in btf_ids.h to always align BTF ID data to 4 bytes.
> > 
> > [1]: 3effc06a4dde ("selftests/bpf: Fix alignment of .BTF_ids")
> > 
> > Signed-off-by: Tony Ambardar <[e-mail]>
> > ---
> >  include/linux/btf_ids.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> > index c0e3e1426a82..c10b163dc340 100644
> > --- a/include/linux/btf_ids.h
> > +++ b/include/linux/btf_ids.h
> > @@ -89,6 +89,7 @@ word							\
> >  #define __BTF_ID_LIST(name, scope)			\
> >  asm(							\
> >  ".pushsection " BTF_IDS_SECTION ",\"a\";       \n"	\
> > +".balign 4, 0;                                 \n"	\
> >  "." #scope " " #name ";                        \n"	\
> >  #name ":;                                      \n"	\
> >  ".popsection;                                  \n");
> 
> This forces all id list symbols to be aligned on 4 bytes.
> Should the same be done for __BTF_SET_START?

it seems all the set macros use __BTF_ID_LIST, so it should be taken
care of by that

> 
> Also, is it guaranteed that all btf ids are organized in lists and sets?
> Grepping through the code it seems they are, but it looks like resolve_btfids
> does not really enforce this, simply looking for symbols matching a special name
> __BTF_ID__<type>__<symbol>[__<id>] .

yes, you need the BTF_ID to be part of list or set to be able to access it

resolve_btfids does not enforce some loose BTF_ID definition without list/set,
but that does not seem to be a problem

thanks,
jirka

Eduard Zingerman Sept. 23, 2024, 11:39 a.m. UTC | #3

On Mon, 2024-09-23 at 12:54 +0200, Jiri Olsa wrote:

[...]

> > > diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> > > index c0e3e1426a82..c10b163dc340 100644
> > > --- a/include/linux/btf_ids.h
> > > +++ b/include/linux/btf_ids.h
> > > @@ -89,6 +89,7 @@ word							\
> > >  #define __BTF_ID_LIST(name, scope)			\
> > >  asm(							\
> > >  ".pushsection " BTF_IDS_SECTION ",\"a\";       \n"	\
> > > +".balign 4, 0;                                 \n"	\
> > >  "." #scope " " #name ";                        \n"	\
> > >  #name ":;                                      \n"	\
> > >  ".popsection;                                  \n");
> > 
> > This forces all id list symbols to be aligned on 4 bytes.
> > Should the same be done for __BTF_SET_START?
> 
> it seems all the set macros use __BTF_ID_LIST, so it should be taken
> care of by that

Apologies, I don't know how I missed __BTF_ID_LIST invocation in
the __BTF_SET_START definition :(

> > Also, is it guaranteed that all btf ids are organized in lists and sets?
> > Grepping through the code it seems they are, but it looks like resolve_btfids
> > does not really enforce this, simply looking for symbols matching a special name
> > __BTF_ID__<type>__<symbol>[__<id>] .
> 
> yes, you need the BTF_ID to be part of list or set to be able to access it
> 
> resolve_btfids does not enforce some loose BTF_ID definition without list/set,
> but that does not seem to be a problem

Understood, thank you.

Patch

diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
index c0e3e1426a82..c10b163dc340 100644
--- a/include/linux/btf_ids.h
+++ b/include/linux/btf_ids.h
@@ -89,6 +89,7 @@  word							\
 #define __BTF_ID_LIST(name, scope)			\
 asm(							\
 ".pushsection " BTF_IDS_SECTION ",\"a\";       \n"	\
+".balign 4, 0;                                 \n"	\
 "." #scope " " #name ";                        \n"	\
 #name ":;                                      \n"	\
 ".popsection;                                  \n");