Commit Graph

62 Commits

Author SHA1 Message Date
Masahiro Yamada
36b0f0deed modpost: refactor get_secindex()
SPECIAL() is only used in get_secindex(). Squash it.

Make the code more readable with more comments.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
2022-08-03 22:56:46 +09:00
Masahiro Yamada
7193cda917 modpost: remove unused Elf_Sword macro
Commit 9ad21c3f3e ("kbuild: try harder to find symbol names in
modpost") added Elf_Sword (in a wrong way), but did not use it at all.

BTW, the current code looks weird.

The fix for the 32-bit part would be:

    Elf64_Sword  -->  Elf32_Sword

(inconsistet prefix, Elf32_ vs Elf64_)

The fix for the 64-bit part would be:

    Elf64_Sxword  -->  Elf64_Sword

(the size is different between Sword and Sxword)

Note:

    Elf32_Sword   ==  Elf64_Sword   ==  int32_t
    Elf32_Sxword  ==  Elf64_Sxword  ==  int64_t

Anyway, let's drop unused code instead of fixing it.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
2022-08-03 19:37:59 +09:00
Masahiro Yamada
abe864b8e1 modpost: use sym_get_data() to get module device_table data
Use sym_get_data() to replace the long code.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
2022-07-27 21:18:00 +09:00
Masahiro Yamada
c5c468dcc2 modpost: reuse ARRAY_SIZE() macro for section_mismatch()
Move ARRAY_SIZE() from file2alias.c to modpost.h to reuse it in
section_mismatch().

Also, move the variable 'check' inside the for-loop.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
2022-05-27 16:15:40 +09:00
Masahiro Yamada
ce79c406a2 modpost: remove left-over cross_compile declaration
This is a remnant of commit 6543becf26 ("mod/file2alias: make
modalias generation safe for cross compiling").

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
2022-05-11 21:46:39 +09:00
Masahiro Yamada
f841536e8c modpost: dump Module.symvers in the same order of modules.order
modpost dumps the exported symbols into Module.symvers, but currently
in random order because it iterates in the hash table.

Add a linked list of exported symbols in struct module, so we can
iterate on symbols per module.

This commit makes Module.symvers much more readable; the outer loop in
write_dump() iterates over the modules in the order of modules.order,
and the inner loop dumps symbols in each module.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
2022-05-08 03:17:01 +09:00
Masahiro Yamada
ab489d6002 modpost: traverse the namespace_list in order
Use the doubly linked list to traverse the list in the added order.
This makes the code more consistent.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
2022-05-08 03:17:00 +09:00
Masahiro Yamada
8a69152be9 modpost: traverse unresolved symbols in order
Currently, modpost manages unresolved in a singly linked list; it adds
a new node to the head, and traverses the list from new to old.

Use a doubly linked list to keep the order in the symbol table in the
ELF file.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
2022-05-08 03:17:00 +09:00
Masahiro Yamada
325eba05e8 modpost: traverse modules in order
Currently, modpost manages modules in a singly linked list; it adds
a new node to the head, and traverses the list from new to old.

It works, but the error messages are shown in the reverse order.

If you have a Makefile like this:

  obj-m += foo.o bar.o

then, modpost shows error messages in bar.o, foo.o, in this order.

Use a doubly linked list to keep the order in modules.order; use
list_add_tail() for the node addition and list_for_each_entry() for
the list traverse.

Now that the kernel's list macros have been imported to modpost, I will
use them actively going forward.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
2022-05-08 03:17:00 +09:00
Masahiro Yamada
5066743e4c modpost: change mod->gpl_compatible to bool type
Currently, mod->gpl_compatible is tristate; it is set to -1 by default,
then to 1 or 0 when MODULE_LICENSE() is found.

Maybe, -1 was chosen to represent the 'unknown' license, but it is not
useful.

The current code:

    if (!mod->gpl_compatible)
            check_for_gpl_usage(exp->export, basename, exp->name);

... only cares whether gpl_compatible is zero or not.

Change it to a bool type with the initial value 'true', which has no
functional change.

The default value should be 'true' instead of 'false'.

Since commit 1d6cd39293 ("modpost: turn missing MODULE_LICENSE() into
error"), unknown module license is an error.

The error message, "missing MODULE_LICENSE()" is enough to explain the
issue. It is not sensible to show another message, "GPL-incompatible
module ... uses GPL-only symbol".

Add comments to explain this.

While I was here, I renamed gpl_compatible to is_gpl_compatible for
clarification, and also slightly refactored the code.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
2022-05-08 03:17:00 +09:00
Masahiro Yamada
58e01fcae1 modpost: use bool type where appropriate
Use 'bool' to clarify that the valid value is true or false.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
2022-05-08 03:17:00 +09:00
Masahiro Yamada
70ddb48db4 modpost: move struct namespace_list to modpost.c
There is no good reason to define struct namespace_list in modpost.h

struct module has pointers to struct namespace_list, but that does
not require the definition of struct namespace_list.

Move it to modpost.c.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
2022-05-08 03:17:00 +09:00
Masahiro Yamada
7ce3e410e0 modpost: remove useless export_from_sec()
With commit 1743694eb2 ("modpost: stop symbol preloading for
modversion CRC") applied, now export_from_sec() is useless.

handle_symbol() is called for every symbol in the ELF.

When 'symname' does not start with "__ksymtab", export_from_sec() is
called, and the returned value is stored in 'export'.

It is used in the last part of handle_symbol():

    if (strstarts(symname, "__ksymtab_")) {
            name = symname + strlen("__ksymtab_");
            sym_add_exported(name, mod, export);
    }

'export' is used only when 'symname' starts with "__ksymtab_".

So, the value returned by export_from_sec() is never used.

Remove useless export_from_sec(). This makes further cleanups possible.

I put the temporary code:

    export = export_unknown;

Otherwise, I would get the compiler warning:

    warning: 'export' may be used uninitialized in this function [-Wmaybe-uninitialized]

This is apparently false positive because

    if (strstarts(symname, "__ksymtab_")

... is a stronger condition than:

    if (strstarts(symname, "__ksymtab")

Anyway, this part will be cleaned up by the next commit.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
2022-05-08 03:16:30 +09:00
Masahiro Yamada
e54dd93a08 modpost: get the *.mod file path more simply
get_src_version() strips 'o' or 'lto.o' from the end of the object file
path (so, postfixlen is 1 or 5), then adds 'mod'.

If you look at the code closely, mod->name already holds the base path
with the extension stripped.

Most of the code changes made by commit 7ac204b545 ("modpost: lto:
strip .lto from module names") was actually unneeded.

sumversion.c does not need strends(), so it can get back local in
modpost.c again.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
2021-09-03 08:17:21 +09:00
Linus Torvalds
21a6ab2131 Merge tag 'modules-for-v5.12' of git://git.kernel.org/pub/scm/linux/kernel/git/jeyu/linux
Pull module updates from Jessica Yu:

 - Retire EXPORT_UNUSED_SYMBOL() and EXPORT_SYMBOL_GPL_FUTURE(). These
   export types were introduced between 2006 - 2008. All the of the
   unused symbols have been long removed and gpl future symbols were
   converted to gpl quite a long time ago, and I don't believe these
   export types have been used ever since. So, I think it should be safe
   to retire those export types now (Christoph Hellwig)

 - Refactor and clean up some aged code cruft in the module loader
   (Christoph Hellwig)

 - Build {,module_}kallsyms_on_each_symbol only when livepatching is
   enabled, as it is the only caller (Christoph Hellwig)

 - Unexport find_module() and module_mutex and fix the last module
   callers to not rely on these anymore. Make module_mutex internal to
   the module loader (Christoph Hellwig)

 - Harden ELF checks on module load and validate ELF structures before
   checking the module signature (Frank van der Linden)

 - Fix undefined symbol warning for clang (Fangrui Song)

 - Fix smatch warning (Dan Carpenter)

* tag 'modules-for-v5.12' of git://git.kernel.org/pub/scm/linux/kernel/git/jeyu/linux:
  module: potential uninitialized return in module_kallsyms_on_each_symbol()
  module: remove EXPORT_UNUSED_SYMBOL*
  module: remove EXPORT_SYMBOL_GPL_FUTURE
  module: move struct symsearch to module.c
  module: pass struct find_symbol_args to find_symbol
  module: merge each_symbol_section into find_symbol
  module: remove each_symbol_in_section
  module: mark module_mutex static
  kallsyms: only build {,module_}kallsyms_on_each_symbol when required
  kallsyms: refactor {,module_}kallsyms_on_each_symbol
  module: use RCU to synchronize find_module
  module: unexport find_module and module_mutex
  drm: remove drm_fb_helper_modinit
  powerpc/powernv: remove get_cxl_module
  module: harden ELF info handling
  module: Ignore _GLOBAL_OFFSET_TABLE_ when warning for undefined symbols
2021-02-23 10:15:33 -08:00
Christoph Hellwig
367948220f module: remove EXPORT_UNUSED_SYMBOL*
EXPORT_UNUSED_SYMBOL* is not actually used anywhere.  Remove the
unused functionality as we generally just remove unused code anyway.

Reviewed-by: Miroslav Benes <mbenes@suse.cz>
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jessica Yu <jeyu@kernel.org>
2021-02-08 12:28:07 +01:00
Christoph Hellwig
f1c3d73e97 module: remove EXPORT_SYMBOL_GPL_FUTURE
As far as I can tell this has never been used at all, and certainly
not any time recently.

Reviewed-by: Miroslav Benes <mbenes@suse.cz>
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jessica Yu <jeyu@kernel.org>
2021-02-08 12:28:02 +01:00
Sami Tolvanen
7ac204b545 modpost: lto: strip .lto from module names
With LTO, everything is compiled into LLVM bitcode, so we have to link
each module into native code before modpost. Kbuild uses the .lto.o
suffix for these files, which also ends up in module information. This
change strips the unnecessary .lto suffix from the module name.

Suggested-by: Bill Wendling <morbo@google.com>
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20201211184633.3213045-11-samitolvanen@google.com
2021-01-14 08:21:09 -08:00
Masahiro Yamada
0fd3fbadd9 modpost: refactor error handling and clarify error/fatal difference
We have 3 log functions. fatal() is special because it lets modpost bail
out immediately. The difference between warn() and error() is the only
prefix parts ("WARNING:" vs "ERROR:").

In my understanding, the expected handling of error() is to propagate
the return code of the function to the exit code of modpost, as
check_exports() etc. already does. This is a good manner in general
because we should display as many error messages as possible in a
single run of modpost.

What is annoying about fatal() is that it kills modpost at the first
error. People would need to run Kbuild again and again until they fix
all errors.

But, unfortunately, people tend to do:
"This case should not be allowed. Let's replace warn() with fatal()."

One of the reasons is probably it is tedious to manually hoist the error
code to the main() function.

This commit refactors error() so any single call for it automatically
makes modpost return the error code.

I also added comments in modpost.h for warn(), error(), and fatal().

Please use fatal() only when you have a strong reason to do so.
For example:

  - Memory shortage (i.e. malloc() etc. has failed)
  - The ELF file is broken, and there is no point to continue parsing
  - Something really odd has happened

For general coding errors, please use error().

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Tested-by: Quentin Perret <qperret@google.com>
2020-12-21 13:57:08 +09:00
Masahiro Yamada
bc72d723ec modpost: rename merror() to error()
The log function names, warn(), merror(), fatal() are inconsistent.

Commit 2a11665945 ("kbuild: distinguish between errors and warnings
in modpost") intentionally chose merror() to avoid the conflict with
the library function error(). See man page of error(3).

But, we are already causing the conflict with warn() because it is also
a library function. See man page of warn(3). err() would be a problem
for the same reason.

The common technique to work around name conflicts is to use macros.
For example:

    /* in a header */
    #define error(fmt, ...)  __error(fmt, ##__VA_ARGS__)
    #define warn(fmt, ...)   __warn(fmt, ##__VA_ARGS__)

    /* function definition */
    void __error(const char *fmt, ...)
    {
            <our implementation>
    }

    void __warn(const char *fmt, ...)
    {
            <our implementation>
    }

In this way, we can implement our own warn() and error(), still we can
include <error.h> and <err.h> with no problem.

And, commit 93c95e526a ("modpost: rework and consolidate logging
interface") already did that.

Since the log functions are all macros, we can use error() without
causing "conflicting types" errors.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
2020-12-21 13:57:08 +09:00
Masahiro Yamada
3b09efc4f0 modpost: change elf_info->size to size_t
Align with the mmap / munmap APIs.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
2020-06-06 23:39:20 +09:00
Masahiro Yamada
a82f794c41 modpost: strip .o from modname before calling new_module()
new_module() conditionally strips the .o because the modname has .o
suffix when it is called from read_symbols(), but no .o when it is
called from read_dump().

It is clearer to strip .o in read_symbols().

I also used flexible-array for mod->name.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
2020-06-06 23:39:20 +09:00
Masahiro Yamada
0b19d54cae modpost: remove mod->skip struct member
The meaning of 'skip' is obscure since it does not explain
"what to skip".

mod->skip is set when it is vmlinux or the module info came from
a dump file.

So, mod->skip is equivalent to (mod->is_vmlinux || mod->from_dump).

For the check in write_namespace_deps_files(), mod->is_vmlinux is
unneeded because the -d option is not passed in the first pass of
modpost.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
2020-06-06 23:39:20 +09:00
Masahiro Yamada
5a438af9db modpost: add mod->is_vmlinux struct member
is_vmlinux() is called in several places to check whether the current
module is vmlinux or not.

It is faster and clearer to check mod->is_vmlinux flag.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
2020-06-06 23:39:19 +09:00
Masahiro Yamada
3379576dd6 modpost: remove mod->is_dot_o struct member
Previously, there were two cases where mod->is_dot_o is unset:

[1] the executable 'vmlinux' in the second pass of modpost
[2] modules loaded by read_dump()

I think [1] was intended usage to distinguish 'vmlinux.o' and 'vmlinux'.
Now that modpost does not parse the executable 'vmlinux', this case
does not happen.

[2] is obscure, maybe a bug. Module.symver stores module paths without
extension. So, none of modules loaded by read_dump() has the .o suffix,
and new_module() unsets ->is_dot_o. Anyway, it is not a big deal because
handle_symbol() is not called for the case.

To sum up, all the parsed ELF files are .o files.

mod->is_dot_o is unneeded.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
2020-06-06 23:38:13 +09:00