| Message ID | 20161129135118.24696-1-kilobyte@angband.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Dne 29.11.2016 v 16:27 Linus Torvalds napsal(a): > On Nov 29, 2016 5:51 AM, "Adam Borowski" <kilobyte@angband.pl > <mailto:kilobyte@angband.pl>> wrote: >> > >> > >> > (a) tested >> >> By many people. > > No. > > I've tested the build *without* this, and it works fine. > >> > (b) explains it >> >> The actual logic is in 4efca4ed0. It wants C prototypes defined in >> asm/asm-prototypes.h that lists symbols defined in assembly -- genksyms >> knows only how to read C code. > > See above. I'm not taking more random patches that "fix" this when it's > not broken for me. Not without very explicit explanations of why that > patch is still needed for others. The original and easily observable bug is that were are not generating symbol checksums for the asm-exported symbols, so they default to 0. This can be seen e.g. in the Module.symvers file. This seemed like a minor issue, because with the functions written in asm, the type checking is rather weak (this has been the case even before Al's patches). However, there is another bug that with _some_ toolchains / architectures, the checksums do not default to 0, but they are simply missing in the ___kcrctab* sections and the module loader complains. We can of course research into the details of the second bug, but we already know that we are not generating the checksums while we should be. Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 29, 2016 at 8:03 AM, Michal Marek <mmarek@suse.com> wrote: > > The original and easily observable bug is that were are not generating > symbol checksums for the asm-exported symbols, so they default to 0. > This can be seen e.g. in the Module.symvers file. This seemed like a > minor issue, because with the functions written in asm, the type > checking is rather weak (this has been the case even before Al's > patches). However, there is another bug that with _some_ toolchains / > architectures, the checksums do not default to 0, but they are simply > missing in the ___kcrctab* sections and the module loader complains. We > can of course research into the details of the second bug, but we > already know that we are not generating the checksums while we should be. So let's just say that "toolchain is buggy" and make a missing kcrctab entry mean zero (or mean "matches anything"). And just shut up the warning. I do *not* want to add random bandaids for something like a broken toolchain issue when I'd really rather just delete the feature. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 29, 2016 at 07:27:12AM -0800, Linus Torvalds wrote: > On Nov 29, 2016 5:51 AM, "Adam Borowski" <kilobyte@angband.pl> wrote: > > > > > > (a) tested > > > > By many people. > > No. > > I've tested the build *without* this, and it works fine. Michal mentioned "why", let's try "where". I have no idea what setup is required to trigger the problem, but here's a simple sufficient one: Current Debian unstable, amd64. git reset --hard v4.9-rc7 git revert cd3caefb make defconfig CONFIG_MODVERSIONS=y (in my case) CONFIG_BTRFS_FS=y (so I can boot) enable a module for testing, I used CONFIG_EXT4_FS=m make bindeb-pkg dpkg -i linux-image_XXXXX.deb modprobe ext4 [ 63.779490] jbd2: no symbol version for memcpy [ 63.779492] jbd2: Unknown symbol memcpy (err -22) [ 63.779550] jbd2: no symbol version for phys_base [ 63.779551] jbd2: Unknown symbol phys_base (err -22) [ 63.779561] jbd2: no symbol version for memset [ 63.779562] jbd2: Unknown symbol memset (err -22) Not sure which piece of toolchain matters here, someone said binutils. In that case, Fedora ships 2.26.1-1.fc25, they were frozen so couldn't update. Debian is at 2.27.51.20161127-1, Gentoo at 2.27, same for Arch, OpenSUSE; Ubuntu at 2.27.51.20161124-1. Thus, if it's indeed binutils, you'll see the breakage as soon as Fedora recovers from the freeze. Meow!
diff --git a/arch/x86/include/asm/asm-prototypes.h b/arch/x86/include/asm/asm-prototypes.h new file mode 100644 index 0000000..ae87224 --- /dev/null +++ b/arch/x86/include/asm/asm-prototypes.h @@ -0,0 +1,12 @@ +#include <asm/ftrace.h> +#include <asm/uaccess.h> +#include <asm/string.h> +#include <asm/page.h> +#include <asm/checksum.h> + +#include <asm-generic/asm-prototypes.h> + +#include <asm/page.h> +#include <asm/pgtable.h> +#include <asm/special_insns.h> +#include <asm/preempt.h> diff --git a/include/asm-generic/asm-prototypes.h b/include/asm-generic/asm-prototypes.h new file mode 100644 index 0000000..df13637 --- /dev/null +++ b/include/asm-generic/asm-prototypes.h @@ -0,0 +1,7 @@ +#include <linux/bitops.h> +extern void *__memset(void *, int, __kernel_size_t); +extern void *__memcpy(void *, const void *, __kernel_size_t); +extern void *__memmove(void *, const void *, __kernel_size_t); +extern void *memset(void *, int, __kernel_size_t); +extern void *memcpy(void *, const void *, __kernel_size_t); +extern void *memmove(void *, const void *, __kernel_size_t);
Commit 4efca4ed ("kbuild: modversions for EXPORT_SYMBOL() for asm") adds modversion support for symbols exported from asm files. Architectures must include C-style declarations for those symbols in asm/asm-prototypes.h in order for them to be versioned. Add these declarations for x86, and an architecture-independent file that can be used for common symbols. User impact: kernels may fail to load modules at all when CONFIG_MODVERSIONS=y. Signed-off-by: Adam Borowski <kilobyte@angband.pl> Tested-by: Kalle Valo <kvalo@codeaurora.org> Acked-by: Nicholas Piggin <npiggin@gmail.com> Tested-by: Peter Wu <peter@lekensteyn.nl> Tested-by: Oliver Hartkopp <socketcan@hartkopp.net> --- > So somebody send me a minimal patch that is > > (a) tested By many people. > (b) explains it The actual logic is in 4efca4ed0. It wants C prototypes defined in asm/asm-prototypes.h that lists symbols defined in assembly -- genksyms knows only how to read C code. > (c) obvious To be honest I don't quite understand what's the real gain over code that was removed by 784d5699eddc, this mostly brings the symbols back. But that's for people wiser than me to explain. > and I'll happily re-enable modversions. The powerpc counterpart to this patch is in mainline as 9e5f688, although a file by that name already existed. As for arm, it looks like it was handled the other way by 8478132.