From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 AE099285073 for ; Thu, 20 Nov 2025 19:56:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763668576; cv=none; b=JxvHW+5SlVuWr7LdI9FciYfJlzu35yRt2L/pg9vSU/NzcraWx3lFT4Ah2EsrJdVwXsWCz/XUkCMDZK0Ru3sEGGy29jx/+jobWVCSlskaoNZQ5Ku4X522iZhvPx+0ayYCofVaAuwzgPjXXD7F3zvD0v9Fk5ejhPMM2k8fF6VpzgU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763668576; c=relaxed/simple; bh=qtHvUpDPE2SB/7d+oRdbHGsrVFL/jkakr/Kb85gdfuY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=EAb5zU6WwKAZU2f2fdiPW+CgbuPRDqU3xZncOniMxPbRH6bmA+P1Sp5R6p3tOdXDi7EM/a8hzcq9pbvKuapkrADAvAcwzUerQDzrBkgzrRIcz6Hxc3I2NheEPpB/OW/X5yq++Vwq3LsAKVrb7p2YbYxLyIUWjWb6eg0gPDWhQFk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=s2EPi/es; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="s2EPi/es" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A2534C4CEF1; Thu, 20 Nov 2025 19:56:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1763668576; bh=qtHvUpDPE2SB/7d+oRdbHGsrVFL/jkakr/Kb85gdfuY=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=s2EPi/esShxVmKeGCrkp0KRAHLvnhdW/GcHJ8d+tDjKFM7F8KsywJTyMOj6UPKEUN JfICeRLft9enfyA48HZS7fZIhmli01y5IKtxQStFgbnQYh+Wg/hJdKNQu5pKrWGI6U aUE4Uh2NfkDnWPjyfjVoId8/4u0TJdwMKJcgE5W/E4ibcScQ9xzkpoJRaXaaMeB0va bCoxFkkS82ym5DC9YfXtS9NvCeowXC/n1nOniqwrGjbrcXl873VWPm7GUHrysXlcpR IFBQ7MYOy5bnX8XxTEKbVaGPK6E7tucBykq4p+bB5GZloovvY47vJAsWFlKC27h9Gl 1Al2xQMhRMsvA== Message-ID: Date: Thu, 20 Nov 2025 20:56:10 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH 1/3] mm/huge_memory: prevent NULL pointer dereference in try_folio_split_to_order() To: Zi Yan Cc: Lorenzo Stoakes , Andrew Morton , Baolin Wang , "Liam R. Howlett" , Nico Pache , Ryan Roberts , Dev Jain , Barry Song , Lance Yang , Miaohe Lin , Naoya Horiguchi , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20251120035953.1115736-1-ziy@nvidia.com> <20251120035953.1115736-2-ziy@nvidia.com> <875584d7-5a68-4f7a-8549-2a9cd6c7f9d8@kernel.org> From: "David Hildenbrand (Red Hat)" Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 11/20/25 15:41, Zi Yan wrote: > On 20 Nov 2025, at 4:25, David Hildenbrand (Red Hat) wrote: > >> On 11/20/25 04:59, Zi Yan wrote: >>> folio_split_supported() used in try_folio_split_to_order() requires >>> folio->mapping to be non NULL, but current try_folio_split_to_order() does >>> not check it. Add the check to prevent NULL pointer dereference. >>> >>> There is no issue in the current code, since try_folio_split_to_order() is >>> only used in truncate_inode_partial_folio(), where folio->mapping is not >>> NULL. >>> >>> Signed-off-by: Zi Yan >>> --- >>> include/linux/huge_mm.h | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h >>> index 1d439de1ca2c..0d55354e3a34 100644 >>> --- a/include/linux/huge_mm.h >>> +++ b/include/linux/huge_mm.h >>> @@ -407,6 +407,13 @@ static inline int split_huge_page_to_order(struct page *page, unsigned int new_o >>> static inline int try_folio_split_to_order(struct folio *folio, >>> struct page *page, unsigned int new_order) >>> { >>> + /* >>> + * Folios that just got truncated cannot get split. Signal to the >>> + * caller that there was a race. >>> + */ >>> + if (!folio_test_anon(folio) && !folio->mapping) >>> + return -EBUSY; >>> + >>> if (!folio_split_supported(folio, new_order, SPLIT_TYPE_NON_UNIFORM, /* warns= */ false)) >>> return split_huge_page_to_order(&folio->page, new_order); >>> return folio_split(folio, new_order, page, NULL); >> >> I guess we'll take the one from Wei >> >> https://lkml.kernel.org/r/20251119235302.24773-1-richard.weiyang@gmail.com >> >> right? > > This is different. Wei’s fix is to __folio_split(), but mine is to > try_folio_split_to_order(). Both call folio_split_supported(), thus > both need the folio->mapping check. Ah, good that I double-checked :) > > That is also my question in the cover letter on whether we should > move folio->mapping check to folio_split_supported() and return > error code instead of bool. Otherwise, any folio_split_supported() > caller needs to check folio->mapping. I think the situation with truncation (-that shmem swapcache thing, let's ignore that for now) is that the folio cannot be split until fully freed. But we don't want to return -EINVAL to the caller, the assumption is that the folio will soon get resolved -- folio freed -- and the caller will be able to make progress. So it's not really expected to be persistent. -EINVAL rather signals "this cannot possibly work, so fail whatever you are trying". We rather want to indicate "there was some race situation, if you try again later it might work or might have resolved itself". Not sure I like returning an error from folio_split_supported(), as it's rather a boolean check (supported vs. not supported). Likely we could just return "false" for truncated folios in folio_split_supported(), but then state that that case must be handled upfront. We could provide another helper to wrap the truncation check, hmmm BTW, I wonder if the is_huge_zero_folio() check should go into folio_split_supported() and just return in -EINVAL. (we shouldn't really trigger that). Similarly we could add a hugetlb sanity check. -- Cheers David