lkml.org 
[lkml]   [2019]   [Sep]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] modpost: Copy namespace string into 'struct symbol'
On Thu, Sep 26, 2019 at 05:24:46PM -0500, Shaun Ruffell wrote:
>When building an out-of-tree module I was receiving many warnings from
>modpost like:
>
> WARNING: module dahdi_vpmadt032_loader uses symbol __kmalloc from namespace ts/dahdi-linux/drivers/dahdi/dahdi-version.o: ..., but does not import it.
> WARNING: module dahdi_vpmadt032_loader uses symbol vpmadtreg_register from namespace linux/drivers/dahdi/dahdi-version.o: ..., but does not import it.
> WARNING: module dahdi_vpmadt032_loader uses symbol param_ops_int from namespace ahdi-linux/drivers/dahdi/dahdi-version.o: ..., but does not import it.
> WARNING: module dahdi_vpmadt032_loader uses symbol __init_waitqueue_head from namespace ux/drivers/dahdi/dahdi-version.o: ..., but does not import it.
> ...
>
>The fundamental issue appears to be that read_dump() is passing a
>pointer to a statically allocated buffer for the namespace which is
>reused as the file is parsed.

Hi Shaun,

Thanks for working on this. I think you are right about the root cause
of this. I will have a closer look at your fix later today.

>This change makes it so that 'struct symbol' holds a copy of the
>namespace string in the same way that it holds a copy of the symbol
>string. Because a copy is being made, handle_modversion can now free the
>temporary copy
>
>Fixes: cb9b55d21fe0 ("modpost: add support for symbol namespaces")
>Cc: Martijn Coenen <maco@android.com>
>Cc: Joel Fernandes (Google) <joel@joelfernandes.org>
>Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>Cc: Matthias Maennich <maennich@google.com>
>Cc: Jessica Yu <jeyu@kernel.org>
>Signed-off-by: Shaun Ruffell <sruffell@sruffell.net>
>---
>
>Hi,
>
>I didn't test that this change works with the namespaces, or investigate why
>read_dump() is only called first while building out-of-tree modules, but it does
>seem correct to me for the symbol to own the memory backing the namespace
>string.
>
>I also realize I'm jumping the gun a bit by testing against master before
>5.4-rc1 is tagged.
>
>Shaun
>
> scripts/mod/modpost.c | 31 +++++++++++++++++++++++++++++--
> 1 file changed, 29 insertions(+), 2 deletions(-)
>
>diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
>index 3961941e8e7a..349832ead200 100644
>--- a/scripts/mod/modpost.c
>+++ b/scripts/mod/modpost.c
>@@ -364,6 +364,24 @@ static const char *sym_extract_namespace(const char **symname)
> return NULL;
> }
>
>+static const char *dup_namespace(const char *namespace)
>+{
>+ if (!namespace || (namespace[0] == '\0'))
>+ return NULL;
>+ return NOFAIL(strdup(namespace));
>+}
>+
>+static bool is_equal(const char *n1, const char *n2)
>+{
>+ if (n1 && !n2)
>+ return false;
>+ if (!n1 && n2)
>+ return false;
>+ if (!n1 && !n2)
>+ return true;
>+ return strcmp(n1, n2) == 0;
>+}
>+
> /**
> * Add an exported symbol - it may have already been added without a
> * CRC, in this case just update the CRC
>@@ -375,7 +393,7 @@ static struct symbol *sym_add_exported(const char *name, const char *namespace,
>
> if (!s) {
> s = new_symbol(name, mod, export);
>- s->namespace = namespace;
>+ s->namespace = dup_namespace(namespace);
> } else {
> if (!s->preloaded) {
> warn("%s: '%s' exported twice. Previous export was in %s%s\n",
>@@ -384,6 +402,12 @@ static struct symbol *sym_add_exported(const char *name, const char *namespace,
> } else {
> /* In case Module.symvers was out of date */
> s->module = mod;
>+
>+ /* In case the namespace was out of date */
>+ if (!is_equal(s->namespace, namespace)) {
>+ free((char *)s->namespace);
>+ s->namespace = dup_namespace(namespace);
>+ }
> }
> }
> s->preloaded = 0;
>@@ -672,7 +696,6 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
> unsigned int crc;
> enum export export;
> bool is_crc = false;
>- const char *name, *namespace;
>
> if ((!is_vmlinux(mod->name) || mod->is_dot_o) &&
> strstarts(symname, "__ksymtab"))
>@@ -744,9 +767,13 @@ static void handle_modversions(struct module *mod, struct elf_info *info,
> default:
> /* All exported symbols */
> if (strstarts(symname, "__ksymtab_")) {
>+ const char *name, *namespace;
>+
> name = symname + strlen("__ksymtab_");
> namespace = sym_extract_namespace(&name);
> sym_add_exported(name, namespace, mod, export);
>+ if (namespace)
>+ free((char *)name);

This probably should free namespace instead.

> }
> if (strcmp(symname, "init_module") == 0)
> mod->has_init = 1;
>--
>2.17.1

Cheers,
Matthias

\
 
 \ /
  Last update: 2019-09-27 10:04    [W:0.145 / U:3.956 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site