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]
Message-ID: <4tilpszhxhvzvszed2a3ubziloabflrkan54wxvfgodp73yso4@fgzapeplkp2s>
Date: Fri, 28 Mar 2025 11:25:19 +0200
From: "Kirill A. Shutemov" <kirill@...temov.name>
To: "Aithal, Srikanth" <sraithal@....com>
Cc: Steven Rostedt <rostedt@...dmis.org>, 
	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>, Tom Lendacky <thomas.lendacky@....com>, 
	Jason Baron <jbaron@...mai.com>, Peter Zijlstra <peterz@...radead.org>, 
	Josh Poimboeuf <jpoimboe@...nel.org>, Ard Biesheuvel <ardb@...nel.org>, 
	Linux-Next Mailing List <linux-next@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>, 
	"Roth, Michael" <Michael.Roth@....com>
Subject: Re: linux-next regression: SNP Guest boot hangs with certain cpu/mem
 config combination

On Fri, Mar 28, 2025 at 02:49:27PM +0530, Aithal, Srikanth wrote:
> On 3/28/2025 2:39 PM, Kirill A. Shutemov wrote:
> > On Fri, Mar 28, 2025 at 10:28:19AM +0200, Kirill A. Shutemov wrote:
> > > On Thu, Mar 27, 2025 at 07:39:22PM +0200, Kirill A. Shutemov wrote:
> > > > On Thu, Mar 27, 2025 at 11:02:24AM -0400, Steven Rostedt wrote:
> > > > > On Thu, 27 Mar 2025 16:43:43 +0200
> > > > > "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com> wrote:
> > > > > 
> > > > > > > > The only option I see so far is to drop static branch from this path.
> > > > > > > > 
> > > > > > > > But I am not sure if it the only case were we use static branch from CPU
> > > > > > > > hotplug callbacks.
> > > > > > > > 
> > > > > > > > Any other ideas?
> > > > > > > 
> > > > > > > 
> > > > > > > Hmmm, didn't take too close a look here, but there is the
> > > > > > > static_key_slow_dec_cpuslocked() variant, would that work here? Is the issue
> > > > > > > the caller may or may not have the cpu_hotplug lock?
> > > > > > 
> > > > > > Yes. This is generic page alloc path and it can be called with and without
> > > > > > the lock.
> > > > > 
> > > > > Note, it's not the static_branch that is an issue, it's enabling/disabling
> > > > > the static branch that is. Changing a static branch takes a bit of work as
> > > > > it does modify the kernel text.
> > > > > 
> > > > > Is it possible to delay the update via a workqueue?
> > > > 
> > > > Ah. Good point. Should work. I'll give it try.
> > > 
> > > The patch below fixes problem for me.
> > 
> > Ah. No, it won't work. We can get there before workqueues are initialized:
> > mm_core_init() is called before workqueue_init_early().
> > 
> > We cannot queue a work. :/
> > 
> > Steven, any other ideas?
> > 
> 
> I have booted the guest with different memory and CPU combinations and have
> not seen any failures with the fix so far. Are there any other scenarios
> that could trigger the above case? Please let me know.

I saw this:

[    1.828379] BUG: kernel NULL pointer dereference, address: 00000000000001c1
[    1.833122] #PF: supervisor read access in kernel mode
[    1.834907] #PF: error_code(0x0000) - not-present page
[    1.836681] PGD 0
[    1.837386] Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
[    1.839051] CPU: 0 UID: 0 PID: 0 Comm: swapper Tainted: G S                 6.14.0-rc5-00015-ga250fa517231-dirty #2266
[    1.842700] Tainted: [S]=CPU_OUT_OF_SPEC
[    1.844036] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown 02/02/2022
[    1.846790] RIP: 0010:__queue_work+0x3f/0x610
[    1.848330] Code: d3 49 89 f5 41 89 fc 83 3d 5d 51 cb 01 00 74 1a 65 8b 05 04 db 94 4a 85 c0 75 0f 65 8b 05 d1 d7 94 4a 85 c0 0f 85 59 03 00 00 <41> 0f b7 85 c1 01 00 00 a9 80 01 00 00 74 57 65 8b 05 23 ec 97 4a
[    1.854803] RSP: 0000:ffffffffb7003c08 EFLAGS: 00010046
[    1.856614] RAX: 0000000000000000 RBX: ff3d51307fb54488 RCX: 0000000000000000
[    1.859083] RDX: ff3d51307fb54488 RSI: 0000000000000000 RDI: 0000000000000200
[    1.861534] RBP: ffffffffb7003e18 R08: 0000000000000001 R09: 0000000000000001
[    1.864018] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000200
[    1.866480] R13: 0000000000000000 R14: 0000000000000092 R15: ff3d51307fb544d8
[    1.868920] FS:  0000000000000000(0000) GS:ff3d513075000000(0000) knlGS:0000000000000000
[    1.871721] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    1.873710] CR2: 00000000000001c1 CR3: 0000000004a5c001 CR4: 00000000000210f0
[    1.876194] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[    1.878629] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400
[    1.881105] Call Trace:
[    1.881936]  <TASK>
[    1.882642]  ? __die+0xda/0x120
[    1.883715]  ? page_fault_oops+0x330/0x3c0
[    1.885144]  ? exc_page_fault+0x43/0x100
[    1.886483]  ? exc_page_fault+0x62/0x100
[    1.887825]  ? __add_to_free_list+0xc0/0x160
[    1.891278]  ? asm_exc_page_fault+0x22/0x30
[    1.892706]  ? __queue_work+0x3f/0x610
[    1.894000]  ? __free_pages_ok+0x2b0/0x530
[    1.895416]  queue_work_on+0xb3/0x110
[    1.896673]  ? __accept_page+0x10b/0x170
[    1.898042]  cond_accept_memory+0x1dc/0x200
[    1.899496]  get_page_from_freelist+0x1ad/0x1380
[    1.901086]  __alloc_frozen_pages_noprof+0xff/0x280
[    1.902760]  alloc_pages_mpol+0x111/0x1c0
[    1.904159]  alloc_pages_noprof+0x7c/0xf0
[    1.905535]  __p4d_alloc+0x31/0x110
[    1.906729]  preallocate_vmalloc_pages+0xdf/0x160
[    1.908397]  mm_core_init+0x25/0x60
[    1.909590]  start_kernel+0x16a/0x400
[    1.910850]  x86_64_start_reservations+0x20/0x20
[    1.912454]  x86_64_start_kernel+0xaa/0xb0
[    1.913856]  common_startup_64+0x13e/0x140
[    1.915269]  </TASK>
[    1.916023] CR2: 00000000000001c1
[    1.917183] ---[ end trace 0000000000000000 ]---
[    1.918764] RIP: 0010:__queue_work+0x3f/0x610

This patch fixes it:

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 9540b41894da..ea5f7e0b675d 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -964,6 +964,9 @@ struct zone {
 #ifdef CONFIG_UNACCEPTED_MEMORY
 	/* Pages to be accepted. All pages on the list are MAX_PAGE_ORDER */
 	struct list_head	unaccepted_pages;
+
+	/* To be called once last page in the zone is accepted */
+	struct work_struct	unaccepted_cleanup;
 #endif
 
 	/* zone flags, see below */
diff --git a/mm/internal.h b/mm/internal.h
index 109ef30fee11..f2e6d42af6eb 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1516,6 +1516,7 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
 
 #ifdef CONFIG_UNACCEPTED_MEMORY
 void accept_page(struct page *page);
+void unaccepted_cleanup_work(struct work_struct *work);
 #else /* CONFIG_UNACCEPTED_MEMORY */
 static inline void accept_page(struct page *page)
 {
diff --git a/mm/mm_init.c b/mm/mm_init.c
index 2630cc30147e..d5a51f65dc4d 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -1404,6 +1404,7 @@ static void __meminit zone_init_free_lists(struct zone *zone)
 
 #ifdef CONFIG_UNACCEPTED_MEMORY
 	INIT_LIST_HEAD(&zone->unaccepted_pages);
+	INIT_WORK(&zone->unaccepted_cleanup, unaccepted_cleanup_work);
 #endif
 }
 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4fe93029bcb6..1b776d0c7784 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6921,6 +6921,11 @@ static DEFINE_STATIC_KEY_FALSE(zones_with_unaccepted_pages);
 
 static bool lazy_accept = true;
 
+void unaccepted_cleanup_work(struct work_struct *work)
+{
+	static_branch_dec(&zones_with_unaccepted_pages);
+}
+
 static int __init accept_memory_parse(char *p)
 {
 	if (!strcmp(p, "lazy")) {
@@ -6959,8 +6964,27 @@ static void __accept_page(struct zone *zone, unsigned long *flags,
 
 	__free_pages_ok(page, MAX_PAGE_ORDER, FPI_TO_TAIL);
 
-	if (last)
-		static_branch_dec(&zones_with_unaccepted_pages);
+	if (last) {
+		/*
+		 * There are two corner cases:
+		 *
+		 * - If allocation occurs during the CPU bring up,
+		 *   static_branch_dec() cannot be used directly as
+		 *   it causes a deadlock on cpu_hotplug_lock.
+		 *
+		 *   Instead, use schedule_work() to prevent deadlock.
+		 *
+		 * - If allocation occurs before workqueues are initialized,
+		 *   static_branch_dec() should be directly.
+		 *
+		 *   Workqueues are initialized before CPU bring up, so this
+		 *   will not conflict with the first scenario.
+		 */
+		if (system_wq)
+			schedule_work(&zone->unaccepted_cleanup);
+		else
+			unaccepted_cleanup_work(&zone->unaccepted_cleanup);
+	}
 }
 
 void accept_page(struct page *page)
-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ