diff mbox series

scripts/mksysmap: Ignore prefixed KCFI symbols

Message ID 20230623140825.ehqk5ndl7uftstwy@google.com (mailing list archive)
State New, archived
Headers show
Series scripts/mksysmap: Ignore prefixed KCFI symbols | expand

Commit Message

Pierre-Clément Tosi June 23, 2023, 2:08 p.m. UTC
The (relatively) new KCFI feature in LLVM/Clang encodes type information
for C functions by generating symbols named __kcfi_typeid_<fname>, which
can then be referenced from assembly. However, some custom build rules
(e.g. EFI, nVHE, or early PIE on arm64) use objcopy to add a prefix to
all the symbols in their object files, making mksysmap's ignore filter
miss those KCFI symbols.

Therefore, explicitly list those twice-prefixed KCFI symbols as ignored.

Alternatively, this could also be achieved in a less verbose way by
ignoring any symbol containing the string "__kcfi_typeid_". However,
listing the combined prefixes explicitly saves us from running the small
risk of ignoring symbols that should be kept.

Signed-off-by: Pierre-Clément Tosi <ptosi@google.com>
---
 scripts/mksysmap | 3 +++
 1 file changed, 3 insertions(+)

Comments

Sami Tolvanen June 23, 2023, 8:45 p.m. UTC | #1
Hi Pierre-Clément,

On Fri, Jun 23, 2023 at 7:08 AM Pierre-Clément Tosi <ptosi@google.com> wrote:
>
> The (relatively) new KCFI feature in LLVM/Clang encodes type information
> for C functions by generating symbols named __kcfi_typeid_<fname>, which
> can then be referenced from assembly. However, some custom build rules
> (e.g. EFI, nVHE, or early PIE on arm64) use objcopy to add a prefix to
> all the symbols in their object files, making mksysmap's ignore filter
> miss those KCFI symbols.
>
> Therefore, explicitly list those twice-prefixed KCFI symbols as ignored.
>
> Alternatively, this could also be achieved in a less verbose way by
> ignoring any symbol containing the string "__kcfi_typeid_". However,
> listing the combined prefixes explicitly saves us from running the small
> risk of ignoring symbols that should be kept.
>
> Signed-off-by: Pierre-Clément Tosi <ptosi@google.com>
> ---
>  scripts/mksysmap | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/scripts/mksysmap b/scripts/mksysmap
> index 26f39772f7a5..17cf4292e26b 100755
> --- a/scripts/mksysmap
> +++ b/scripts/mksysmap
> @@ -61,7 +61,10 @@ ${NM} -n ${1} | sed >${2} -e "
>  / __microLA25Thunk_/d
>
>  # CFI type identifiers
> +/ __efistub___kcfi_typeid_/d

Does the existing __efistub_/d rule not catch this?

>  / __kcfi_typeid_/d
> +/ __kvm_nvhe___kcfi_typeid_/d
> +/ __pi___kcfi_typeid_/d

Either way, otherwise this looks reasonable to me. Thanks for the patch!

Reviewed-by: Sami Tolvanen <samitolvanen@google.com>

Sami
Masahiro Yamada June 24, 2023, 9:02 a.m. UTC | #2
On Sat, Jun 24, 2023 at 5:45 AM Sami Tolvanen <samitolvanen@google.com> wrote:
>
> Hi Pierre-Clément,
>
> On Fri, Jun 23, 2023 at 7:08 AM Pierre-Clément Tosi <ptosi@google.com> wrote:
> >
> > The (relatively) new KCFI feature in LLVM/Clang encodes type information
> > for C functions by generating symbols named __kcfi_typeid_<fname>, which
> > can then be referenced from assembly. However, some custom build rules
> > (e.g. EFI, nVHE, or early PIE on arm64) use objcopy to add a prefix to
> > all the symbols in their object files, making mksysmap's ignore filter
> > miss those KCFI symbols.
> >
> > Therefore, explicitly list those twice-prefixed KCFI symbols as ignored.
> >
> > Alternatively, this could also be achieved in a less verbose way by
> > ignoring any symbol containing the string "__kcfi_typeid_". However,
> > listing the combined prefixes explicitly saves us from running the small
> > risk of ignoring symbols that should be kept.
> >
> > Signed-off-by: Pierre-Clément Tosi <ptosi@google.com>
> > ---
> >  scripts/mksysmap | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/scripts/mksysmap b/scripts/mksysmap
> > index 26f39772f7a5..17cf4292e26b 100755
> > --- a/scripts/mksysmap
> > +++ b/scripts/mksysmap
> > @@ -61,7 +61,10 @@ ${NM} -n ${1} | sed >${2} -e "
> >  / __microLA25Thunk_/d
> >
> >  # CFI type identifiers
> > +/ __efistub___kcfi_typeid_/d
>
> Does the existing __efistub_/d rule not catch this?


Agree with Sami.

This line looks redundant to me.




>
> >  / __kcfi_typeid_/d
> > +/ __kvm_nvhe___kcfi_typeid_/d
> > +/ __pi___kcfi_typeid_/d
>
> Either way, otherwise this looks reasonable to me. Thanks for the patch!
>
> Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
>
> Sami
diff mbox series

Patch

diff --git a/scripts/mksysmap b/scripts/mksysmap
index 26f39772f7a5..17cf4292e26b 100755
--- a/scripts/mksysmap
+++ b/scripts/mksysmap
@@ -61,7 +61,10 @@  ${NM} -n ${1} | sed >${2} -e "
 / __microLA25Thunk_/d
 
 # CFI type identifiers
+/ __efistub___kcfi_typeid_/d
 / __kcfi_typeid_/d
+/ __kvm_nvhe___kcfi_typeid_/d
+/ __pi___kcfi_typeid_/d
 
 # CRC from modversions
 / __crc_/d