| 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
| ||
|
Message-ID: <081537c0-0d74-0242-451a-e6bd6f71cdd9@roeck-us.net>
Date: Wed, 6 Sep 2023 01:11:38 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Helge Deller <deller@....de>,
Linus Torvalds <torvalds@...ux-foundation.org>,
linux-kernel@...r.kernel.org, linux-fbdev@...r.kernel.org,
dri-devel@...ts.freedesktop.org
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
linux-parisc@...r.kernel.org, Borislav Petkov <bp@...e.de>,
Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH] lockdep: Fix static memory detection even more
On 9/6/23 00:18, Helge Deller wrote:
> * Guenter Roeck <linux@...ck-us.net>:
>> On 9/3/23 14:11, Helge Deller wrote:
>>> * Guenter Roeck <linux@...ck-us.net>:
>>>> Hi,
>>>>
>>>> On Sat, Aug 12, 2023 at 05:48:52PM +0200, Helge Deller wrote:
>>>>> On the parisc architecture, lockdep reports for all static objects which
>>>>> are in the __initdata section (e.g. "setup_done" in devtmpfs,
>>>>> "kthreadd_done" in init/main.c) this warning:
>>>>>
>>>>> INFO: trying to register non-static key.
>>>>>
>>>>> The warning itself is wrong, because those objects are in the __initdata
>>>>> section, but the section itself is on parisc outside of range from
>>>>> _stext to _end, which is why the static_obj() functions returns a wrong
>>>>> answer.
>>>>>
>>>>> While fixing this issue, I noticed that the whole existing check can
>>>>> be simplified a lot.
>>>>> Instead of checking against the _stext and _end symbols (which include
>>>>> code areas too) just check for the .data and .bss segments (since we check a
>>>>> data object). This can be done with the existing is_kernel_core_data()
>>>>> macro.
>>>>>
>>>>> In addition objects in the __initdata section can be checked with
>>>>> init_section_contains().
>>>>>
>>>>> This partly reverts and simplifies commit bac59d18c701 ("x86/setup: Fix static
>>>>> memory detection").
>>>>>
>>>>> Tested on x86-64 and parisc.
>>>>>
>>>>> Signed-off-by: Helge Deller <deller@....de>
>>>>> Fixes: bac59d18c701 ("x86/setup: Fix static memory detection")
>>>>
>>>> On loongarch, this patch results in the following backtrace.
>>>>
>>>> EFI stub: Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path
>>>> EFI stub: Exiting boot services
>>>> [ 0.000000] INFO: trying to register non-static key.
>>>> [ 0.000000] The code is fine but needs lockdep annotation, or maybe
>>>> [ 0.000000] you didn't initialize this object before use?
>>>> [ 0.000000] turning off the locking correctness validator.
>>>> [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.5.0+ #1
>>>> [ 0.000000] Stack : 0000000000000000 0000000000000000 9000000000223d6c 9000000001df0000
>>>> [ 0.000000] 9000000001df39a0 9000000001df39a8 0000000000000000 0000000000000000
>>>> [ 0.000000] 9000000001df39a8 0000000000000001 0000000000000000 900000000154b910
>>>> [ 0.000000] fffffffffffffffe 9000000001df39a8 0000000000000000 0000000000000000
>>>> [ 0.000000] 0000000000000001 0000000000000003 0000000000000010 0000000000000030
>>>> [ 0.000000] 0000000000000063 0000000000000001 0000000000000000 0000000000000000
>>>> [ 0.000000] 0000000000000000 0000000000000000 9000000001c60650 9000000001e12000
>>>> [ 0.000000] 0000000000000000 9000000001560bc0 0000000000000000 9000000002ee6000
>>>> [ 0.000000] 0000000000000000 0000000000000000 9000000000223d84 0000000000000000
>>>> [ 0.000000] 00000000000000b0 0000000000000004 0000000000000000 0000000000000800
>>>> [ 0.000000] ...
>>>> [ 0.000000] Call Trace:
>>>> [ 0.000000] [<9000000000223d84>] show_stack+0x5c/0x180
>>>> [ 0.000000] [<900000000153e0b4>] dump_stack_lvl+0x88/0xd0
>>>> [ 0.000000] [<90000000002bc548>] register_lock_class+0x768/0x770
>>>> [ 0.000000] [<90000000002bc710>] __lock_acquire+0xb0/0x2a18
>>>> [ 0.000000] [<90000000002bba1c>] lock_acquire+0x11c/0x328
>>>> [ 0.000000] [<9000000000b34a60>] __debug_object_init+0x60/0x244
>>>> [ 0.000000] [<9000000000337f94>] init_cgroup_housekeeping+0xe8/0x144
>>>> [ 0.000000] [<900000000033e364>] init_cgroup_root+0x38/0xa0
>>>> [ 0.000000] [<90000000017801ac>] cgroup_init_early+0x44/0x16c
>>>> [ 0.000000] [<9000000001770758>] start_kernel+0x50/0x624
>>>> [ 0.000000] [<90000000015410b4>] kernel_entry+0xb4/0xc4
>>>>
>>>> Reverting it fixes the problem. Bisect log attached.
>>>>
>>>> This is also seen in v6.5.y and v6.4.y since the patch has been applied
>>>> to those branches.
>>>
>>> Does this happens with CONFIG_SMP=n ?
>>> If so, I think the untested patch below might fix the issue.
>>>
>>
>> No, this is loongarch:defconfig with various debug options enabled.
>> That has CONFIG_SMP=y.
>
> Could you apply below patch and verify with the contents of the
> System.map file where the lock is located ?
>
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index e85b5ad3e206..db0a301f9740 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -969,7 +969,7 @@ static bool assign_lock_key(struct lockdep_map *lock)
> else {
> /* Debug-check: all keys must be persistent! */
> debug_locks_off();
> - pr_err("INFO: trying to register non-static key.\n");
> + pr_err("INFO: trying to register non-static key at %08lx.\n", addr);
> pr_err("The code is fine but needs lockdep annotation, or maybe\n");
> pr_err("you didn't initialize this object before use?\n");
> pr_err("turning off the locking correctness validator.\n");
90000000015602d0 D __la_abs_end
...
90000000016815c0 d fill_pool_map.3 <--- lock pointer
...
9000000001770000 T __init_begin
Guenter
Powered by blists - more mailing lists