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;
From:   Shaun Ruffell <sruffell@sruffell.net>
To:     linux-kernel@vger.kernel.org
Cc:     Shaun Ruffell <sruffell@sruffell.net>,
        Martijn Coenen <maco@android.com>,
        Joel Fernandes <joel@joelfernandes.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Matthias Maennich <maennich@google.com>,
        Jessica Yu <jeyu@kernel.org>
Subject: [PATCH] modpost: Copy namespace string into 'struct symbol'
Date:   Thu, 26 Sep 2019 17:24:46 -0500
Message-Id: <20190926222446.30510-1-sruffell@sruffell.net>
In-Reply-To: <20190906103235.197072-5-maennich@google.com>
References: <20190906103235.197072-5-maennich@google.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.

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);
 		}
 		if (strcmp(symname, "init_module") == 0)
 			mod->has_init = 1;
-- 
2.17.1