[PATCH v2 0/3] module: trivial cleanups for symbol search

[PATCH v2 0/3] module: trivial cleanups for symbol search

[Masahiro Yamada]

V2: rebase on module-next.


Masahiro Yamada (3):
  module: do not pass opaque pointer for symbol search
  module: do not binary-search in __ksymtab_gpl if fsa->gplok is false
  module: merge check_exported_symbol() into
    find_exported_symbol_in_section()

 kernel/module/main.c | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)

-- 
2.32.0

[PATCH v2 1/3] module: do not pass opaque pointer for symbol search

[Masahiro Yamada]

There is no need to use an opaque pointer for check_exported_symbol()
or find_exported_symbol_in_section.

Pass (struct find_symbol_arg *) explicitly.

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

 kernel/module/main.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index fe44d46c378b..28ece86d99dc 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -244,11 +244,9 @@ static __maybe_unused void *any_section_objs(const struct load_info *info,
 #endif
 
 static bool check_exported_symbol(const struct symsearch *syms,
-				  struct module *owner,
-				  unsigned int symnum, void *data)
+				  struct module *owner, unsigned int symnum,
+				  struct find_symbol_arg *fsa)
 {
-	struct find_symbol_arg *fsa = data;
-
 	if (!fsa->gplok && syms->license == GPL_ONLY)
 		return false;
 	fsa->owner = owner;
@@ -285,16 +283,15 @@ int cmp_name(const void *name, const void *sym)
 
 static bool find_exported_symbol_in_section(const struct symsearch *syms,
 					    struct module *owner,
-					    void *data)
+					    struct find_symbol_arg *fsa)
 {
-	struct find_symbol_arg *fsa = data;
 	struct kernel_symbol *sym;
 
 	sym = bsearch(fsa->name, syms->start, syms->stop - syms->start,
 			sizeof(struct kernel_symbol), cmp_name);
 
 	if (sym != NULL && check_exported_symbol(syms, owner,
-						 sym - syms->start, data))
+						 sym - syms->start, fsa))
 		return true;
 
 	return false;
-- 
2.32.0

[PATCH v2 2/3] module: do not binary-search in __ksymtab_gpl if fsa->gplok is false

[Masahiro Yamada]

Currently, !fsa->gplok && syms->license == GPL_ONLY) is checked after
bsearch() succeeds.

It is meaningless to do the binary search in the GPL symbol table when
fsa->gplok is false because we know find_exported_symbol_in_section()
will fail anyway.

This check should be done before bsearch().

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

 kernel/module/main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 28ece86d99dc..22a860d42c16 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -247,8 +247,6 @@ static bool check_exported_symbol(const struct symsearch *syms,
 				  struct module *owner, unsigned int symnum,
 				  struct find_symbol_arg *fsa)
 {
-	if (!fsa->gplok && syms->license == GPL_ONLY)
-		return false;
 	fsa->owner = owner;
 	fsa->crc = symversion(syms->crcs, symnum);
 	fsa->sym = &syms->start[symnum];
@@ -287,6 +285,9 @@ static bool find_exported_symbol_in_section(const struct symsearch *syms,
 {
 	struct kernel_symbol *sym;
 
+	if (!fsa->gplok && syms->license == GPL_ONLY)
+		return false;
+
 	sym = bsearch(fsa->name, syms->start, syms->stop - syms->start,
 			sizeof(struct kernel_symbol), cmp_name);
 
-- 
2.32.0

[PATCH v2 3/3] module: merge check_exported_symbol() into find_exported_symbol_in_section()

[Masahiro Yamada]

Now check_exported_symbol() always succeeds.

Merge it into find_exported_symbol_in_search() to make the code concise.

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

 kernel/module/main.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 22a860d42c16..14686571d4fc 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -243,17 +243,6 @@ static __maybe_unused void *any_section_objs(const struct load_info *info,
 #define symversion(base, idx) ((base != NULL) ? ((base) + (idx)) : NULL)
 #endif
 
-static bool check_exported_symbol(const struct symsearch *syms,
-				  struct module *owner, unsigned int symnum,
-				  struct find_symbol_arg *fsa)
-{
-	fsa->owner = owner;
-	fsa->crc = symversion(syms->crcs, symnum);
-	fsa->sym = &syms->start[symnum];
-	fsa->license = syms->license;
-	return true;
-}
-
 static const char *kernel_symbol_name(const struct kernel_symbol *sym)
 {
 #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
@@ -290,12 +279,15 @@ static bool find_exported_symbol_in_section(const struct symsearch *syms,
 
 	sym = bsearch(fsa->name, syms->start, syms->stop - syms->start,
 			sizeof(struct kernel_symbol), cmp_name);
+	if (!sym)
+		return false;
 
-	if (sym != NULL && check_exported_symbol(syms, owner,
-						 sym - syms->start, fsa))
-		return true;
+	fsa->owner = owner;
+	fsa->crc = symversion(syms->crcs, sym - syms->start);
+	fsa->sym = sym;
+	fsa->license = syms->license;
 
-	return false;
+	return true;
 }
 
 /*
-- 
2.32.0

Re: [PATCH v2 0/3] module: trivial cleanups for symbol search

[Luis Chamberlain]

On Thu, May 05, 2022 at 12:52:09PM +0900, Masahiro Yamada wrote:
> V2: rebase on module-next.

Thanks! Pushed onto modules-testing, if there are no issues reported
I'll move to modules-next.

  Luis