| Message ID | 20180410012206.4395-1-mpe@ellerman.id.au (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 73aca179d78eaa11604ba0783a6d8b2125fbc332 |
| Headers | show |
| Series | powerpc/modules: Fix crashes by adding CONFIG_RELOCATABLE to vermagic | expand |
On Tue, 2018-04-10 at 01:22:06 UTC, Michael Ellerman wrote: > If you build the kernel with CONFIG_RELOCATABLE=n, then install the > modules, rebuild the kernel with CONFIG_RELOCATABLE=y and leave the > old modules installed, we crash something like: > > Unable to handle kernel paging request for data at address 0xd000000018d66cef > Faulting instruction address: 0xc0000000021ddd08 > Oops: Kernel access of bad area, sig: 11 [#1] > Modules linked in: x_tables autofs4 > CPU: 2 PID: 1 Comm: systemd Not tainted 4.16.0-rc6-gcc_ubuntu_le-g99fec39 #1 > ... > NIP check_version.isra.22+0x118/0x170 > Call Trace: > __ksymtab_xt_unregister_table+0x58/0xfffffffffffffcb8 [x_tables] (unreliable) > resolve_symbol+0xb4/0x150 > load_module+0x10e8/0x29a0 > SyS_finit_module+0x110/0x140 > system_call+0x58/0x6c > > This happens because since commit 71810db27c1c ("modversions: treat > symbol CRCs as 32 bit quantities"), a relocatable kernel encodes and > handles symbol CRCs differently from a non-relocatable kernel. > > Although it's possible we could try and detect this situation and > handle it, it's much more robust to simply make the state of > CONFIG_RELOCATABLE part of the module vermagic. > > Fixes: 71810db27c1c ("modversions: treat symbol CRCs as 32 bit quantities") > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> Applied to powerpc fixes. https://git.kernel.org/powerpc/c/73aca179d78eaa11604ba0783a6d8b cheers
On 11 April 2018 at 16:49, Michael Ellerman <patch-notifications@ellerman.id.au> wrote: > On Tue, 2018-04-10 at 01:22:06 UTC, Michael Ellerman wrote: >> If you build the kernel with CONFIG_RELOCATABLE=n, then install the >> modules, rebuild the kernel with CONFIG_RELOCATABLE=y and leave the >> old modules installed, we crash something like: >> >> Unable to handle kernel paging request for data at address 0xd000000018d66cef >> Faulting instruction address: 0xc0000000021ddd08 >> Oops: Kernel access of bad area, sig: 11 [#1] >> Modules linked in: x_tables autofs4 >> CPU: 2 PID: 1 Comm: systemd Not tainted 4.16.0-rc6-gcc_ubuntu_le-g99fec39 #1 >> ... >> NIP check_version.isra.22+0x118/0x170 >> Call Trace: >> __ksymtab_xt_unregister_table+0x58/0xfffffffffffffcb8 [x_tables] (unreliable) >> resolve_symbol+0xb4/0x150 >> load_module+0x10e8/0x29a0 >> SyS_finit_module+0x110/0x140 >> system_call+0x58/0x6c >> >> This happens because since commit 71810db27c1c ("modversions: treat >> symbol CRCs as 32 bit quantities"), a relocatable kernel encodes and >> handles symbol CRCs differently from a non-relocatable kernel. >> >> Although it's possible we could try and detect this situation and >> handle it, it's much more robust to simply make the state of >> CONFIG_RELOCATABLE part of the module vermagic. >> >> Fixes: 71810db27c1c ("modversions: treat symbol CRCs as 32 bit quantities") >> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > > Applied to powerpc fixes. > > https://git.kernel.org/powerpc/c/73aca179d78eaa11604ba0783a6d8b > > cheers Thanks for the cc. I guess this only affects powerpc, given that it is the only arch that switches between CRC immediate values and CRC offsets depending on the configuration.
Ard Biesheuvel <ard.biesheuvel@linaro.org> writes: > On 11 April 2018 at 16:49, Michael Ellerman > <patch-notifications@ellerman.id.au> wrote: >> On Tue, 2018-04-10 at 01:22:06 UTC, Michael Ellerman wrote: >>> If you build the kernel with CONFIG_RELOCATABLE=n, then install the >>> modules, rebuild the kernel with CONFIG_RELOCATABLE=y and leave the >>> old modules installed, we crash something like: >>> >>> Unable to handle kernel paging request for data at address 0xd000000018d66cef >>> Faulting instruction address: 0xc0000000021ddd08 >>> Oops: Kernel access of bad area, sig: 11 [#1] >>> Modules linked in: x_tables autofs4 >>> CPU: 2 PID: 1 Comm: systemd Not tainted 4.16.0-rc6-gcc_ubuntu_le-g99fec39 #1 >>> ... >>> NIP check_version.isra.22+0x118/0x170 >>> Call Trace: >>> __ksymtab_xt_unregister_table+0x58/0xfffffffffffffcb8 [x_tables] (unreliable) >>> resolve_symbol+0xb4/0x150 >>> load_module+0x10e8/0x29a0 >>> SyS_finit_module+0x110/0x140 >>> system_call+0x58/0x6c >>> >>> This happens because since commit 71810db27c1c ("modversions: treat >>> symbol CRCs as 32 bit quantities"), a relocatable kernel encodes and >>> handles symbol CRCs differently from a non-relocatable kernel. >>> >>> Although it's possible we could try and detect this situation and >>> handle it, it's much more robust to simply make the state of >>> CONFIG_RELOCATABLE part of the module vermagic. >>> >>> Fixes: 71810db27c1c ("modversions: treat symbol CRCs as 32 bit quantities") >>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> >> >> Applied to powerpc fixes. >> >> https://git.kernel.org/powerpc/c/73aca179d78eaa11604ba0783a6d8b > > Thanks for the cc. I guess this only affects powerpc, given that it is > the only arch that switches between CRC immediate values and CRC > offsets depending on the configuration. No worries. Is there any reason we shouldn't always turn on CONFIG_MODULE_REL_CRCS? It seems to work, but I wanted to test it more before switching to that, hence the quick fix above. arch/um looks like it might be switching based on config, but I don't know enough to say: config LD_SCRIPT_STATIC bool default y depends on STATIC_LINK config LD_SCRIPT_DYN bool default y depends on !LD_SCRIPT_STATIC select MODULE_REL_CRCS if MODVERSIONS cheers
On 16 April 2018 at 16:10, Michael Ellerman <mpe@ellerman.id.au> wrote: > Ard Biesheuvel <ard.biesheuvel@linaro.org> writes: > >> On 11 April 2018 at 16:49, Michael Ellerman >> <patch-notifications@ellerman.id.au> wrote: >>> On Tue, 2018-04-10 at 01:22:06 UTC, Michael Ellerman wrote: >>>> If you build the kernel with CONFIG_RELOCATABLE=n, then install the >>>> modules, rebuild the kernel with CONFIG_RELOCATABLE=y and leave the >>>> old modules installed, we crash something like: >>>> >>>> Unable to handle kernel paging request for data at address 0xd000000018d66cef >>>> Faulting instruction address: 0xc0000000021ddd08 >>>> Oops: Kernel access of bad area, sig: 11 [#1] >>>> Modules linked in: x_tables autofs4 >>>> CPU: 2 PID: 1 Comm: systemd Not tainted 4.16.0-rc6-gcc_ubuntu_le-g99fec39 #1 >>>> ... >>>> NIP check_version.isra.22+0x118/0x170 >>>> Call Trace: >>>> __ksymtab_xt_unregister_table+0x58/0xfffffffffffffcb8 [x_tables] (unreliable) >>>> resolve_symbol+0xb4/0x150 >>>> load_module+0x10e8/0x29a0 >>>> SyS_finit_module+0x110/0x140 >>>> system_call+0x58/0x6c >>>> >>>> This happens because since commit 71810db27c1c ("modversions: treat >>>> symbol CRCs as 32 bit quantities"), a relocatable kernel encodes and >>>> handles symbol CRCs differently from a non-relocatable kernel. >>>> >>>> Although it's possible we could try and detect this situation and >>>> handle it, it's much more robust to simply make the state of >>>> CONFIG_RELOCATABLE part of the module vermagic. >>>> >>>> Fixes: 71810db27c1c ("modversions: treat symbol CRCs as 32 bit quantities") >>>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> >>> >>> Applied to powerpc fixes. >>> >>> https://git.kernel.org/powerpc/c/73aca179d78eaa11604ba0783a6d8b >> >> Thanks for the cc. I guess this only affects powerpc, given that it is >> the only arch that switches between CRC immediate values and CRC >> offsets depending on the configuration. > > No worries. > > Is there any reason we shouldn't always turn on CONFIG_MODULE_REL_CRCS? > It seems to work, but I wanted to test it more before switching to that, > hence the quick fix above. > > > arch/um looks like it might be switching based on config, but I don't > know enough to say: > > config LD_SCRIPT_STATIC > bool > default y > depends on STATIC_LINK > > config LD_SCRIPT_DYN > bool > default y > depends on !LD_SCRIPT_STATIC > select MODULE_REL_CRCS if MODVERSIONS > The only reason not to enable it is that it ends up taking more space on a 32-bit architecture with CONFIG_RELOCATABLE=n, given that you need to record both the relative offset and the actual CRC value (both 32-bit quantities) rather than just the CRC itself. On a 64-bit arch with CONFIG_RELOCATABLE=n, you end up replacing a single 64-bit quantity with two 32-bit quantities, so it doesn't really matter.
diff --git a/arch/powerpc/include/asm/module.h b/arch/powerpc/include/asm/module.h index 7e28442827f1..4f6573934792 100644 --- a/arch/powerpc/include/asm/module.h +++ b/arch/powerpc/include/asm/module.h @@ -15,9 +15,19 @@ #ifdef CC_USING_MPROFILE_KERNEL -#define MODULE_ARCH_VERMAGIC "mprofile-kernel" +#define MODULE_ARCH_VERMAGIC_FTRACE "mprofile-kernel " +#else +#define MODULE_ARCH_VERMAGIC_FTRACE "" #endif +#ifdef CONFIG_RELOCATABLE +#define MODULE_ARCH_VERMAGIC_RELOCATABLE "relocatable " +#else +#define MODULE_ARCH_VERMAGIC_RELOCATABLE "" +#endif + +#define MODULE_ARCH_VERMAGIC MODULE_ARCH_VERMAGIC_FTRACE MODULE_ARCH_VERMAGIC_RELOCATABLE + #ifndef __powerpc64__ /* * Thanks to Paul M for explaining this.
If you build the kernel with CONFIG_RELOCATABLE=n, then install the modules, rebuild the kernel with CONFIG_RELOCATABLE=y and leave the old modules installed, we crash something like: Unable to handle kernel paging request for data at address 0xd000000018d66cef Faulting instruction address: 0xc0000000021ddd08 Oops: Kernel access of bad area, sig: 11 [#1] Modules linked in: x_tables autofs4 CPU: 2 PID: 1 Comm: systemd Not tainted 4.16.0-rc6-gcc_ubuntu_le-g99fec39 #1 ... NIP check_version.isra.22+0x118/0x170 Call Trace: __ksymtab_xt_unregister_table+0x58/0xfffffffffffffcb8 [x_tables] (unreliable) resolve_symbol+0xb4/0x150 load_module+0x10e8/0x29a0 SyS_finit_module+0x110/0x140 system_call+0x58/0x6c This happens because since commit 71810db27c1c ("modversions: treat symbol CRCs as 32 bit quantities"), a relocatable kernel encodes and handles symbol CRCs differently from a non-relocatable kernel. Although it's possible we could try and detect this situation and handle it, it's much more robust to simply make the state of CONFIG_RELOCATABLE part of the module vermagic. Fixes: 71810db27c1c ("modversions: treat symbol CRCs as 32 bit quantities") Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> --- arch/powerpc/include/asm/module.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)