diff mbox series

lockdep: Fix static memory detection even more

Message ID ZNep5EcYskP9HtGD@p100
State New
Headers
Series lockdep: Fix static memory detection even more |

Commit Message

Helge Deller Aug. 12, 2023, 3:48 p.m. UTC 
  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@gmx.de>
Fixes: bac59d18c701 ("x86/setup: Fix static memory detection")

---

Comments

kernel test robot Aug. 14, 2023, 1:40 p.m. UTC | #1 
Hello,

kernel test robot noticed "BUG:key#has_not_been_registered" on:

commit: 94d4413e506da48ea18f1cc982202874d35c76b1 ("[PATCH] lockdep: Fix static memory detection even more")
url: https://github.com/intel-lab-lkp/linux/commits/Helge-Deller/lockdep-Fix-static-memory-detection-even-more/20230812-235022
base: https://git.kernel.org/cgit/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/all/ZNep5EcYskP9HtGD@p100/
patch subject: [PATCH] lockdep: Fix static memory detection even more

in testcase: boot

compiler: gcc-12
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

(please refer to attached dmesg/kmsg for entire log/backtrace)



If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202308141646.d3160c77-oliver.sang@intel.com



[    0.575811][    T1] BUG: key b35c282c has not been registered!
[    0.576580][    T1] ------------[ cut here ]------------
[    0.577216][    T1] DEBUG_LOCKS_WARN_ON(1)
[    0.577236][    T1] WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:4888 lockdep_init_map_type+0x155/0x250
[    0.578389][    T1] Modules linked in:
[    0.578856][    T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.5.0-rc4-00587-g94d4413e506d #2
[    0.579899][    T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[    0.581143][    T1] EIP: lockdep_init_map_type+0x155/0x250
[    0.581677][    T1] Code: 70 a2 51 b4 85 db 75 ab e8 78 79 80 00 85 c0 74 a2 8b 0d 98 0a d4 b3 85 c9 75 98 68 59 2a 3b b3 68 8a ce 36 b3 e8 2b 7a f8 ff <0f> 0b 58
 5a eb 83 8d 74 26 00 90 e8 4b 79 80 00 85 c0 0f 84 71 ff
[    0.584028][    T1] EAX: 00000016 EBX: 00000000 ECX: 00000000 EDX: 00000000
[    0.584881][    T1] ESI: b3384ac4 EDI: b35c282c EBP: b67fbe58 ESP: b67fbe44
[    0.585009][    T1] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010286
[    0.585927][    T1] CR0: 80050033 CR2: ff999000 CR3: 03f25000 CR4: 000406d0
[    0.586773][    T1] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[    0.587629][    T1] DR6: fffe0ff0 DR7: 00000400
[    0.588198][    T1] Call Trace:
[    0.588344][    T1]  ? show_regs+0x74/0x80
[    0.588860][    T1]  ? lockdep_init_map_type+0x155/0x250
[    0.589521][    T1]  ? __warn+0x71/0x160
[    0.590011][    T1]  ? lockdep_init_map_type+0x155/0x250
[    0.590684][    T1]  ? report_bug+0x158/0x180
[    0.591242][    T1]  ? exc_overflow+0x40/0x40
[    0.591677][    T1]  ? handle_bug+0x2a/0x50
[    0.592202][    T1]  ? exc_invalid_op+0x25/0x70
[    0.592749][    T1]  ? handle_exception+0x14d/0x14d
[    0.593337][    T1]  ? exc_overflow+0x40/0x40
[    0.593881][    T1]  ? lockdep_init_map_type+0x155/0x250
[    0.594502][    T1]  ? exc_overflow+0x40/0x40
[    0.595011][    T1]  ? lockdep_init_map_type+0x155/0x250
[    0.595653][    T1]  __kernfs_create_file+0x6a/0xe0
[    0.596244][    T1]  sysfs_add_bin_file_mode_ns+0x4e/0xc0
[    0.596891][    T1]  sysfs_create_bin_file+0x5c/0x90
[    0.597496][    T1]  ? nsproxy_cache_init+0x30/0x30
[    0.598090][    T1]  ksysfs_init+0x5c/0x90
[    0.598350][    T1]  ? nsproxy_cache_init+0x30/0x30
[    0.598935][    T1]  do_one_initcall+0x5b/0x2f0
[    0.599480][    T1]  ? parse_args+0x1f3/0x350
[    0.600028][    T1]  do_initcalls+0xe4/0x1e0
[    0.600558][    T1]  kernel_init_freeable+0x171/0x2a0
[    0.601187][    T1]  ? rest_init+0x1b0/0x1b0
[    0.601677][    T1]  kernel_init+0x17/0x1b0
[    0.602193][    T1]  ret_from_fork+0x35/0x50
[    0.602714][    T1]  ? rest_init+0x1b0/0x1b0
[    0.603235][    T1]  ret_from_fork_asm+0x12/0x20
[    0.603812][    T1]  entry_INT80_32+0x10d/0x10d
[    0.604359][    T1] irq event stamp: 7609
[    0.604842][    T1] hardirqs last  enabled at (7609): [<b1114e0e>] __up_console_sem+0x5e/0x80
[    0.605011][    T1] hardirqs last disabled at (7608): [<b1114df5>] __up_console_sem+0x45/0x80
[    0.606012][    T1] softirqs last  enabled at (7260): [<b27a3d23>] __do_softirq+0x2a3/0x3a8
[    0.607008][    T1] softirqs last disabled at (7229): [<b102f2e6>] do_softirq_own_stack+0x26/0x30
[    0.608086][    T1] ---[ end trace 0000000000000000 ]---



The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20230814/202308141646.d3160c77-oliver.sang@intel.com
Peter Zijlstra Aug. 14, 2023, 2:17 p.m. UTC | #2 
On Mon, Aug 14, 2023 at 09:40:42PM +0800, kernel test robot wrote:
> 
> 
> Hello,
> 
> kernel test robot noticed "BUG:key#has_not_been_registered" on:
> 
> commit: 94d4413e506da48ea18f1cc982202874d35c76b1 ("[PATCH] lockdep: Fix static memory detection even more")
> url: https://github.com/intel-lab-lkp/linux/commits/Helge-Deller/lockdep-Fix-static-memory-detection-even-more/20230812-235022
> base: https://git.kernel.org/cgit/linux/kernel/git/akpm/mm.git mm-everything
> patch link: https://lore.kernel.org/all/ZNep5EcYskP9HtGD@p100/
> patch subject: [PATCH] lockdep: Fix static memory detection even more
> 
> in testcase: boot
> 
> compiler: gcc-12
> test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
> 
> (please refer to attached dmesg/kmsg for entire log/backtrace)
> 
> 
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <oliver.sang@intel.com>
> | Closes: https://lore.kernel.org/oe-lkp/202308141646.d3160c77-oliver.sang@intel.com
> 
> 
> 
> [    0.575811][    T1] BUG: key b35c282c has not been registered!
> [    0.576580][    T1] ------------[ cut here ]------------
> [    0.577216][    T1] DEBUG_LOCKS_WARN_ON(1)
> [    0.577236][    T1] WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:4888 lockdep_init_map_type+0x155/0x250
> [    0.578389][    T1] Modules linked in:
> [    0.578856][    T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.5.0-rc4-00587-g94d4413e506d #2

> [    0.595653][    T1]  __kernfs_create_file+0x6a/0xe0
> [    0.596244][    T1]  sysfs_add_bin_file_mode_ns+0x4e/0xc0
> [    0.596891][    T1]  sysfs_create_bin_file+0x5c/0x90
> [    0.597496][    T1]  ? nsproxy_cache_init+0x30/0x30
> [    0.598090][    T1]  ksysfs_init+0x5c/0x90

Problem seems to be __ro_after_init. Your patch only considers
is_kernel_core_data(), which seems to not include these other fancy data
sections we have.
Linux regression tracking (Thorsten Leemhuis) Sept. 21, 2023, 9:03 a.m. UTC | #3 
[TLDR: This mail in primarily relevant for Linux regression tracking. A
change or fix related to the regression discussed in this thread was
posted or applied, but it did not use a Closes: tag to point to the
report, as Linus and the documentation call for. Things happen, no
worries -- but now the regression tracking bot needs to be told manually
about the fix. See link in footer if these mails annoy you.]

On 10.09.23 13:20, Linux regression tracking #adding (Thorsten Leemhuis)
wrote:
> On 03.09.23 21:36, Guenter Roeck wrote:
>>
>> 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.
>>>
>> [...]
>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>> Fixes: bac59d18c701 ("x86/setup: Fix static memory detection")
>>
>> On loongarch, this patch results in the following backtrace.
>> [...]> # first bad commit: [0a6b58c5cd0dfd7961e725212f0fc8dfc5d96195]
> lockdep: fix static memory detection even more
> 
> Thanks for the report. To be sure the issue doesn't fall through the
> cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
> tracking bot:
> 
> #regzbot ^introduced 0a6b58c5cd0dfd7961e725212f0fc8dfc5d961
> #regzbot title lockdep: backtrace on loongarch
> #regzbot ignore-activity

Fix fow in next (as 68ffa230daa0d35b7cce476098433d763d5fd42f)

#regzbot fix: LoongArch: Fix lockdep static memory detection
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.
diff mbox series

Patch

diff --git a/arch/x86/include/asm/sections.h b/arch/x86/include/asm/sections.h
index a6e8373a5170..3fa87e5e11ab 100644
--- a/arch/x86/include/asm/sections.h
+++ b/arch/x86/include/asm/sections.h
@@ -2,8 +2,6 @@ 
 #ifndef _ASM_X86_SECTIONS_H
 #define _ASM_X86_SECTIONS_H

-#define arch_is_kernel_initmem_freed arch_is_kernel_initmem_freed
-
 #include <asm-generic/sections.h>
 #include <asm/extable.h>

@@ -18,20 +16,4 @@  extern char __end_of_kernel_reserve[];

 extern unsigned long _brk_start, _brk_end;

-static inline bool arch_is_kernel_initmem_freed(unsigned long addr)
-{
-	/*
-	 * If _brk_start has not been cleared, brk allocation is incomplete,
-	 * and we can not make assumptions about its use.
-	 */
-	if (_brk_start)
-		return 0;
-
-	/*
-	 * After brk allocation is complete, space between _brk_end and _end
-	 * is available for allocation.
-	 */
-	return addr >= _brk_end && addr < (unsigned long)&_end;
-}
-
 #endif	/* _ASM_X86_SECTIONS_H */
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 111607d91489..957b785d0b59 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -819,34 +819,20 @@  static int very_verbose(struct lock_class *class)
  * Is this the address of a static object:
  */
 #ifdef __KERNEL__
-/*
- * Check if an address is part of freed initmem. After initmem is freed,
- * memory can be allocated from it, and such allocations would then have
- * addresses within the range [_stext, _end].
- */
-#ifndef arch_is_kernel_initmem_freed
-static int arch_is_kernel_initmem_freed(unsigned long addr)
-{
-	if (system_state < SYSTEM_FREEING_INITMEM)
-		return 0;
-
-	return init_section_contains((void *)addr, 1);
-}
-#endif
-
 static int static_obj(const void *obj)
 {
-	unsigned long start = (unsigned long) &_stext,
-		      end   = (unsigned long) &_end,
-		      addr  = (unsigned long) obj;
+	unsigned long addr = (unsigned long) obj;

-	if (arch_is_kernel_initmem_freed(addr))
-		return 0;
+	if (is_kernel_core_data(addr))
+		return 1;

 	/*
-	 * static variable?
+	 * in initdata section and used during bootup only?
+	 * NOTE: On some platforms the initdata section is
+	 * outside of the _stext ... _end range.
 	 */
-	if ((addr >= start) && (addr < end))
+	if (system_state < SYSTEM_FREEING_INITMEM &&
+		init_section_contains((void *)addr, 1))
 		return 1;

 	/*