linux-kernel - Re: [PATCH] LoongArch: Fix lockdep static memory detection

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 15 Sep 2023 07:07:48 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Helge Deller <deller@....de>, Huacai Chen <chenhuacai@...nel.org>
Cc:     WANG Xuerui <kernel@...0n.name>, loongarch@...ts.linux.dev,
        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

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 <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.
> 

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