| Message ID | 20190801060657.5932-1-efremov@linux.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5] modpost: check for static EXPORT_SYMBOL* functions | expand |
On Thu, Aug 1, 2019 at 3:07 PM Denis Efremov <efremov@linux.com> wrote: > > This patch adds a check to warn about static EXPORT_SYMBOL* functions > during the modpost. In most of the cases, a static symbol marked for > exporting is an odd combination that should be fixed either by deleting > the exporting mark or by removing the static attribute and adding the > appropriate declaration to headers. > > This check could help to detect the following problems: > 1. 550113d4e9f5 ("i2c: add newly exported functions to the header, too") > 2. 54638c6eaf44 ("net: phy: make exported variables non-static") > 3. 98ef2046f28b ("mm: remove the exporting of totalram_pages") > 4. 73df167c819e ("s390/zcrypt: remove the exporting of ap_query_configuration") > 5. a57caf8c527f ("sunrpc/cache: remove the exporting of cache_seq_next") > 6. e4e4730698c9 ("crypto: skcipher - remove the exporting of skcipher_walk_next") > 7. 14b4c48bb1ce ("gve: Remove the exporting of gve_probe") > 8. 9b79ee9773a8 ("scsi: libsas: remove the exporting of sas_wait_eh") > 9. ... > > The build time impact is very limited and is almost at the unnoticeable > level (< 1 sec). > > Acked-by: Emil Velikov <emil.l.velikov@gmail.com> > Signed-off-by: Denis Efremov <efremov@linux.com> > --- Applied to linux-kbuild. Thanks.
Hi Denis, On Thu, Aug 8, 2019 at 12:12 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > On Thu, Aug 1, 2019 at 3:07 PM Denis Efremov <efremov@linux.com> wrote: > > > > This patch adds a check to warn about static EXPORT_SYMBOL* functions > > during the modpost. In most of the cases, a static symbol marked for > > exporting is an odd combination that should be fixed either by deleting > > the exporting mark or by removing the static attribute and adding the > > appropriate declaration to headers. > > > > This check could help to detect the following problems: > > 1. 550113d4e9f5 ("i2c: add newly exported functions to the header, too") > > 2. 54638c6eaf44 ("net: phy: make exported variables non-static") > > 3. 98ef2046f28b ("mm: remove the exporting of totalram_pages") > > 4. 73df167c819e ("s390/zcrypt: remove the exporting of ap_query_configuration") > > 5. a57caf8c527f ("sunrpc/cache: remove the exporting of cache_seq_next") > > 6. e4e4730698c9 ("crypto: skcipher - remove the exporting of skcipher_walk_next") > > 7. 14b4c48bb1ce ("gve: Remove the exporting of gve_probe") > > 8. 9b79ee9773a8 ("scsi: libsas: remove the exporting of sas_wait_eh") > > 9. ... > > > > The build time impact is very limited and is almost at the unnoticeable > > level (< 1 sec). > > > > Acked-by: Emil Velikov <emil.l.velikov@gmail.com> > > Signed-off-by: Denis Efremov <efremov@linux.com> > > --- > > Applied to linux-kbuild. Thanks. I noticed this patch produces false-positive warnings for external module build. When I built my external module, it showed the following, the last five are false positives. make[1]: Entering directory '/home/masahiro/workspace/linux-kbuild' Building modules, stage 2. MODPOST 2 modules WARNING: "drm_client_close" [vmlinux] is a static EXPORT_SYMBOL WARNING: "ahci_em_messages" [vmlinux] is a static EXPORT_SYMBOL_GPL WARNING: "ftrace_set_clr_event" [vmlinux] is a static EXPORT_SYMBOL_GPL WARNING: "nf_log_dump_packet_common" [net/netfilter/nf_log_common] is a static EXPORT_SYMBOL_GPL WARNING: "nf_log_l2packet" [net/netfilter/nf_log_common] is a static EXPORT_SYMBOL_GPL WARNING: "nf_log_dump_sk_uid_gid" [net/netfilter/nf_log_common] is a static EXPORT_SYMBOL_GPL WARNING: "nf_log_dump_udp_header" [net/netfilter/nf_log_common] is a static EXPORT_SYMBOL_GPL WARNING: "nf_log_dump_tcp_header" [net/netfilter/nf_log_common] is a static EXPORT_SYMBOL_GPL make[1]: Leaving directory '/home/masahiro/workspace/linux-kbuild' I squashed the following fix-up. diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 3e6d36ddfcdf..2773f9f9bae2 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -2386,6 +2386,7 @@ static void read_dump(const char *fname, unsigned int kernel) s = sym_add_exported(symname, mod, export_no(export)); s->kernel = kernel; s->preloaded = 1; + s->is_static = 0; sym_update_crc(symname, mod, crc, export_no(export)); } release_file(file, size);
On 13.08.2019 19:07, Masahiro Yamada wrote: > Hi Denis, > > I squashed the following fix-up. > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 3e6d36ddfcdf..2773f9f9bae2 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -2386,6 +2386,7 @@ static void read_dump(const char *fname, > unsigned int kernel) > s = sym_add_exported(symname, mod, export_no(export)); > s->kernel = kernel; > s->preloaded = 1; > + s->is_static = 0; > sym_update_crc(symname, mod, crc, export_no(export)); > } > release_file(file, size); Hi! Thank you very much indeed. Best regards, Denis
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index f277e116e0eb..3e6d36ddfcdf 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -169,6 +169,7 @@ struct symbol { unsigned int kernel:1; /* 1 if symbol is from kernel * (only for external modules) **/ unsigned int preloaded:1; /* 1 if symbol from Module.symvers, or crc */ + unsigned int is_static:1; /* 1 if symbol is not global */ enum export export; /* Type of export */ char name[0]; }; @@ -201,6 +202,7 @@ static struct symbol *alloc_symbol(const char *name, unsigned int weak, strcpy(s->name, name); s->weak = weak; s->next = next; + s->is_static = 1; return s; } @@ -1980,6 +1982,21 @@ static void read_symbols(const char *modname) handle_modversions(mod, &info, sym, symname); handle_moddevtable(mod, &info, sym, symname); } + + // 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 = 0; + } + } + if (!is_vmlinux(modname) || vmlinux_section_warnings) check_sec_ref(mod, modname, &info); @@ -2425,6 +2442,7 @@ int main(int argc, char **argv) char *dump_write = NULL, *files_source = NULL; int opt; int err; + int n; struct ext_sym_list *extsym_iter; struct ext_sym_list *extsym_start = NULL; @@ -2520,6 +2538,19 @@ int main(int argc, char **argv) if (sec_mismatch_count && sec_mismatch_fatal) fatal("modpost: 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 = symbolhash[n]; + + while (s) { + if (s->is_static) + warn("\"%s\" [%s] is a static %s\n", + s->name, s->module->name, + export_str(s->export)); + + s = s->next; + } + } + free(buf.p); return err;
This patch adds a check to warn about static EXPORT_SYMBOL* functions during the modpost. In most of the cases, a static symbol marked for exporting is an odd combination that should be fixed either by deleting the exporting mark or by removing the static attribute and adding the appropriate declaration to headers. This check could help to detect the following problems: 1. 550113d4e9f5 ("i2c: add newly exported functions to the header, too") 2. 54638c6eaf44 ("net: phy: make exported variables non-static") 3. 98ef2046f28b ("mm: remove the exporting of totalram_pages") 4. 73df167c819e ("s390/zcrypt: remove the exporting of ap_query_configuration") 5. a57caf8c527f ("sunrpc/cache: remove the exporting of cache_seq_next") 6. e4e4730698c9 ("crypto: skcipher - remove the exporting of skcipher_walk_next") 7. 14b4c48bb1ce ("gve: Remove the exporting of gve_probe") 8. 9b79ee9773a8 ("scsi: libsas: remove the exporting of sas_wait_eh") 9. ... The build time impact is very limited and is almost at the unnoticeable level (< 1 sec). Acked-by: Emil Velikov <emil.l.velikov@gmail.com> Signed-off-by: Denis Efremov <efremov@linux.com> --- scripts/mod/modpost.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)