From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (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 D4B8B3148BB for ; Mon, 24 Nov 2025 16:22:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764001324; cv=none; b=aQ1s/clL2126/Y8Kxj3Fdf1rCG1+5U5t953T7QK3AyH/haMxrtvICq/j2WPl0rJf6Z453RgmCPGIivLUMiYmB3R2frwgZg86In43Upzyst5Dd+KsfA+jlcjevZFfR/+0Xqk4pu6+ggMVawOt6nY2ocNXLJsVCWK/H/mj3tlwGFM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764001324; c=relaxed/simple; bh=AHQ8lpG6m269I8EQa2xE41/qDjT2dOBjGSmSSmZ5/tA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=kTmyeVyOVmh8H6IffK9yzeaNFHOv4X9AxsL6BwQAKoWHRIJbXuiNPdPaiN1aNS+0sSx7BYWCbtRrTzjaivpif+pEdjXksFfM/ffqqlqf1aVyJZuX3HigEsJVTpHeRKxOxTt1IqMwzqr8XNIErv4jqqeqNpVZoZfjFvHcMwnFI6Q= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=CYd/G4C9; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="CYd/G4C9" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=7WX90pJ5tXKEJjUZci6+pSEheLnjdAzO/pRFmPh4C/M=; b=CYd/G4C9W9UeNAzrhTfgnXJ7YO VOPKxJQJaMbZ55ev+OV74Ppn7hX9MSC9cBIxRm3Nqokzq4ptw4omqFhnFJKkjhf4VapV++OjIYUO+ 3eG2URmbIu9fLo/iB6Cmy7zQfy2I3C0iHUYlNsRI8dE/wsjVba2WzxYCNSC4bQDWAHIgsXEtKYaU6 +Q+4ibDzfBLu+7MAiWIEJEy81cAFLuRuMFdEpN2FdJwZbThpClRpWsccaLIWENNf5r61qSPeuTd7N dYAVRq8ksK4DGBuAthJhcbw7mhrfN78R8gBkJ1VO/7OONBlf5t6HDB/s0EPsLJggaC3I9XMwZLF04 nkA/+TsA==; Received: from willy by casper.infradead.org with local (Exim 4.98.2 #2 (Red Hat Linux)) id 1vNZK3-00000007Dpa-3piz; Mon, 24 Nov 2025 16:21:51 +0000 Date: Mon, 24 Nov 2025 16:21:51 +0000 From: Matthew Wilcox To: Shardul Bankar Cc: linux-mm@kvack.org, dev.jain@arm.com, david@kernel.org, linux-kernel@vger.kernel.org, syzbot+a785d07959bc94837d51@syzkaller.appspotmail.com, akpm@linux-foundation.org, lorenzo.stoakes@oracle.com, ziy@nvidia.com, baolin.wang@linux.alibaba.com, Liam.Howlett@oracle.com, npache@redhat.com, ryan.roberts@arm.com, baohua@kernel.org, lance.yang@linux.dev, janak@mpiricsoftware.com, shardulsb08@gmail.com Subject: Re: [PATCH v2] mm: khugepaged: fix memory leak in collapse_file xas retry loop Message-ID: References: <703387c8908a609c3de966574dfcf481c5a97216.camel@mpiricsoftware.com> <20251124161149.1302507-1-shardul.b@mpiricsoftware.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=us-ascii Content-Disposition: inline In-Reply-To: <20251124161149.1302507-1-shardul.b@mpiricsoftware.com> On Mon, Nov 24, 2025 at 09:41:49PM +0530, Shardul Bankar wrote: > collapse_file() uses xas_create_range() in a retry loop that calls > xas_nomem() on -ENOMEM and then retries. xas_nomem() may allocate a > spare xa_node and store it in xas->xa_alloc. > > If the lock is dropped after xas_nomem(), another thread can expand > the xarray tree in the meantime. On the next retry, xas_create_range() > can then succeed trivially without consuming the node stored in > xas->xa_alloc. If we then either succeed or give up and go to the > rollback path without calling xas_destroy(), that spare node leaks. Then wouldn't freeing the excess node in xas_create_range() be the correct fix, instead of requiring the caller to think about this? > Fix this by calling xas_destroy(&xas) in both the success case > (!xas_error(&xas)) and the failure case where xas_nomem() returns > false and we abort. xas_destroy() will free any unused spare node in > xas->xa_alloc and is a no-op if there is nothing left to free. > > Link: https://syzkaller.appspot.com/bug?id=a274d65fc733448ed518ad15481ed575669dd98c > Fixes: cae106dd67b9 ("mm/khugepaged: refactor collapse_file control flow") > Signed-off-by: Shardul Bankar > --- > v2: > - Call xas_destroy() on both success and failure > - Explained retry semantics and xa_alloc / concurrency risk > - Dropped cleanup_empty_nodes from previous proposal > > mm/khugepaged.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index abe54f0043c7..0794a99c807f 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -1872,11 +1872,14 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr, > do { > xas_lock_irq(&xas); > xas_create_range(&xas); > - if (!xas_error(&xas)) > + if (!xas_error(&xas)) { > + xas_destroy(&xas); > break; > + } > xas_unlock_irq(&xas); > if (!xas_nomem(&xas, GFP_KERNEL)) { > result = SCAN_FAIL; > + xas_destroy(&xas); > goto rollback; > } > } while (1); > -- > 2.34.1 > >