From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 22C8F429CDF for ; Thu, 16 Jul 2026 17:55:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784224508; cv=none; b=udSYgFFcNWKexcUau7Fg7crFfEQ+qS+zV7/Hbn+EaCDDMnhdCf8VbGbPL0fuSxOlVbQnXNUDEFB84fNjfUuNYtdrB3sVklU1juvEqR0kNdMzCMfiixIoJEpxtW/6m2EMp0VK6U0H7tnh1Mj0SUooQP1TOoAXZnk/Bw1oicL6XwI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784224508; c=relaxed/simple; bh=l+LPDYkczMDccuwzQeNL214gJDVtllNq+z4NyVol+/M=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=BFNd3ydJp/rNw1hrou4lLVYVawr0YQ98tp7DmEbvjObhOxqTILHSYXNvoMIxaF/iFFwO2dCTnkbgswqTzIyf4UAG8Bxe5LQjz/uqyhqL/P+OKmfZOZMUZFXI00lcBVuhjpB27XTGKNGw4E920Oga+uOLuJASMndyGxfYhiyHylA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=hpR0d/sP; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="hpR0d/sP" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 28C4D1F000E9; Thu, 16 Jul 2026 17:55:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1784224506; bh=NAUFRQ/cidMd8yqzkIKc7eM2o6tRmex8bNnZtM1uXTM=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=hpR0d/sPHSm+332aNzNAAlS8jkrZ6LwMYrEJCUIv6LdCjYSpq7+4KpTgseRuDoljz GXm+MiGye23OSCrDh2OHvFgIHf1n2bQFUWJ5YtcldlprOGcz1w133yvgqko4YTpFyL ZhMIJ/dw5ClqqFFt+dcrru6VtWMfymZd1Vy6uTnZCNhJJiP5Ja3vOeQyP41+e4sQWk hUZNZTWeg5CuG9LwUqp6QDGj21VYStOkqenwG2Gd+1Bx/XamB7i2wP9h1JT7TLySAu v9DDDMTCVQ+shBZHNcK8ES/kRokkIW0Xt1ip5frFpBIwIC5dIPpaBp8gGEunBimOFG fqiLMWh8u6oLw== Date: Thu, 16 Jul 2026 18:54:55 +0100 From: "Lorenzo Stoakes (ARM)" To: "Denis V. Lunev" Cc: "Denis V. Lunev" , Dave Hansen , Andy Lutomirski , Peter Zijlstra , x86@kernel.org, Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , "Mike Rapoport (Microsoft)" , Juergen Gross , Kiryl Shutsemau , linux-kernel@vger.kernel.org Subject: Re: [PATCH] x86/mm/pat: take cpa_lock around large-page collapse Message-ID: References: <20260626163213.2284080-1-den@openvz.org> <3ddb7f76-db6e-4453-b5db-fb13d8fc1f2d@virtuozzo.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <3ddb7f76-db6e-4453-b5db-fb13d8fc1f2d@virtuozzo.com> On Wed, Jul 15, 2026 at 02:20:59PM +0200, Denis V. Lunev wrote: > On 6/26/26 18:32, Denis V. Lunev wrote: > > This email originated from an IP that might not be authorized by the domain it was sent from. > > Do not click links or open attachments unless it is an email you expected to receive. > > Loading and unloading modules concurrently on several CPUs on a KASAN > > build, with a short delay injected at the CPA page-table lookup to > > widen the window, faults within minutes: > > > > BUG: KASAN: use-after-free in __change_page_attr+0x7cc/0x7e0 > > Write of size 8 at addr ffff888181139718 by task modprobe > > ... > > The buggy address belongs to the physical page: > > pfn:0x181139 ... page_type: f2(table) > > > > cpa_collapse_large_pages() rebuilds a leaf PMD from its 4K PTEs and > > frees the old PTE-table pages, while __change_page_attr() fetches a > > PTE pointer from a lockless lookup_address_in_pgd_attr() and writes > > it with set_pte_atomic() only later. When module text is served from > > a shared large ROX mapping the two run on the same PMD: > > > > CPU A (module load) CPU B (module finalize) > > ------------------- ----------------------- > > execmem_make_temp_rw > > set_memory_nx > > __change_page_attr > > split 2M -> 4K table P > > kpte = &P[i] (lockless) > > execmem_restore_rox > > set_memory_rox (CPA_COLLAPSE) > > cpa_collapse_large_pages > > rebuild leaf PMD > > flush_tlb_all > > pagetable_free(P) > > set_pte_atomic(kpte, ...) > > -> writes into freed P > > > > P is a page-table page (page_type: table), reused at once, so the > > write corrupts whatever got the page next: a bad-pte or bad-page > > splat, or a fatal fault once P has been turned into read-only text. > > > > The flush_tlb_all() before the free does not close this: its IPI only > > serializes against page-table walkers that run with interrupts off > > (e.g. GUP-fast); the walk in __change_page_attr() runs with interrupts > > on, so nothing stops it from holding a stale pointer into P. > > > > Serialize the collapse - the PMD rebuild, TLB flush and PTE-table > > free - under cpa_lock, the lock __change_page_attr() takes for the > > split path, so a concurrent walker can no longer hold a pointer into > > a table the collapse is about to free. > > > > debug_pagealloc bypasses cpa_lock in __change_page_attr() (the direct > > map is 4K then, with no large pages to serialize), so the lock cannot > > order the two there. Skip the collapse in that config: it is only an > > optimization, and not freeing the tables leaves the unserialized walk > > nothing to race. > > > > Fixes: 41d88484c71c ("x86/mm/pat: restore large ROX pages after fragmentation") > > Signed-off-by: Denis V. Lunev > > --- > > arch/x86/mm/pat/set_memory.c | 16 +++++++++++++++- > > 1 file changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c > > index d023a40a1e03..ff6e3f612986 100644 > > --- a/arch/x86/mm/pat/set_memory.c > > +++ b/arch/x86/mm/pat/set_memory.c > > @@ -418,6 +418,16 @@ static void cpa_collapse_large_pages(struct cpa_data *cpa) > > int collapsed = 0; > > int i; > > > > + /* > > + * debug_pagealloc bypasses cpa_lock, so __change_page_attr() walks > > + * unserialized and freeing collapsed PTE-tables could race it; skip > > + * the optional merge there. > > + */ > > + if (debug_pagealloc_enabled()) > > + return; > > + > > + spin_lock(&cpa_lock); > > + > > if (cpa->flags & (CPA_PAGES_ARRAY | CPA_ARRAY)) { > > for (i = 0; i < cpa->numpages; i++) > > collapsed += collapse_large_pages(__cpa_addr(cpa, i), > > @@ -431,8 +441,10 @@ static void cpa_collapse_large_pages(struct cpa_data *cpa) > > collapsed += collapse_large_pages(addr, &pgtables); > > } > > > > - if (!collapsed) > > + if (!collapsed) { > > + spin_unlock(&cpa_lock); > > return; > > + } > > > > flush_tlb_all(); > > > > @@ -440,6 +452,8 @@ static void cpa_collapse_large_pages(struct cpa_data *cpa) > > list_del(&ptdesc->pt_list); > > pagetable_free(ptdesc); > > } > > + > > + spin_unlock(&cpa_lock); > > } > > > > static void cpa_flush(struct cpa_data *cpa, int cache) > > > > base-commit: b81d185839fade27f7c4e885856696cf497d53c1 > ok, guys :-) > > What is the direction to go with this change? > (the patch is working for me well in downstream). I am solving another issue in CPA, which I think can be solved by taking a different lock here which equally serialises CPA collapse walkers. I will be doing a respin on that series relatively soon for it assuming the approach is workable. See https://lore.kernel.org/linux-mm/aljb7hoFxL7aUZvG@lucifer/ > > Thank you in advance, >     Den > Thanks, Lorenzo