Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Muchun Song <muchun.song@linux.dev>
To: Sourav Panda <souravpanda@google.com>
Cc: osalvador@suse.de, akpm@linux-foundation.org, david@kernel.org,
	surenb@google.com, fvdl@google.com, gthelen@google.com,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Vlastimil Babka <vbabka@kernel.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Michal Hocko <mhocko@suse.com>,
	Brendan Jackman <jackmanb@google.com>,
	Johannes Weiner <hannes@cmpxchg.org>, Zi Yan <ziy@nvidia.com>
Subject: Re: [PATCH v3] mm/hugetlb: Fix null nodemask in alloc_fresh_hugetlb_folio
Date: Mon, 6 Jul 2026 14:17:39 +0800	[thread overview]
Message-ID: <B8DC1E3C-24B1-4608-86B1-5F12B2FF4DBE@linux.dev> (raw)
In-Reply-To: <20260705175119.440599-1-souravpanda@google.com>



> On Jul 6, 2026, at 01:51, Sourav Panda <souravpanda@google.com> wrote:
> 
> alloc_buddy_hugetlb_folio_with_mpol() can pass a NULL nodemask to
> alloc_fresh_hugetlb_folio() as a fallback to allocate from all
> nodes. If order is gigantic, alloc_fresh_hugetlb_folio() propagates
> the NULL nodemask down to hugetlb_cma_alloc_frozen_folio() which blindly
> dereferences it in for_each_node_mask(), leading to a null pointer
> dereference.
> 
> Similarly, if the CMA allocation fails, the fallback
> alloc_contig_frozen_pages() is also called with a NULL nodemask,
> which may cause issues.
> 
> Fix this by explicitly checking if nodemask is NULL in
> alloc_fresh_hugetlb_folio() and defaulting to
> cpuset_current_mems_allowed. This ensures that both the CMA and
> contiguous allocators receive a valid nodemask safely using a seqcount
> loop to prevent torn reads.
> 
> From a userspace perspective, this bug allows an unprivileged user to
> crash the kernel (trigger a panic) by requesting a gigantic hugepage
> allocation with MPOL_PREFERRED_MANY on a system where CMA is only
> configured on a subset of NUMA nodes.
> 
> This can be reproduced by booting a VM with two NUMA nodes, restricting
> CMA to Node 1 (e.g., hugetlb_cma=1:1G default_hugepagesz=1G hugepagesz=1G
> hugepages=0), and running a program that allocates a 1GB hugepage area
> without reserving, restricts allocation to Node 0 using mbind() with
> MPOL_PREFERRED_MANY, and triggers a page fault:
> 
>  void *ptr = mmap(NULL, 1UL << 30, PROT_READ | PROT_WRITE,
>                   MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB |
>                   MAP_HUGE_1GB | MAP_NORESERVE, -1, 0);
>  unsigned long nodemask = 1; /* Node 0 */
>  mbind(ptr, 1UL << 30, MPOL_PREFERRED_MANY, &nodemask,
>        sizeof(nodemask) * 8, 0);
>  memset(ptr, 0, 1UL << 30); /* Trigger fault */
> 
> This results in a NULL pointer dereference:
> 
>  BUG: kernel NULL pointer dereference, address: 0000000000000000
>  #PF: supervisor read access in kernel mode
>  #PF: error_code(0x0000) - not-present page
>  Oops: Oops: 0000 [#1] SMP NOPTI
>  RIP: 0010:hugetlb_cma_alloc_frozen_folio+0x75/0x120
>  Call Trace:
>   <TASK>
>   only_alloc_fresh_hugetlb_folio.isra.0+0x2c/0x160
>   alloc_surplus_hugetlb_folio+0x6d/0x100
>   alloc_hugetlb_folio+0x3c5/0x660
>   hugetlb_no_page+0x3d9/0x650
> 
> Additionally, this patch adds a missing node_isset(nid, *nodemask) check
> in hugetlb_cma_alloc_frozen_folio() to ensure the initial node allocation
> attempt respects the memory policy.
> 
> Fixes: eb02f14c4a2b ("mm/hugetlb: allow overcommitting gigantic hugepages")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sourav Panda <souravpanda@google.com>
> ---
> Changes in v3:
> - Condensed the reproducer and QEMU setup from v1 discussion into a readable summary per Andrew Morton.
> - Safely read cpuset_current_mems_allowed using a seqcount loop to prevent torn reads.
> - v2: https://lore.kernel.org/linux-mm/20260704174930.2885785-1-souravpanda@google.com/
> - v1: https://lore.kernel.org/linux-mm/20260702215713.627941-1-souravpanda@google.com/
> mm/hugetlb.c     | 12 ++++++++++++
> mm/hugetlb_cma.c |  2 +-
> 2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 571212b80835..ee67ea29c003 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1864,6 +1864,18 @@ static struct folio *alloc_fresh_hugetlb_folio(struct hstate *h,
> gfp_t gfp_mask, int nid, nodemask_t *nmask)
> {
> 	struct folio *folio;
> + 	nodemask_t local_node_mask;
> +
> + 	if (!nmask) {
> + 		unsigned int cpuset_mems_cookie;
> +
> + 		do {
> + 			cpuset_mems_cookie = read_mems_allowed_begin();
> + 			local_node_mask = cpuset_current_mems_allowed;
> + 		} while (read_mems_allowed_retry(cpuset_mems_cookie));
> +
> + 		nmask = &local_node_mask;
> + 	}

As Andrew pointed it out, it is not helpful to prevent any false-positive
allocation failures. We need something like the following:

  seq = read_mems_allowed_begin();

  alloc_nodemask = nodemask;
  if (cpusets_enabled() && !alloc_nodemask)
        alloc_nodemask = &cpuset_current_mems_allowed;

  for_each_node_mask(node, alloc_nodemask) {
        if (cpusets_enabled() && nodemask &&
            !cpuset_zone_allowed(zone, gfp_mask))
                continue;

        ...
  }

  if (!page && read_mems_allowed_retry(seq))
        goto retry;


After further consideration, I believe we only need to fix
alloc_gigantic_frozen_folio(), as alloc_buddy_frozen_folio() already
handles the NULL nodemask properly via prepare_alloc_pages().

To address the issue in alloc_gigantic_frozen_folio(), we have two
potential options:

- Option 1: Fix it directly within alloc_gigantic_frozen_folio().
  This would centrally cover both of its underlying allocation paths:
  hugetlb_cma_alloc_frozen_folio() and alloc_contig_frozen_pages().

- Option 2: Fix hugetlb_cma_alloc_frozen_folio() and alloc_contig_frozen_pages()
  separately. The rationale here is that since the buddy allocator
  already handles this case internally, and alloc_contig_frozen_pages()
  is also a low-level allocator, it would be better to keep their behaviors
  consistent.

Both approaches are viable from my perspective. I'm CCing more MM maintainers
and reviewers to gather your insights and decide on the best path forward.

Muchun,
Thanks.

> 
> 	folio = only_alloc_fresh_hugetlb_folio(h, gfp_mask, nid, nmask, NULL);
> 	if (folio)
> diff --git a/mm/hugetlb_cma.c b/mm/hugetlb_cma.c
> index 39344d6c78d8..79dbd0baafa3 100644
> --- a/mm/hugetlb_cma.c
> +++ b/mm/hugetlb_cma.c
> @@ -34,7 +34,7 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
> 	if (!hugetlb_cma_size)
> 		return NULL;
> 
> - 	if (hugetlb_cma[nid])
> + 	if (hugetlb_cma[nid] && node_isset(nid, *nodemask))
> 		page = cma_alloc_frozen_compound(hugetlb_cma[nid], order);
> 
> 	if (!page && !(gfp_mask & __GFP_THISNODE)) {
> -- 
> 2.55.0.rc0.799.gd6f94ed593-goog
> 



  parent reply	other threads:[~2026-07-06  6:18 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-05 17:51 [PATCH v3] mm/hugetlb: Fix null nodemask in alloc_fresh_hugetlb_folio Sourav Panda
2026-07-05 19:04 ` Andrew Morton
2026-07-06  6:17 ` Muchun Song [this message]
2026-07-06  6:55   ` Sourav Panda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=B8DC1E3C-24B1-4608-86B1-5F12B2FF4DBE@linux.dev \
    --to=muchun.song@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=fvdl@google.com \
    --cc=gthelen@google.com \
    --cc=hannes@cmpxchg.org \
    --cc=jackmanb@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=osalvador@suse.de \
    --cc=souravpanda@google.com \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox