All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masahiro Yamada <masahiroy@kernel.org>
To: linux-kbuild@vger.kernel.org
Cc: clang-built-linux@googlegroups.com, linux-kernel@vger.kernel.org,
	Nicolas Schier a <nicolas@fjasle.eu>,
	Ard Biesheuvel <ardb@kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	linuxppc-dev@lists.ozlabs.org, linux-um@lists.infradead.org,
	linux-s390@vger.kernel.org,
	Nick Desaulniers <ndesaulniers@google.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	Kees Cook <keescook@chromium.org>,
	Masahiro Yamada <masahiroy@kernel.org>
Subject: [PATCH v3 01/15] modpost: mitigate false-negatives for static EXPORT_SYMBOL checks
Date: Thu,  5 May 2022 16:22:30 +0900	[thread overview]
Message-ID: <20220505072244.1155033-2-masahiroy@kernel.org> (raw)
In-Reply-To: <20220505072244.1155033-1-masahiroy@kernel.org>

The 'static' specifier and EXPORT_SYMBOL() are an odd combination.

Since commit 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL*
functions"), modpost tries to detect it, but there are false negatives.

Here is the sample code.

[Sample 1]

  Makefile:

    obj-m += mymod1.o mymod2.o

  mymod1.c:

    #include <linux/export.h>
    #include <linux/module.h>
    static void foo(void) {}
    EXPORT_SYMBOL(foo);
    MODULE_LICENSE("GPL");

  mymod2.c:

    #include <linux/module.h>
    void foo(void) {}
    MODULE_LICENSE("GPL");

mymod1 exports the static symbol 'foo', but modpost cannot catch it
because it is fooled by the same name symbol in another module, mymod2.
(Without mymod2, modpost can detect the error in mymod1)

find_symbol() returns the first symbol found in the hash table with the
given name. This hash table is global, so it may return a symbol from
an unrelated module. So, a global symbol in a module may unset the
'is_static' flag of another module.

To mitigate this issue, add sym_find_with_module(), which receives the
module pointer as the second argument. If non-NULL pointer is passed, it
returns the symbol in the specified module. If NULL is passed, it is
equivalent to find_module().

Please note there are still false positives in the composite module,
like below (or when both are built-in). I have no idea how to do this
correctly.

[Sample 2]  (not fixed by this commit)

  Makefile:
    obj-m += mymod.o
    mymod-objs := mymod1.o mymod2.o

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v2)

Changes in v2:
  - Rename the new func to sym_find_with_module()

 scripts/mod/modpost.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index b605f4a58759..a55fa2b88a9a 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -272,7 +272,7 @@ static void sym_add_unresolved(const char *name, struct module *mod, bool weak)
 	list_add_tail(&sym->list, &mod->unresolved_symbols);
 }
 
-static struct symbol *find_symbol(const char *name)
+static struct symbol *sym_find_with_module(const char *name, struct module *mod)
 {
 	struct symbol *s;
 
@@ -281,12 +281,17 @@ static struct symbol *find_symbol(const char *name)
 		name++;
 
 	for (s = symbolhash[tdb_hash(name) % SYMBOL_HASH_SIZE]; s; s = s->next) {
-		if (strcmp(s->name, name) == 0)
+		if (strcmp(s->name, name) == 0 && (!mod || s->module == mod))
 			return s;
 	}
 	return NULL;
 }
 
+static struct symbol *find_symbol(const char *name)
+{
+	return sym_find_with_module(name, NULL);
+}
+
 struct namespace_list {
 	struct list_head list;
 	char namespace[];
@@ -2063,8 +2068,9 @@ static void read_symbols(const char *modname)
 
 		if (bind == STB_GLOBAL || bind == STB_WEAK) {
 			struct symbol *s =
-				find_symbol(remove_dot(info.strtab +
-						       sym->st_name));
+				sym_find_with_module(remove_dot(info.strtab +
+								sym->st_name),
+						     mod);
 
 			if (s)
 				s->is_static = false;
-- 
2.32.0


WARNING: multiple messages have this Message-ID (diff)
From: Masahiro Yamada <masahiroy@kernel.org>
To: linux-kbuild@vger.kernel.org
Cc: linux-s390@vger.kernel.org, Nicolas Schier a <nicolas@fjasle.eu>,
	Peter Zijlstra <peterz@infradead.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	linux-um@lists.infradead.org, linux-kernel@vger.kernel.org,
	clang-built-linux@googlegroups.com,
	Luis Chamberlain <mcgrof@kernel.org>,
	Sami Tolvanen <samitolvanen@google.com>,
	linuxppc-dev@lists.ozlabs.org, Ard Biesheuvel <ardb@kernel.org>,
	Kees Cook <keescook@chromium.org>
Subject: [PATCH v3 01/15] modpost: mitigate false-negatives for static EXPORT_SYMBOL checks
Date: Thu,  5 May 2022 16:22:30 +0900	[thread overview]
Message-ID: <20220505072244.1155033-2-masahiroy@kernel.org> (raw)
In-Reply-To: <20220505072244.1155033-1-masahiroy@kernel.org>

The 'static' specifier and EXPORT_SYMBOL() are an odd combination.

Since commit 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL*
functions"), modpost tries to detect it, but there are false negatives.

Here is the sample code.

[Sample 1]

  Makefile:

    obj-m += mymod1.o mymod2.o

  mymod1.c:

    #include <linux/export.h>
    #include <linux/module.h>
    static void foo(void) {}
    EXPORT_SYMBOL(foo);
    MODULE_LICENSE("GPL");

  mymod2.c:

    #include <linux/module.h>
    void foo(void) {}
    MODULE_LICENSE("GPL");

mymod1 exports the static symbol 'foo', but modpost cannot catch it
because it is fooled by the same name symbol in another module, mymod2.
(Without mymod2, modpost can detect the error in mymod1)

find_symbol() returns the first symbol found in the hash table with the
given name. This hash table is global, so it may return a symbol from
an unrelated module. So, a global symbol in a module may unset the
'is_static' flag of another module.

To mitigate this issue, add sym_find_with_module(), which receives the
module pointer as the second argument. If non-NULL pointer is passed, it
returns the symbol in the specified module. If NULL is passed, it is
equivalent to find_module().

Please note there are still false positives in the composite module,
like below (or when both are built-in). I have no idea how to do this
correctly.

[Sample 2]  (not fixed by this commit)

  Makefile:
    obj-m += mymod.o
    mymod-objs := mymod1.o mymod2.o

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v2)

Changes in v2:
  - Rename the new func to sym_find_with_module()

 scripts/mod/modpost.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index b605f4a58759..a55fa2b88a9a 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -272,7 +272,7 @@ static void sym_add_unresolved(const char *name, struct module *mod, bool weak)
 	list_add_tail(&sym->list, &mod->unresolved_symbols);
 }
 
-static struct symbol *find_symbol(const char *name)
+static struct symbol *sym_find_with_module(const char *name, struct module *mod)
 {
 	struct symbol *s;
 
@@ -281,12 +281,17 @@ static struct symbol *find_symbol(const char *name)
 		name++;
 
 	for (s = symbolhash[tdb_hash(name) % SYMBOL_HASH_SIZE]; s; s = s->next) {
-		if (strcmp(s->name, name) == 0)
+		if (strcmp(s->name, name) == 0 && (!mod || s->module == mod))
 			return s;
 	}
 	return NULL;
 }
 
+static struct symbol *find_symbol(const char *name)
+{
+	return sym_find_with_module(name, NULL);
+}
+
 struct namespace_list {
 	struct list_head list;
 	char namespace[];
@@ -2063,8 +2068,9 @@ static void read_symbols(const char *modname)
 
 		if (bind == STB_GLOBAL || bind == STB_WEAK) {
 			struct symbol *s =
-				find_symbol(remove_dot(info.strtab +
-						       sym->st_name));
+				sym_find_with_module(remove_dot(info.strtab +
+								sym->st_name),
+						     mod);
 
 			if (s)
 				s->is_static = false;
-- 
2.32.0


WARNING: multiple messages have this Message-ID (diff)
From: Masahiro Yamada <masahiroy@kernel.org>
To: linux-kbuild@vger.kernel.org
Cc: clang-built-linux@googlegroups.com, linux-kernel@vger.kernel.org,
	Nicolas Schier a <nicolas@fjasle.eu>,
	Ard Biesheuvel <ardb@kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	linuxppc-dev@lists.ozlabs.org, linux-um@lists.infradead.org,
	linux-s390@vger.kernel.org,
	Nick Desaulniers <ndesaulniers@google.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	Kees Cook <keescook@chromium.org>,
	Masahiro Yamada <masahiroy@kernel.org>
Subject: [PATCH v3 01/15] modpost: mitigate false-negatives for static EXPORT_SYMBOL checks
Date: Thu,  5 May 2022 16:22:30 +0900	[thread overview]
Message-ID: <20220505072244.1155033-2-masahiroy@kernel.org> (raw)
In-Reply-To: <20220505072244.1155033-1-masahiroy@kernel.org>

The 'static' specifier and EXPORT_SYMBOL() are an odd combination.

Since commit 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL*
functions"), modpost tries to detect it, but there are false negatives.

Here is the sample code.

[Sample 1]

  Makefile:

    obj-m += mymod1.o mymod2.o

  mymod1.c:

    #include <linux/export.h>
    #include <linux/module.h>
    static void foo(void) {}
    EXPORT_SYMBOL(foo);
    MODULE_LICENSE("GPL");

  mymod2.c:

    #include <linux/module.h>
    void foo(void) {}
    MODULE_LICENSE("GPL");

mymod1 exports the static symbol 'foo', but modpost cannot catch it
because it is fooled by the same name symbol in another module, mymod2.
(Without mymod2, modpost can detect the error in mymod1)

find_symbol() returns the first symbol found in the hash table with the
given name. This hash table is global, so it may return a symbol from
an unrelated module. So, a global symbol in a module may unset the
'is_static' flag of another module.

To mitigate this issue, add sym_find_with_module(), which receives the
module pointer as the second argument. If non-NULL pointer is passed, it
returns the symbol in the specified module. If NULL is passed, it is
equivalent to find_module().

Please note there are still false positives in the composite module,
like below (or when both are built-in). I have no idea how to do this
correctly.

[Sample 2]  (not fixed by this commit)

  Makefile:
    obj-m += mymod.o
    mymod-objs := mymod1.o mymod2.o

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v2)

Changes in v2:
  - Rename the new func to sym_find_with_module()

 scripts/mod/modpost.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index b605f4a58759..a55fa2b88a9a 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -272,7 +272,7 @@ static void sym_add_unresolved(const char *name, struct module *mod, bool weak)
 	list_add_tail(&sym->list, &mod->unresolved_symbols);
 }
 
-static struct symbol *find_symbol(const char *name)
+static struct symbol *sym_find_with_module(const char *name, struct module *mod)
 {
 	struct symbol *s;
 
@@ -281,12 +281,17 @@ static struct symbol *find_symbol(const char *name)
 		name++;
 
 	for (s = symbolhash[tdb_hash(name) % SYMBOL_HASH_SIZE]; s; s = s->next) {
-		if (strcmp(s->name, name) == 0)
+		if (strcmp(s->name, name) == 0 && (!mod || s->module == mod))
 			return s;
 	}
 	return NULL;
 }
 
+static struct symbol *find_symbol(const char *name)
+{
+	return sym_find_with_module(name, NULL);
+}
+
 struct namespace_list {
 	struct list_head list;
 	char namespace[];
@@ -2063,8 +2068,9 @@ static void read_symbols(const char *modname)
 
 		if (bind == STB_GLOBAL || bind == STB_WEAK) {
 			struct symbol *s =
-				find_symbol(remove_dot(info.strtab +
-						       sym->st_name));
+				sym_find_with_module(remove_dot(info.strtab +
+								sym->st_name),
+						     mod);
 
 			if (s)
 				s->is_static = false;
-- 
2.32.0


_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um


  reply	other threads:[~2022-05-05  7:26 UTC|newest]

Thread overview: 93+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-05  7:22 [PATCH v3 00/15] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS) Masahiro Yamada
2022-05-05  7:22 ` Masahiro Yamada
2022-05-05  7:22 ` Masahiro Yamada
2022-05-05  7:22 ` Masahiro Yamada [this message]
2022-05-05  7:22   ` [PATCH v3 01/15] modpost: mitigate false-negatives for static EXPORT_SYMBOL checks Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05 19:25   ` Nicolas Schier
2022-05-05 19:25     ` Nicolas Schier
2022-05-05 19:25     ` Nicolas Schier
2022-05-05  7:22 ` [PATCH v3 02/15] modpost: change the license of EXPORT_SYMBOL to bool type Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05 13:48   ` Masahiro Yamada
2022-05-05 13:48     ` Masahiro Yamada
2022-05-05 13:48     ` Masahiro Yamada
2022-05-05 19:53     ` Nicolas Schier
2022-05-05 19:53       ` Nicolas Schier
2022-05-05 19:53       ` Nicolas Schier
2022-05-05  7:22 ` [PATCH v3 03/15] modpost: merge add_{intree_flag,retpoline,staging_flag} to add_header Masahiro Yamada
2022-05-05  7:22   ` [PATCH v3 03/15] modpost: merge add_{intree_flag, retpoline, staging_flag} " Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05 19:58   ` [PATCH v3 03/15] modpost: merge add_{intree_flag,retpoline,staging_flag} " Nicolas Schier
2022-05-05 19:58     ` Nicolas Schier
2022-05-05 19:58     ` Nicolas Schier
2022-05-05  7:22 ` [PATCH v3 04/15] modpost: move *.mod.c generation to write_mod_c_files() Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05 20:06   ` Nicolas Schier
2022-05-05 20:06     ` Nicolas Schier
2022-05-05 20:06     ` Nicolas Schier
2022-05-05  7:22 ` [PATCH v3 05/15] kbuild: generate a list of objects in vmlinux Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22 ` [PATCH v3 06/15] kbuild: record symbol versions in *.cmd files Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22 ` [PATCH v3 07/15] modpost: extract symbol versions from " Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05 20:09   ` Nicolas Schier
2022-05-05 20:09     ` Nicolas Schier
2022-05-05 20:09     ` Nicolas Schier
2022-05-05  7:22 ` [PATCH v3 08/15] kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22 ` [PATCH v3 09/15] kbuild: stop merging *.symversions Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05 20:19   ` Nicolas Schier
2022-05-05 20:19     ` Nicolas Schier
2022-05-05 20:19     ` Nicolas Schier
2022-05-05  7:22 ` [PATCH v3 10/15] genksyms: adjust the output format to modpost Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05 20:22   ` Nicolas Schier
2022-05-05 20:22     ` Nicolas Schier
2022-05-05 20:22     ` Nicolas Schier
2022-05-05  7:22 ` [PATCH v3 11/15] kbuild: do not create *.prelink.o for Clang LTO or IBT Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22 ` [PATCH v3 12/15] modpost: simplify the ->is_static initialization Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05 20:27   ` Nicolas Schier
2022-05-05 20:27     ` Nicolas Schier
2022-05-05 20:27     ` Nicolas Schier
2022-05-05  7:22 ` [PATCH v3 13/15] modpost: use hlist for hash table implementation Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22 ` [PATCH v3 14/15] kbuild: make built-in.a rule robust against too long argument error Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05 20:29   ` Nicolas Schier
2022-05-05 20:29     ` Nicolas Schier
2022-05-05 20:29     ` Nicolas Schier
2022-05-05 20:31   ` Nick Desaulniers
2022-05-05 20:31     ` Nick Desaulniers
2022-05-05 20:31     ` Nick Desaulniers
2022-05-05  7:22 ` [PATCH v3 15/15] kbuild: make *.mod " Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05  7:22   ` Masahiro Yamada
2022-05-05 20:31   ` Nicolas Schier
2022-05-05 20:31     ` Nicolas Schier
2022-05-05 20:31     ` Nicolas Schier
2022-05-05 16:49 ` [PATCH v3 00/15] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS) Masahiro Yamada
2022-05-05 16:49   ` Masahiro Yamada
2022-05-05 16:49   ` Masahiro Yamada
2022-05-06 22:45 ` Nathan Chancellor
2022-05-06 22:45   ` Nathan Chancellor
2022-05-06 22:45   ` Nathan Chancellor
2022-05-08 18:28 ` Masahiro Yamada
2022-05-08 18:28   ` Masahiro Yamada
2022-05-08 18:28   ` Masahiro Yamada

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220505072244.1155033-2-masahiroy@kernel.org \
    --to=masahiroy@kernel.org \
    --cc=ardb@kernel.org \
    --cc=clang-built-linux@googlegroups.com \
    --cc=keescook@chromium.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux-um@lists.infradead.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mcgrof@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=nicolas@fjasle.eu \
    --cc=peterz@infradead.org \
    --cc=samitolvanen@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.