Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200226142608.19499-1-jeyu@kernel.org> <20200226142608.19499-2-jeyu@kernel.org>
In-Reply-To: <20200226142608.19499-2-jeyu@kernel.org>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Wed, 4 Mar 2020 00:21:02 +0900
Message-ID: <CAK7LNATD7MHY7UDAZW7VwgC27bfPZ=TGEUY7LpVzxRyuJA6jOQ@mail.gmail.com>
Subject: Re: [PATCH v2 2/2] modpost: return error if module is missing ns
 imports and MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS=n
To:     Jessica Yu <jeyu@kernel.org>
Cc:     Matthias Maennich <maennich@google.com>,
        Joe Perches <joe@perches.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Feb 26, 2020 at 11:26 PM Jessica Yu <jeyu@kernel.org> 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.
>
> Signed-off-by: Jessica Yu <jeyu@kernel.org>
> ---


When you send v3, could you please fix the
following checkpatch warning and error?


WARNING: Possible unwrapped commit description (prefer a maximum 75
chars per line)
#59:
Currently when CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS=n,
modpost only warns

ERROR: do not initialise statics to 0
#107: FILE: scripts/mod/modpost.c:43:
+static int allow_missing_ns_imports = 0;


>  scripts/Makefile.modpost | 15 ++++++++-------
>  scripts/mod/modpost.c    | 14 +++++++++++---
>  2 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index b4d3f2d122ac..957eed6a17a5 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -46,13 +46,14 @@ include scripts/Kbuild.include
>  kernelsymfile := $(objtree)/Module.symvers
>  modulesymfile := $(firstword $(KBUILD_EXTMOD))/Module.symvers
>
> -MODPOST = scripts/mod/modpost                                          \
> -       $(if $(CONFIG_MODVERSIONS),-m)                                  \
> -       $(if $(CONFIG_MODULE_SRCVERSION_ALL),-a)                        \
> -       $(if $(KBUILD_EXTMOD),-i,-o) $(kernelsymfile)                   \
> -       $(if $(KBUILD_EXTMOD),$(addprefix -e ,$(KBUILD_EXTRA_SYMBOLS))) \
> -       $(if $(KBUILD_EXTMOD),-o $(modulesymfile))                      \
> -       $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E)                  \
> +MODPOST = scripts/mod/modpost                                                          \
> +       $(if $(CONFIG_MODVERSIONS),-m)                                                  \
> +       $(if $(CONFIG_MODULE_SRCVERSION_ALL),-a)                                        \
> +       $(if $(KBUILD_EXTMOD),-i,-o) $(kernelsymfile)                                   \
> +       $(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)$(KBUILD_NSDEPS),-N)       \
>         $(if $(KBUILD_MODPOST_WARN),-w)
>
>  ifdef MODPOST_VMLINUX
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 3201a2ac5cc4..d164be63c2e3 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;
> +/* If set to 1, only warn (instead of error) about missing ns imports */
> +static int allow_missing_ns_imports = 0;
>
>  enum export {
>         export_plain,      export_unused,     export_gpl,
> @@ -2209,8 +2211,11 @@ 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);
> +                       warn_unless(!allow_missing_ns_imports,
> +                                   "module %s uses symbol %s from namespace %s, but does not import it.\n",
> +                                   basename, exp->name, exp->namespace);
> +                       if (!allow_missing_ns_imports)
> +                               err = 1;
>                         add_namespace(&mod->missing_namespaces, exp->namespace);
>                 }
>
> @@ -2553,7 +2558,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;
> @@ -2591,6 +2596,9 @@ int main(int argc, char **argv)
>                 case 'E':
>                         sec_mismatch_fatal = 1;
>                         break;
> +               case 'N':
> +                       allow_missing_ns_imports = 1;
> +                       break;
>                 case 'd':
>                         missing_namespace_deps = optarg;
>                         break;
> --
> 2.16.4
>


-- 
Best Regards
Masahiro Yamada