Commit 31cb50b5 authored by Masahiro Yamada's avatar Masahiro Yamada

kbuild: check static EXPORT_SYMBOL* by script instead of modpost

The 'static' specifier and EXPORT_SYMBOL() are an odd combination.

Commit 15bfc234 ("modpost: check for static EXPORT_SYMBOL*
functions") tried to detect it, but this check has false negatives.

Here is the sample code.

  Makefile:

    obj-y += foo1.o foo2.o

  foo1.c:

    #include <linux/export.h>
    static void foo(void) {}
    EXPORT_SYMBOL(foo);

  foo2.c:

    void foo(void) {}

foo1.c exports the static symbol 'foo', but modpost cannot catch it
because it is fooled by foo2.c, which has a global symbol with the
same name.

s->is_static is cleared if a global symbol with the same name is found
somewhere, but EXPORT_SYMBOL() and the global symbol do not necessarily
belong to the same compilation unit.

This check should be done per compilation unit, but I do not know how
to do it in modpost. modpost runs against vmlinux.o or modules, which
merges multiple objects, then forgets their origin.

modpost cannot parse individual objects because they may not be ELF but
LLVM IR when CONFIG_LTO_CLANG=y.

Add a simple bash script to parse the output from ${NM}. This works for
CONFIG_LTO_CLANG=y because llvm-nm can dump symbols of LLVM IR files.

Revert 15bfc234.
Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
Reviewed-by: default avatarNick Desaulniers <ndesaulniers@google.com>
Tested-by: default avatarNathan Chancellor <nathan@kernel.org>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM-14 (x86-64)
parent 534671e0
......@@ -249,9 +249,12 @@ cmd_gen_ksymdeps = \
$(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
endif
cmd_check_local_export = $(srctree)/scripts/check-local-export $@
define rule_cc_o_c
$(call cmd_and_fixdep,cc_o_c)
$(call cmd,gen_ksymdeps)
$(call cmd,check_local_export)
$(call cmd,checksrc)
$(call cmd,checkdoc)
$(call cmd,gen_objtooldep)
......@@ -262,6 +265,7 @@ endef
define rule_as_o_S
$(call cmd_and_fixdep,as_o_S)
$(call cmd,gen_ksymdeps)
$(call cmd,check_local_export)
$(call cmd,gen_objtooldep)
$(call cmd,gen_symversions_S)
endef
......
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0-only
#
# Copyright (C) 2022 Masahiro Yamada <masahiroy@kernel.org>
#
# Exit with error if a local exported symbol is found.
# EXPORT_SYMBOL should be used for global symbols.
set -e
declare -A symbol_types
declare -a export_symbols
exit_code=0
while read value type name
do
# Skip the line if the number of fields is less than 3.
#
# case 1)
# For undefined symbols, the first field (value) is empty.
# The outout looks like this:
# " U _printk"
# It is unneeded to record undefined symbols.
#
# case 2)
# For Clang LTO, llvm-nm outputs a line with type 't' but empty name:
# "---------------- t"
if [[ -z ${name} ]]; then
continue
fi
# save (name, type) in the associative array
symbol_types[${name}]=${type}
# append the exported symbol to the array
if [[ ${name} == __ksymtab_* ]]; then
export_symbols+=(${name#__ksymtab_})
fi
# If there is no symbol in the object, ${NM} (both GNU nm and llvm-nm)
# shows 'no symbols' diagnostic (but exits with 0). It is harmless and
# hidden by '2>/dev/null'. However, it suppresses real error messages
# as well. Add a hand-crafted error message here.
#
# Use --quiet instead of 2>/dev/null when we upgrade the minimum version
# of binutils to 2.37, llvm to 13.0.0.
#
# Then, the following line will be really simple:
# done < <(${NM} --quiet ${1})
done < <(${NM} ${1} 2>/dev/null || { echo "${0}: ${NM} failed" >&2; false; } )
# Catch error in the process substitution
wait $!
for name in "${export_symbols[@]}"
do
# nm(3) says "If lowercase, the symbol is usually local"
if [[ ${symbol_types[$name]} =~ [a-z] ]]; then
echo "$@: error: local symbol '${name}' was exported" >&2
exit_code=1
fi
done
exit ${exit_code}
......@@ -212,7 +212,6 @@ struct symbol {
unsigned int crc;
bool crc_valid;
bool weak;
bool is_static; /* true if symbol is not global */
bool is_gpl_only; /* exported by EXPORT_SYMBOL_GPL */
char name[];
};
......@@ -242,7 +241,7 @@ static struct symbol *alloc_symbol(const char *name)
memset(s, 0, sizeof(*s));
strcpy(s->name, name);
s->is_static = true;
return s;
}
......@@ -2064,20 +2063,6 @@ static void read_symbols(const char *modname)
sym_get_data(&info, sym));
}
// check for static EXPORT_SYMBOL_* functions && global vars
for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
unsigned char bind = ELF_ST_BIND(sym->st_info);
if (bind == STB_GLOBAL || bind == STB_WEAK) {
struct symbol *s =
find_symbol(remove_dot(info.strtab +
sym->st_name));
if (s)
s->is_static = false;
}
}
check_sec_ref(modname, &info);
if (!mod->is_vmlinux) {
......@@ -2507,7 +2492,6 @@ static void read_dump(const char *fname)
mod->from_dump = true;
}
s = sym_add_exported(symname, mod, gpl_only);
s->is_static = false;
sym_set_crc(s, crc);
sym_update_namespace(symname, namespace);
}
......@@ -2572,7 +2556,6 @@ int main(int argc, char **argv)
char *missing_namespace_deps = NULL;
char *dump_write = NULL, *files_source = NULL;
int opt;
int n;
LIST_HEAD(dump_lists);
struct dump_list *dl, *dl2;
......@@ -2648,15 +2631,6 @@ int main(int argc, char **argv)
if (sec_mismatch_count && !sec_mismatch_warn_only)
error("Section mismatches detected.\n"
"Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.\n");
for (n = 0; n < SYMBOL_HASH_SIZE; n++) {
struct symbol *s;
for (s = symbolhash[n]; s; s = s->next) {
if (s->is_static)
error("\"%s\" [%s] is a static EXPORT_SYMBOL\n",
s->name, s->module->name);
}
}
if (nr_unresolved > MAX_UNRESOLVED_REPORTS)
warn("suppressed %u unresolved symbol warnings because there were too many)\n",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment