lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 17 Feb 2020 14:56:44 +0000 From: Matthias Maennich <maennich@...gle.com> To: Jessica Yu <jeyu@...nel.org> Cc: Masahiro Yamada <yamada.masahiro@...ionext.com>, linux-kernel@...r.kernel.org, Martijn Coenen <maco@...roid.com> Subject: Re: [PATCH] modpost: return error if module is missing ns imports and MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS=n Hi Jessica! On Fri, Feb 14, 2020 at 03:37:09PM +0100, Jessica Yu wrote: >Currently when CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS=n, modpost only warns >when a module is missing namespace imports. Under this configuration, such a module >cannot be loaded into the kernel anyway, as the module loader would reject it. >We might as well return a build error when a module is missing namespace imports >under CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS=n, so that the build >warning does not go ignored/unnoticed. I generally agree with the idea of the patch. Thanks for working on this! I also can't remember any reason why I did not write it like this initially. Probably just because I introduced this configuration option quite late in the development process of the initial patches. > >Signed-off-by: Jessica Yu <jeyu@...nel.org> >--- > scripts/Makefile.modpost | 1 + > scripts/mod/modpost.c | 19 +++++++++++++++---- > 2 files changed, 16 insertions(+), 4 deletions(-) > >diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost >index b4d3f2d122ac..a53660f910a9 100644 >--- a/scripts/Makefile.modpost >+++ b/scripts/Makefile.modpost >@@ -53,6 +53,7 @@ MODPOST = scripts/mod/modpost \ > $(if $(KBUILD_EXTMOD),$(addprefix -e ,$(KBUILD_EXTRA_SYMBOLS))) \ > $(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \ > $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E) \ >+ $(if $(CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS),,-N) \ > $(if $(KBUILD_MODPOST_WARN),-w) > > ifdef MODPOST_VMLINUX >diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c >index 7edfdb2f4497..53e966f7d557 100644 >--- a/scripts/mod/modpost.c >+++ b/scripts/mod/modpost.c >@@ -39,6 +39,8 @@ static int sec_mismatch_count = 0; > static int sec_mismatch_fatal = 0; > /* ignore missing files */ > static int ignore_missing_files; >+/* Return an error when there are missing namespace imports */ >+static int missing_ns_import_error = 0; A more suitable name is maybe missing_ns_import_is_error or follow the naming of the config option: allow_missing_ns_imports (with default = 1). > > enum export { > export_plain, export_unused, export_gpl, >@@ -2216,9 +2218,15 @@ static int check_exports(struct module *mod) > > if (exp->namespace && > !module_imports_namespace(mod, exp->namespace)) { >- warn("module %s uses symbol %s from namespace %s, but does not import it.\n", >- basename, exp->name, exp->namespace); >- add_namespace(&mod->missing_namespaces, exp->namespace); >+ if (missing_ns_import_error) { >+ merror("module %s uses symbol %s from namespace %s, but does not import it.\n", >+ basename, exp->name, exp->namespace); I would like to avoid the code duplication here. The string literal is identical for both cases. >+ err = 1; Also, if we fail here, we might as well help the user to fix it by suggesting to run `make nsdeps` (once per failed modpost run). Speaking of which, `make nsdeps` is currently broken by this patch as it relies on a successful (yet warning-full) build of the modules. So, in case of `make nsdeps`, we probably have to omit the -N flag again when invoking modpost. Cheers, Matthias >+ } else { >+ warn("module %s uses symbol %s from namespace %s, but does not import it.\n", >+ basename, exp->name, exp->namespace); >+ } >+ add_namespace(&mod->missing_namespaces, exp->namespace); > } > > if (!mod->gpl_compatible) >@@ -2560,7 +2568,7 @@ int main(int argc, char **argv) > struct ext_sym_list *extsym_iter; > struct ext_sym_list *extsym_start = NULL; > >- while ((opt = getopt(argc, argv, "i:e:mnsT:o:awEd:")) != -1) { >+ while ((opt = getopt(argc, argv, "i:e:mnsT:o:awENd:")) != -1) { > switch (opt) { > case 'i': > kernel_read = optarg; >@@ -2598,6 +2606,9 @@ int main(int argc, char **argv) > case 'E': > sec_mismatch_fatal = 1; > break; >+ case 'N': >+ missing_ns_import_error = 1; >+ break; > case 'd': > missing_namespace_deps = optarg; > break; >-- >2.16.4 >
Powered by blists - more mailing lists