| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 15 Sep 2023 12:10:23 +0200
From: Helge Deller <deller@....de>
To: Huacai Chen <chenhuacai@...nel.org>
Cc: WANG Xuerui <kernel@...0n.name>, loongarch@...ts.linux.dev,
Guenter Roeck <linux@...ck-us.net>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] LoongArch: Fix lockdep static memory detection
On 9/15/23 11:23, Huacai Chen wrote:
> On Fri, Sep 15, 2023 at 4:16 PM Helge Deller <deller@....de> wrote:
>>
>> On 9/15/23 05:22, Huacai Chen wrote:
>>> Hi Helge,
>>>
>>> On Wed, Sep 13, 2023 at 3:18 AM Helge Deller <deller@....de> wrote:
>>>>
>>>> Since commit 0a6b58c5cd0d ("lockdep: fix static memory detection even
>>>> more") the lockdep code uses is_kernel_core_data(), is_kernel_rodata()
>>>> and init_section_contains() to verify if a lock is located inside a
>>>> kernel static data section.
>>>>
>>>> This change triggers a failure on LoongArch, for which the vmlinux.lds.S
>>>> script misses to put the locks (as part of in the .data.rel symbols)
>>>> into the Linux data section.
>>>> This patch fixes the lockdep problem by moving *(.data.rel*) symbols
>>>> into the kernel data section (from _sdata to _edata).
>>>>
>>>> Additionally, move other wrongly assigned symbols too:
>>>> - altinstructions into the _initdata section,
>>
>>> I think altinstructions cannot be put into _initdata because it will
>>> be used by modules.
>>
>> No.
>> arch/loongarch/kernel/vmlinux.lds.S is used for the static parts of the kernel
>> and altinstructions are replaced before modules are loaded.
>> For altinstructions in modules the linker script scripts/module.lds.S is used.
> OK, then what about .got/.plt? It seems arm64 also doesn't put them in
> the data section.
arm64 seems to throw away all plt entries already at link time (and just keeps
the got.plt in the read-only data section).
It even checks at link time, that there are no plt entries in the binary:
ASSERT(SIZEOF(.plt) == 0, "Unexpected run-time procedure linkages detected!")
I don't know for loongarch, but if you need the plt entries for loongarch, it's
safest & best to put them into the read-only data section too, which is what my patch does.
Up to now, you have them completely outside of code & data sections.
In the end you need to decide for your platform. My patch is a suggestion, which I think
is correct (untested by me, but Guenter replied he tested it).
But to fix the lockdep problem at minimum the move of the .data.rel section
is needed.
Helge
>>
>>>> - PLT symbols behind the read-only section, and
>>>> - *(.la_abs) into the data section.
>>>>
>>>> Signed-off-by: Helge Deller <deller@....de>
>>>> Reported-by: Guenter Roeck <linux@...ck-us.net>
>>>> Fixes: 0a6b58c5cd0d ("lockdep: fix static memory detection even more")
>>>> Cc: stable <stable@...nel.org> # v6.4+
>>>>
>>>> diff --git a/arch/loongarch/kernel/vmlinux.lds.S b/arch/loongarch/kernel/vmlinux.lds.S
>>>> index b1686afcf876..bb2ec86f37a8 100644
>>>> --- a/arch/loongarch/kernel/vmlinux.lds.S
>>>> +++ b/arch/loongarch/kernel/vmlinux.lds.S
>>>> @@ -53,33 +53,6 @@ SECTIONS
>>>> . = ALIGN(PECOFF_SEGMENT_ALIGN);
>>>> _etext = .;
>>>>
>>>> - /*
>>>> - * struct alt_inst entries. From the header (alternative.h):
>>>> - * "Alternative instructions for different CPU types or capabilities"
>>>> - * Think locking instructions on spinlocks.
>>>> - */
>>>> - . = ALIGN(4);
>>>> - .altinstructions : AT(ADDR(.altinstructions) - LOAD_OFFSET) {
>>>> - __alt_instructions = .;
>>>> - *(.altinstructions)
>>>> - __alt_instructions_end = .;
>>>> - }
>>>> -
>>>> -#ifdef CONFIG_RELOCATABLE
>>>> - . = ALIGN(8);
>>>> - .la_abs : AT(ADDR(.la_abs) - LOAD_OFFSET) {
>>>> - __la_abs_begin = .;
>>>> - *(.la_abs)
>>>> - __la_abs_end = .;
>>>> - }
>>>> -#endif
>>>> -
>>>> - .got : ALIGN(16) { *(.got) }
>>>> - .plt : ALIGN(16) { *(.plt) }
>>>> - .got.plt : ALIGN(16) { *(.got.plt) }
>>>> -
>>>> - .data.rel : { *(.data.rel*) }
>>>> -
>>>> . = ALIGN(PECOFF_SEGMENT_ALIGN);
>>>> __init_begin = .;
>>>> __inittext_begin = .;
>>>> @@ -94,6 +67,18 @@ SECTIONS
>>>>
>>>> __initdata_begin = .;
>>>>
>>>> + /*
>>>> + * struct alt_inst entries. From the header (alternative.h):
>>>> + * "Alternative instructions for different CPU types or capabilities"
>>>> + * Think locking instructions on spinlocks.
>>>> + */
>>>> + . = ALIGN(4);
>>>> + .altinstructions : AT(ADDR(.altinstructions) - LOAD_OFFSET) {
>>>> + __alt_instructions = .;
>>>> + *(.altinstructions)
>>>> + __alt_instructions_end = .;
>>>> + }
>>>> +
>>>> INIT_DATA_SECTION(16)
>>>> .exit.data : {
>>>> EXIT_DATA
>>>> @@ -113,6 +98,11 @@ SECTIONS
>>>>
>>>> _sdata = .;
>>>> RO_DATA(4096)
>>>> +
>>>> + .got : ALIGN(16) { *(.got) }
>>>> + .plt : ALIGN(16) { *(.plt) }
>>>> + .got.plt : ALIGN(16) { *(.got.plt) }
>>>> +
>>>> RW_DATA(1 << CONFIG_L1_CACHE_SHIFT, PAGE_SIZE, THREAD_SIZE)
>>>>
>>>> .rela.dyn : ALIGN(8) {
>>>> @@ -121,6 +111,17 @@ SECTIONS
>>>> __rela_dyn_end = .;
>>>> }
>>>>
>>>> + .data.rel : { *(.data.rel*) }
>>>> +
>>>> +#ifdef CONFIG_RELOCATABLE
>>>> + . = ALIGN(8);
>>>> + .la_abs : AT(ADDR(.la_abs) - LOAD_OFFSET) {
>>>> + __la_abs_begin = .;
>>>> + *(.la_abs)
>>>> + __la_abs_end = .;
>>>> + }
>>>> +#endif
>>>> +
>>>> .sdata : {
>>>> *(.sdata)
>>>> }
>>
Powered by blists - more mailing lists