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,
- PLT symbols behind the read-only section, and
- *(.la_abs) into the data section.
Signed-off-by: Helge Deller <[email protected]>
Reported-by: Guenter Roeck <[email protected]>
Fixes: 0a6b58c5cd0d ("lockdep: fix static memory detection even more")
Cc: stable <[email protected]> # 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)
}
On Tue, Sep 12, 2023 at 09:18:37PM +0200, Helge Deller 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,
> - PLT symbols behind the read-only section, and
> - *(.la_abs) into the data section.
>
> Signed-off-by: Helge Deller <[email protected]>
> Reported-by: Guenter Roeck <[email protected]>
> Fixes: 0a6b58c5cd0d ("lockdep: fix static memory detection even more")
> Cc: stable <[email protected]> # v6.4+
Tested-by: Guenter Roeck <[email protected]>
>
> 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)
> }
PING to Loongarch maintainers!
Without this patch, lockdep is broken on LoongArch on kernel v6.1 and above.
(patch below wrongly mentions kernel 6.4, but actually it needs backport to v6.1 too).
Helge
On 9/12/23 22:31, Guenter Roeck wrote:
> On Tue, Sep 12, 2023 at 09:18:37PM +0200, Helge Deller 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,
>> - PLT symbols behind the read-only section, and
>> - *(.la_abs) into the data section.
>>
>> Signed-off-by: Helge Deller <[email protected]>
>> Reported-by: Guenter Roeck <[email protected]>
>> Fixes: 0a6b58c5cd0d ("lockdep: fix static memory detection even more")
>> Cc: stable <[email protected]> # v6.4+
>
> Tested-by: Guenter Roeck <[email protected]>
>
>>
>> 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)
>> }
Hi Helge,
On Wed, Sep 13, 2023 at 3:18 AM Helge Deller <[email protected]> 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.
Huacai
> - PLT symbols behind the read-only section, and
> - *(.la_abs) into the data section.
>
> Signed-off-by: Helge Deller <[email protected]>
> Reported-by: Guenter Roeck <[email protected]>
> Fixes: 0a6b58c5cd0d ("lockdep: fix static memory detection even more")
> Cc: stable <[email protected]> # 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)
> }
On Fri, Sep 15, 2023 at 4:16 PM Helge Deller <[email protected]> wrote:
>
> On 9/15/23 05:22, Huacai Chen wrote:
> > Hi Helge,
> >
> > On Wed, Sep 13, 2023 at 3:18 AM Helge Deller <[email protected]> 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.
Huacai
>
> Helge
>
>
> >> - PLT symbols behind the read-only section, and
> >> - *(.la_abs) into the data section.
> >>
> >> Signed-off-by: Helge Deller <[email protected]>
> >> Reported-by: Guenter Roeck <[email protected]>
> >> Fixes: 0a6b58c5cd0d ("lockdep: fix static memory detection even more")
> >> Cc: stable <[email protected]> # 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)
> >> }
>
On 9/15/23 11:23, Huacai Chen wrote:
> On Fri, Sep 15, 2023 at 4:16 PM Helge Deller <[email protected]> wrote:
>>
>> On 9/15/23 05:22, Huacai Chen wrote:
>>> Hi Helge,
>>>
>>> On Wed, Sep 13, 2023 at 3:18 AM Helge Deller <[email protected]> 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 <[email protected]>
>>>> Reported-by: Guenter Roeck <[email protected]>
>>>> Fixes: 0a6b58c5cd0d ("lockdep: fix static memory detection even more")
>>>> Cc: stable <[email protected]> # 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)
>>>> }
>>
On 9/15/23 05:22, Huacai Chen wrote:
> Hi Helge,
>
> On Wed, Sep 13, 2023 at 3:18 AM Helge Deller <[email protected]> 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.
Helge
>> - PLT symbols behind the read-only section, and
>> - *(.la_abs) into the data section.
>>
>> Signed-off-by: Helge Deller <[email protected]>
>> Reported-by: Guenter Roeck <[email protected]>
>> Fixes: 0a6b58c5cd0d ("lockdep: fix static memory detection even more")
>> Cc: stable <[email protected]> # 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)
>> }
On Fri, Sep 15, 2023 at 10:19 PM WANG Xuerui <[email protected]> wrote:
>
> Hi,
>
> On 9/15/23 22:07, Guenter Roeck wrote:
> > Hi Helge,
> >
> > On 9/15/23 03:10, Helge Deller wrote:
> >> On 9/15/23 11:23, Huacai Chen wrote:
> >>> On Fri, Sep 15, 2023 at 4:16 PM Helge Deller <[email protected]> wrote:
> >>>>
> >>>> On 9/15/23 05:22, Huacai Chen wrote:
> >>>>> Hi Helge,
> >>>>>
> >>>>> On Wed, Sep 13, 2023 at 3:18 AM Helge Deller <[email protected]> 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.
> >>
> >
> > Just my $0.02 .. it might make sense to concentrate on the minimum to
> > get the immediate
> > problem fixed. Loongarch maintainers can then decide at their own pace
> > if they want
> > to apply any of the other changes you suggested. After all, unless I
> > am missing
> > something, those additional changes are not really needed in stable
> > releases.
>
> Sorry for coming late, but as reviewer of arch/loongarch, I'd agree with
> Guenter and Helge here: let's fix the immediate problem and investigate
> the rest later -- it's not like the problems are *definitely* orthogonal
> in this case, and at least *some* progress would be appreciated.
>
> I'll try to reproduce the problem and test the fix during the weekend,
> so hopefully Huacai can get the fix in before -rc2 or -rc3. Thanks for
> the attention and fix.
If all changes are OK, I have no objection to putting them in a single patch.
Huacai
>
> --
> WANG "xen0n" Xuerui
>
> Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
>
>
Hi,
On 9/15/23 22:07, Guenter Roeck wrote:
> Hi Helge,
>
> On 9/15/23 03:10, Helge Deller wrote:
>> On 9/15/23 11:23, Huacai Chen wrote:
>>> On Fri, Sep 15, 2023 at 4:16 PM Helge Deller <[email protected]> wrote:
>>>>
>>>> On 9/15/23 05:22, Huacai Chen wrote:
>>>>> Hi Helge,
>>>>>
>>>>> On Wed, Sep 13, 2023 at 3:18 AM Helge Deller <[email protected]> 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.
>>
>
> Just my $0.02 .. it might make sense to concentrate on the minimum to
> get the immediate
> problem fixed. Loongarch maintainers can then decide at their own pace
> if they want
> to apply any of the other changes you suggested. After all, unless I
> am missing
> something, those additional changes are not really needed in stable
> releases.
Sorry for coming late, but as reviewer of arch/loongarch, I'd agree with
Guenter and Helge here: let's fix the immediate problem and investigate
the rest later -- it's not like the problems are *definitely* orthogonal
in this case, and at least *some* progress would be appreciated.
I'll try to reproduce the problem and test the fix during the weekend,
so hopefully Huacai can get the fix in before -rc2 or -rc3. Thanks for
the attention and fix.
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
Hi Helge,
On 9/15/23 03:10, Helge Deller wrote:
> On 9/15/23 11:23, Huacai Chen wrote:
>> On Fri, Sep 15, 2023 at 4:16 PM Helge Deller <[email protected]> wrote:
>>>
>>> On 9/15/23 05:22, Huacai Chen wrote:
>>>> Hi Helge,
>>>>
>>>> On Wed, Sep 13, 2023 at 3:18 AM Helge Deller <[email protected]> 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.
>
Just my $0.02 .. it might make sense to concentrate on the minimum to get the immediate
problem fixed. Loongarch maintainers can then decide at their own pace if they want
to apply any of the other changes you suggested. After all, unless I am missing
something, those additional changes are not really needed in stable releases.
Thanks,
Guenter