Re: powerpc/modules: Fix crashes by adding CONFIG_RELOCATABLE to vermagic

[Ard Biesheuvel]

On 16 April 2018 at 16:10, Michael Ellerman  wrote:
> Ard Biesheuvel  writes:
>
>> On 11 April 2018 at 16:49, Michael Ellerman
>>  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 
 0xd00018d66cef
   Faulting instruction address: 0xc21ddd08
   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/0xfcb8 [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 
>>>
>>> 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.

Re: powerpc/modules: Fix crashes by adding CONFIG_RELOCATABLE to vermagic

[Michael Ellerman]

Ard Biesheuvel  writes:

> On 11 April 2018 at 16:49, Michael Ellerman
>  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 
>>> 0xd00018d66cef
>>>   Faulting instruction address: 0xc21ddd08
>>>   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/0xfcb8 [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 
>>
>> 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

Re: powerpc/modules: Fix crashes by adding CONFIG_RELOCATABLE to vermagic

[Ard Biesheuvel]

On 11 April 2018 at 16:49, Michael Ellerman
 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 
>> 0xd00018d66cef
>>   Faulting instruction address: 0xc21ddd08
>>   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/0xfcb8 [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 
>
> 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.

Re: powerpc/modules: Fix crashes by adding CONFIG_RELOCATABLE to vermagic

[Michael Ellerman]

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 
> 0xd00018d66cef
>   Faulting instruction address: 0xc21ddd08
>   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/0xfcb8 [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 

Applied to powerpc fixes.

https://git.kernel.org/powerpc/c/73aca179d78eaa11604ba0783a6d8b

cheers