[ Upstream commit 3a3f1e573a105328a2cca45a7cfbebabbf5e3192 ]
The > comparison should be >= to prevent an out of bounds array
access.
Fixes: 52dc0595d5 ("modpost: handle relocations mismatch in __ex_table.")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit 56a24b8ce6a7f9c4a21b2276a8644f6f3d8fc14d ]
addend_arm_rel() processes R_ARM_PC24, R_ARM_CALL, R_ARM_JUMP24 in a
wrong way.
Here, test code.
[test code for R_ARM_JUMP24]
.section .init.text,"ax"
bar:
bx lr
.section .text,"ax"
.globl foo
foo:
b bar
[test code for R_ARM_CALL]
.section .init.text,"ax"
bar:
bx lr
.section .text,"ax"
.globl foo
foo:
push {lr}
bl bar
pop {pc}
If you compile it with ARM multi_v7_defconfig, modpost will show the
symbol name, (unknown).
WARNING: modpost: vmlinux.o: section mismatch in reference: foo (section: .text) -> (unknown) (section: .init.text)
(You need to use GNU linker instead of LLD to reproduce it.)
Fix the code to make modpost show the correct symbol name.
I imported (with adjustment) sign_extend32() from include/linux/bitops.h.
The '+8' is the compensation for pc-relative instruction. It is
documented in "ELF for the Arm Architecture" [1].
"If the relocation is pc-relative then compensation for the PC bias
(the PC value is 8 bytes ahead of the executing instruction in Arm
state and 4 bytes in Thumb state) must be encoded in the relocation
by the object producer."
[1]: https://github.com/ARM-software/abi-aa/blob/main/aaelf32/aaelf32.rst
Fixes: 56a974fa2d ("kbuild: make better section mismatch reports on arm")
Fixes: 6e2e340b59 ("ARM: 7324/1: modpost: Fix section warnings for ARM for many compilers")
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit b7c63520f6703a25eebb4f8138fed764fcae1c6f ]
addend_arm_rel() processes R_ARM_ABS32 in a wrong way.
Here, test code.
[test code 1]
#include <linux/init.h>
int __initdata foo;
int get_foo(void) { return foo; }
If you compile it with ARM versatile_defconfig, modpost will show the
symbol name, (unknown).
WARNING: modpost: vmlinux.o: section mismatch in reference: get_foo (section: .text) -> (unknown) (section: .init.data)
(You need to use GNU linker instead of LLD to reproduce it.)
If you compile it for other architectures, modpost will show the correct
symbol name.
WARNING: modpost: vmlinux.o: section mismatch in reference: get_foo (section: .text) -> foo (section: .init.data)
For R_ARM_ABS32, addend_arm_rel() sets r->r_addend to a wrong value.
I just mimicked the code in arch/arm/kernel/module.c.
However, there is more difficulty for ARM.
Here, test code.
[test code 2]
#include <linux/init.h>
int __initdata foo;
int get_foo(void) { return foo; }
int __initdata bar;
int get_bar(void) { return bar; }
With this commit applied, modpost will show the following messages
for ARM versatile_defconfig:
WARNING: modpost: vmlinux.o: section mismatch in reference: get_foo (section: .text) -> foo (section: .init.data)
WARNING: modpost: vmlinux.o: section mismatch in reference: get_bar (section: .text) -> foo (section: .init.data)
The reference from 'get_bar' to 'foo' seems wrong.
I have no solution for this because it is true in assembly level.
In the following output, relocation at 0x1c is no longer associated
with 'bar'. The two relocation entries point to the same symbol, and
the offset to 'bar' is encoded in the instruction 'r0, [r3, #4]'.
Disassembly of section .text:
00000000 <get_foo>:
0: e59f3004 ldr r3, [pc, #4] @ c <get_foo+0xc>
4: e5930000 ldr r0, [r3]
8: e12fff1e bx lr
c: 00000000 .word 0x00000000
00000010 <get_bar>:
10: e59f3004 ldr r3, [pc, #4] @ 1c <get_bar+0xc>
14: e5930004 ldr r0, [r3, #4]
18: e12fff1e bx lr
1c: 00000000 .word 0x00000000
Relocation section '.rel.text' at offset 0x244 contains 2 entries:
Offset Info Type Sym.Value Sym. Name
0000000c 00000c02 R_ARM_ABS32 00000000 .init.data
0000001c 00000c02 R_ARM_ABS32 00000000 .init.data
When find_elf_symbol() gets into a situation where relsym->st_name is
zero, there is no guarantee to get the symbol name as written in C.
I am keeping the current logic because it is useful in many architectures,
but the symbol name is not always correct depending on the optimization.
I left some comments in find_tosym().
Fixes: 56a974fa2d ("kbuild: make better section mismatch reports on arm")
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit d0acc76a49aa917c1a455d11d32d34a01e8b2835 ]
find_extable_entry_size() is completely broken. It has awesome comments
about how to calculate sizeof(struct exception_table_entry).
It was based on these assumptions:
- struct exception_table_entry has two fields
- both of the fields have the same size
Then, we came up with this equation:
(offset of the second field) * 2 == (size of struct)
It was true for all architectures when commit 52dc0595d5 ("modpost:
handle relocations mismatch in __ex_table.") was applied.
Our mathematics broke when commit 548acf1923 ("x86/mm: Expand the
exception table logic to allow new handling options") introduced the
third field.
Now, the definition of exception_table_entry is highly arch-dependent.
For x86, sizeof(struct exception_table_entry) is apparently 12, but
find_extable_entry_size() sets extable_entry_size to 8.
I could fix it, but I do not see much value in this code.
extable_entry_size is used just for selecting a slightly different
error message.
If the first field ("insn") references to a non-executable section,
The relocation at %s+0x%lx references
section "%s" which is not executable, IOW
it is not possible for the kernel to fault
at that address. Something is seriously wrong
and should be fixed.
If the second field ("fixup") references to a non-executable section,
The relocation at %s+0x%lx references
section "%s" which is not executable, IOW
the kernel will fault if it ever tries to
jump to it. Something is seriously wrong
and should be fixed.
Merge the two error messages rather than adding even more complexity.
Change fatal() to error() to make it continue running and catch more
possible errors.
Fixes: 548acf1923 ("x86/mm: Expand the exception table logic to allow new handling options")
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
commit fb27e70f6e408dee5d22b083e7a38a59e6118253 upstream.
modpost now reads CRCs from .*.cmd files, parsing them using strtol().
This is inconsistent with its parsing of Module.symvers and with their
definition as *unsigned* 32-bit values.
strtol() clamps values to [LONG_MIN, LONG_MAX], and when building on a
32-bit system this changes all CRCs >= 0x80000000 to be 0x7fffffff.
Change extract_crcs_for_object() to use strtoul() instead.
Cc: stable@vger.kernel.org
Fixes: f292d875d0 ("modpost: extract symbol versions from *.cmd files")
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Since commit 7b4537199a ("kbuild: link symbol CRCs at final link,
removing CONFIG_MODULE_REL_CRCS"), module versioning is broken on
some architectures. Loading a module fails with "disagrees about
version of symbol module_layout".
On such architectures (e.g. ARCH=sparc build with sparc64_defconfig),
modpost shows a warning, like follows:
WARNING: modpost: EXPORT symbol "_mcount" [vmlinux] version generation failed, symbol will not be versioned.
Is "_mcount" prototyped in <asm/asm-prototypes.h>?
Previously, it was a harmless warning (CRC check was just skipped),
but now wrong CRCs are used for comparison because invalid CRCs are
just skipped.
$ sparc64-linux-gnu-nm -n vmlinux
[snip]
0000000000c2cea0 r __ksymtab__kstrtol
0000000000c2ceb8 r __ksymtab__kstrtoul
0000000000c2ced0 r __ksymtab__local_bh_enable
0000000000c2cee8 r __ksymtab__mcount
0000000000c2cf00 r __ksymtab__printk
0000000000c2cf18 r __ksymtab__raw_read_lock
0000000000c2cf30 r __ksymtab__raw_read_lock_bh
[snip]
0000000000c53b34 D __crc__kstrtol
0000000000c53b38 D __crc__kstrtoul
0000000000c53b3c D __crc__local_bh_enable
0000000000c53b40 D __crc__printk
0000000000c53b44 D __crc__raw_read_lock
0000000000c53b48 D __crc__raw_read_lock_bh
Please notice __crc__mcount is missing here.
When the module subsystem looks up a CRC that comes after, it results
in reading out a wrong address. For example, when __crc__printk is
needed, the module subsystem reads 0xc53b44 instead of 0xc53b40.
All CRC entries must be output for correct index accessing. Invalid
CRCs will be unused, but are needed to keep the one-to-one mapping
between __ksymtab_* and __crc_*.
The best is to fix all modpost warnings, but several warnings are still
remaining on less popular architectures.
Fixes: 7b4537199a ("kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS")
Reported-by: matoro <matoro_mailinglist_kernel@matoro.tk>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Tested-by: matoro <matoro_mailinglist_kernel@matoro.tk>
Pull Kbuild updates from Masahiro Yamada:
- Remove the support for -O3 (CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE_O3)
- Fix error of rpm-pkg cross-builds
- Support riscv for checkstack tool
- Re-enable -Wformwat warnings for Clang
- Clean up modpost, Makefiles, and misc scripts
* tag 'kbuild-v5.20' of git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild: (30 commits)
modpost: remove .symbol_white_list field entirely
modpost: remove unneeded .symbol_white_list initializers
modpost: add PATTERNS() helper macro
modpost: shorten warning messages in report_sec_mismatch()
Revert "Kbuild, lto, workaround: Don't warn for initcall_reference in modpost"
modpost: use more reliable way to get fromsec in section_rel(a)()
modpost: add array range check to sec_name()
modpost: refactor get_secindex()
kbuild: set EXIT trap before creating temporary directory
modpost: remove unused Elf_Sword macro
Makefile.extrawarn: re-enable -Wformat for clang
kbuild: add dtbs_prepare target
kconfig: Qt5: tell the user which packages are required
modpost: use sym_get_data() to get module device_table data
modpost: drop executable ELF support
checkstack: add riscv support for scripts/checkstack.pl
kconfig: shorten the temporary directory name for cc-option
scripts: headers_install.sh: Update config leak ignore entries
kbuild: error out if $(INSTALL_MOD_PATH) contains % or :
kbuild: error out if $(KBUILD_EXTMOD) contains % or :
...
It is not so useful to have symbol whitelists in arrays. With this
over-engineering, the code is difficult to follow.
Let's do it more directly, and collect the relevant code to one place.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
The ->symbol_white_list field is referenced in secref_whitelist(),
only when 'fromsec' is data_sections.
/* Check for pattern 2 */
if (match(tosec, init_exit_sections) &&
match(fromsec, data_sections) &&
match(fromsym, mismatch->symbol_white_list))
return 0;
If .fromsec is not data sections, the .symbol_white_list member is
not used by anyone.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
This will be useful to define a NULL-terminated array inside a function
call.
Currently, string arrays passed to match() are defined in separate
places:
static const char *const init_sections[] = { ALL_INIT_SECTIONS, NULL };
static const char *const text_sections[] = { ALL_TEXT_SECTIONS, NULL };
static const char *const optim_symbols[] = { "*.constprop.*", NULL };
...
/* Check for pattern 5 */
if (match(fromsec, text_sections) &&
match(tosec, init_sections) &&
match(fromsym, optim_symbols))
return 0;
With the new helper macro, you can list the patterns directly in the
function call, like this:
/* Check for pattern 5 */
if (match(fromsec, PATTERNS(ALL_TEXT_SECTIONS)) &&
match(tosec, PATTERNS(ALL_INIT_SECTIONS)) &&
match(fromsym, PATTERNS("*.contprop.*")))
return 0;
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Each section mismatch results in long warning messages. Too much.
Make each warning fit in one line, and remove a lot of messy code.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
This reverts commit 77ab21adae.
Even after 8 years later, GCC LTO has not been upstreamed. Also, it said
"This is a workaround". If this is needed in the future, it should be
added in a proper way.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Acked-by: Jiri Slaby <jirislaby@kernel.org>
The section name of Rel and Rela starts with ".rel" and ".rela"
respectively (but, I do not know whether this is specification or
convention).
For example, ".rela.text" holds relocation entries applied to the
".text" section.
So, the code chops the ".rel" or ".rela" prefix to get the name of
the section to which the relocation applies.
However, I do not like to skip 4 or 5 bytes blindly because it is
potential memory overrun.
The ELF specification provides a more reliable way to do this.
- The sh_info field holds extra information, whose interpretation
depends on the section type
- If the section type is SHT_REL or SHT_RELA, the sh_info field holds
the section header index of the section to which the relocation
applies.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
The section index is always positive, so the argument, secindex, should
be unsigned.
Also, inserted the array range check.
If sym->st_shndx is a special section index (between SHN_LORESERVE and
SHN_HIRESERVE), there is no corresponding section header.
For example, if a symbol specifies an absolute value, sym->st_shndx is
SHN_ABS (=0xfff1).
The current users do not cause the out-of-range access of
info->sechddrs[], but it is better to avoid such a pitfall.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
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>
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>
Pull KUnit updates from Shuah Khan:
"This consists of several fixes and an important feature to discourage
running KUnit tests on production systems. Running tests on a
production system could leave the system in a bad state.
Summary:
- Add a new taint type, TAINT_TEST to signal that a test has been
run.
This should discourage people from running these tests on
production systems, and to make it easier to tell if tests have
been run accidentally (by loading the wrong configuration, etc)
- Several documentation and tool enhancements and fixes"
* tag 'linux-kselftest-kunit-5.20-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest: (29 commits)
Documentation: KUnit: Fix example with compilation error
Documentation: kunit: Add CLI args for kunit_tool
kcsan: test: Add a .kunitconfig to run KCSAN tests
kunit: executor: Fix a memory leak on failure in kunit_filter_tests
clk: explicitly disable CONFIG_UML_PCI_OVER_VIRTIO in .kunitconfig
mmc: sdhci-of-aspeed: test: Use kunit_test_suite() macro
nitro_enclaves: test: Use kunit_test_suite() macro
thunderbolt: test: Use kunit_test_suite() macro
kunit: flatten kunit_suite*** to kunit_suite** in .kunit_test_suites
kunit: unify module and builtin suite definitions
selftest: Taint kernel when test module loaded
module: panic: Taint the kernel when selftest modules load
Documentation: kunit: fix example run_kunit func to allow spaces in args
Documentation: kunit: Cleanup run_wrapper, fix x-ref
kunit: test.h: fix a kernel-doc markup
kunit: tool: Enable virtio/PCI by default on UML
kunit: tool: make --kunitconfig repeatable, blindly concat
kunit: add coverage_uml.config to enable GCOV on UML
kunit: tool: refactor internal kconfig handling, allow overriding
kunit: tool: introduce --qemu_args
...
Since commit 269a535ca9 ("modpost: generate vmlinux.symvers and
reuse it for the second modpost"), modpost only parses relocatable
files (ET_REL).
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
This reverts commit 4d10c223ba.
Commit 37744feebc ("sh: remove sh5 support") removed the sh64 support
entirely.
Note:
.cranges was only used for sh64 ever.
Commit 211dc24b8744 ("Remove sh5 and sh64 support") in binutils-gdb
already removed the relevant code.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Taint the kernel with TAINT_TEST whenever a test module loads, by adding
a new "TEST" module property, and setting it for all modules in the
tools/testing directory. This property can also be set manually, for
tests which live outside the tools/testing directory with:
MODULE_INFO(test, "Y");
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Reviewed-by: Aaron Tomlin <atomlin@redhat.com>
Acked-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: David Gow <davidgow@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Since commit f02e8a6596 ("module: Sort exported symbols"),
EXPORT_SYMBOL* is placed in the individual section ___ksymtab(_gpl)+<sym>
(3 leading underscores instead of 2).
Since then, modpost cannot detect the bad combination of EXPORT_SYMBOL
and __init/__exit.
Fix the .fromsec field.
Fixes: f02e8a6596 ("module: Sort exported symbols")
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Pull more Kbuild updates from Masahiro Yamada:
- Fix build regressions for parisc, csky, nios2, openrisc
- Simplify module builds for CONFIG_LTO_CLANG and CONFIG_X86_KERNEL_IBT
- Remove arch/parisc/nm, which was presumably a workaround for old
tools
- Check the odd combination of EXPORT_SYMBOL and 'static' precisely
- Make external module builds robust against "too long argument error"
- Support j, k keys for moving the cursor in nconfig
* tag 'kbuild-v5.19-3' of git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild: (25 commits)
kbuild: Allow to select bash in a modified environment
scripts: kconfig: nconf: make nconfig accept jk keybindings
modpost: use fnmatch() to simplify match()
modpost: simplify mod->name allocation
kbuild: factor out the common objtool arguments
kbuild: move vmlinux.o link to scripts/Makefile.vmlinux_o
kbuild: clean .tmp_* pattern by make clean
kbuild: remove redundant cleanups in scripts/link-vmlinux.sh
kbuild: rebuild multi-object modules when objtool is updated
kbuild: add cmd_and_savecmd macro
kbuild: make *.mod rule robust against too long argument error
kbuild: make built-in.a rule robust against too long argument error
kbuild: check static EXPORT_SYMBOL* by script instead of modpost
parisc: remove arch/parisc/nm
kbuild: do not create *.prelink.o for Clang LTO or IBT
kbuild: replace $(linked-object) with CONFIG options
kbuild: do not try to parse *.cmd files for objects provided by compiler
kbuild: replace $(if A,A,B) with $(or A,B) in scripts/Makefile.modpost
modpost: squash if...else-if in find_elf_symbol2()
modpost: reuse ARRAY_SIZE() macro for section_mismatch()
...
Replace the own implementation for wildcard (glob) matching with
a function call to fnmatch().
Also, change the return type to 'bool'.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
mod->name is set to the ELF filename with the suffix ".o" stripped.
The current code calls strdup() and free() to manipulate the string,
but a simpler approach is to pass new_module() with the name length
subtracted by 2.
Also, check if the passed filename ends with ".o" before stripping it.
The current code blindly chops the suffix:
tmp[strlen(tmp) - 2] = '\0'
It will cause buffer under-run if strlen(tmp) < 2;
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
