Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6] mm/hugetlb_cma: Fix null nodemask dereference in hugetlb_cma_alloc_frozen_folio
@ 2026-08-10 23:08 Sourav Panda
  2026-08-11  2:48 ` Muchun Song
  0 siblings, 1 reply; 7+ messages in thread
From: Sourav Panda @ 2026-08-10 23:08 UTC (permalink / raw)
  To: muchun.song, osalvador, akpm
  Cc: usama.arif, shakeel.butt, wangkefeng.wang, anshuman.khandual,
	david, surenb, fvdl, gthelen, hannes, riel, sj, vbabka, mhocko,
	bjackman, zi.yan, souravpanda, linux-mm, linux-kernel

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() via
alloc_gigantic_frozen_folio().

Additionally, hugetlb_cma_alloc_frozen_folio() previously attempted
allocation on hugetlb_cma[nid] without verifying if nid is included in
the caller's nodemask. Adding a node_isset(nid, *nodemask) check ensures
the initial preferred node allocation honors the memory policy / nodemask.

However, hugetlb_cma_alloc_frozen_folio() dereferences the nodemask in
node_isset(nid, *nodemask) and for_each_node_mask(node, *nodemask),
leading to a null pointer dereference kernel panic when nodemask is NULL.

Fix this by checking if nodemask is NULL in
hugetlb_cma_alloc_frozen_folio() and defaulting it to
cpuset_current_mems_allowed. Enclose the allocation attempts within
the cpuset seqcount retry loop so that if the cpuset changes concurrently
during allocation, the attempts are retried using the updated nodemask.
This ensures that the initial node check and fallback loop safely honor
the task's cpuset without violating cpuset constraints or causing NULL
pointer dereferences or unexpected allocation failures.

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

Fixes: eb02f14c4a2b ("mm/hugetlb: allow overcommitting gigantic hugepages")
Cc: stable@vger.kernel.org
Signed-off-by: Sourav Panda <souravpanda@google.com>
---
Changes in v6:
- Enclosed the CMA allocation attempts within the cpuset seqcount retry loop,
  retrying allocation upon cpuset mems_allowed updates to prevent unexpected
  allocation failures as suggested by Muchun Song.
- v5: https://lore.kernel.org/linux-mm/20260809043250.2917406-1-souravpanda@google.com/
- v4: https://lore.kernel.org/linux-mm/20260726072935.3513996-1-souravpanda@google.com/
- v3: https://lore.kernel.org/linux-mm/20260705175119.440599-1-souravpanda@google.com/
- 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_cma.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/mm/hugetlb_cma.c b/mm/hugetlb_cma.c
index 39344d6c78d8..9debf033d4fd 100644
--- a/mm/hugetlb_cma.c
+++ b/mm/hugetlb_cma.c
@@ -3,6 +3,7 @@
 #include <linux/mm.h>
 #include <linux/cma.h>
 #include <linux/compiler.h>
+#include <linux/cpuset.h>
 #include <linux/mm_inline.h>
 
 #include <asm/page.h>
@@ -30,15 +31,27 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
 	int node;
 	struct folio *folio;
 	struct page *page = NULL;
+	const nodemask_t *nmask;
+	nodemask_t local_node_mask;
+	unsigned int cpuset_mems_cookie;
 
 	if (!hugetlb_cma_size)
 		return NULL;
 
-	if (hugetlb_cma[nid])
+retry_cpuset:
+	if (!nodemask) {
+		cpuset_mems_cookie = read_mems_allowed_begin();
+		local_node_mask = cpuset_current_mems_allowed;
+		nmask = &local_node_mask;
+	} else {
+		nmask = nodemask;
+	}
+
+	if (hugetlb_cma[nid] && node_isset(nid, *nmask))
 		page = cma_alloc_frozen_compound(hugetlb_cma[nid], order);
 
 	if (!page && !(gfp_mask & __GFP_THISNODE)) {
-		for_each_node_mask(node, *nodemask) {
+		for_each_node_mask(node, *nmask) {
 			if (node == nid || !hugetlb_cma[node])
 				continue;
 
@@ -48,8 +61,12 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
 		}
 	}
 
-	if (!page)
+	if (!page) {
+		if (!nodemask &&
+		    unlikely(read_mems_allowed_retry(cpuset_mems_cookie)))
+			goto retry_cpuset;
 		return NULL;
+	}
 
 	folio = page_folio(page);
 	folio_set_hugetlb_cma(folio);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v6] mm/hugetlb_cma: Fix null nodemask dereference in hugetlb_cma_alloc_frozen_folio
  2026-08-10 23:08 [PATCH v6] mm/hugetlb_cma: Fix null nodemask dereference in hugetlb_cma_alloc_frozen_folio Sourav Panda
@ 2026-08-11  2:48 ` Muchun Song
  2026-08-11  5:26   ` Anshuman Khandual
  0 siblings, 1 reply; 7+ messages in thread
From: Muchun Song @ 2026-08-11  2:48 UTC (permalink / raw)
  To: Sourav Panda
  Cc: osalvador, akpm, usama.arif, shakeel.butt, wangkefeng.wang,
	anshuman.khandual, david, surenb, fvdl, gthelen, hannes, riel, sj,
	vbabka, mhocko, bjackman, zi.yan, linux-mm, linux-kernel



> On Aug 11, 2026, at 07:08, 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() via
> alloc_gigantic_frozen_folio().
> 
> Additionally, hugetlb_cma_alloc_frozen_folio() previously attempted
> allocation on hugetlb_cma[nid] without verifying if nid is included in
> the caller's nodemask. Adding a node_isset(nid, *nodemask) check ensures
> the initial preferred node allocation honors the memory policy / nodemask.
> 
> However, hugetlb_cma_alloc_frozen_folio() dereferences the nodemask in
> node_isset(nid, *nodemask) and for_each_node_mask(node, *nodemask),
> leading to a null pointer dereference kernel panic when nodemask is NULL.
> 
> Fix this by checking if nodemask is NULL in
> hugetlb_cma_alloc_frozen_folio() and defaulting it to
> cpuset_current_mems_allowed. Enclose the allocation attempts within
> the cpuset seqcount retry loop so that if the cpuset changes concurrently
> during allocation, the attempts are retried using the updated nodemask.
> This ensures that the initial node check and fallback loop safely honor
> the task's cpuset without violating cpuset constraints or causing NULL
> pointer dereferences or unexpected allocation failures.
> 
> 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
> 
> Fixes: eb02f14c4a2b ("mm/hugetlb: allow overcommitting gigantic hugepages")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sourav Panda <souravpanda@google.com>
> ---
> Changes in v6:
> - Enclosed the CMA allocation attempts within the cpuset seqcount retry loop,
>  retrying allocation upon cpuset mems_allowed updates to prevent unexpected
>  allocation failures as suggested by Muchun Song.
> - v5: https://lore.kernel.org/linux-mm/20260809043250.2917406-1-souravpanda@google.com/
> - v4: https://lore.kernel.org/linux-mm/20260726072935.3513996-1-souravpanda@google.com/
> - v3: https://lore.kernel.org/linux-mm/20260705175119.440599-1-souravpanda@google.com/
> - 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_cma.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/hugetlb_cma.c b/mm/hugetlb_cma.c
> index 39344d6c78d8..9debf033d4fd 100644
> --- a/mm/hugetlb_cma.c
> +++ b/mm/hugetlb_cma.c
> @@ -3,6 +3,7 @@
> #include <linux/mm.h>
> #include <linux/cma.h>
> #include <linux/compiler.h>
> +#include <linux/cpuset.h>
> #include <linux/mm_inline.h>
> 
> #include <asm/page.h>
> @@ -30,15 +31,27 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
> 	int node;
> 	struct folio *folio;
> 	struct page *page = NULL;
> + 	const nodemask_t *nmask;
> + 	nodemask_t local_node_mask;
> + 	unsigned int cpuset_mems_cookie;
> 
> 	if (!hugetlb_cma_size)
> 		return NULL;
> 
> - 	if (hugetlb_cma[nid])
> +		retry_cpuset:
> + 	if (!nodemask) {
> + 		cpuset_mems_cookie = read_mems_allowed_begin();
> + 		local_node_mask = cpuset_current_mems_allowed;
> + 		nmask = &local_node_mask;

I don't think we need a local_node_mask, nmask = &cpuset_current_mems_allowed
is enough. nodemask_t could be a huge array, so it might consume a lot of memory
on the stack. I don't think we need to do that unless we see a clear necessity.

Otherwise, LGTM.

Thanks.

> + 	} else {
> + 		nmask = nodemask;
> + 	}
> +
> + 	if (hugetlb_cma[nid] && node_isset(nid, *nmask))
> 		page = cma_alloc_frozen_compound(hugetlb_cma[nid], order);
> 
> 	if (!page && !(gfp_mask & __GFP_THISNODE)) {
> - 	for_each_node_mask(node, *nodemask) {
> + 	for_each_node_mask(node, *nmask) {
> 		if (node == nid || !hugetlb_cma[node])
> 			continue;
> 
> @@ -48,8 +61,12 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
> 		}
> 	}
> 
> - 	if (!page)
> + 	if (!page) {
> + 		if (!nodemask &&
> + 		    unlikely(read_mems_allowed_retry(cpuset_mems_cookie)))
> + 			goto retry_cpuset;
> 		return NULL;
> + 	}
> 
> 	folio = page_folio(page);
> 	folio_set_hugetlb_cma(folio);
> -- 
> 2.55.0



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6] mm/hugetlb_cma: Fix null nodemask dereference in hugetlb_cma_alloc_frozen_folio
  2026-08-11  2:48 ` Muchun Song
@ 2026-08-11  5:26   ` Anshuman Khandual
  2026-08-11  5:45     ` Sourav Panda
  2026-08-11  6:26     ` Muchun Song
  0 siblings, 2 replies; 7+ messages in thread
From: Anshuman Khandual @ 2026-08-11  5:26 UTC (permalink / raw)
  To: Muchun Song
  Cc: Sourav Panda, osalvador, akpm, usama.arif, shakeel.butt,
	wangkefeng.wang, david, surenb, fvdl, gthelen, hannes, riel, sj,
	vbabka, mhocko, bjackman, zi.yan, linux-mm, linux-kernel

On Tue, Aug 11, 2026 at 10:48:31AM +0800, Muchun Song wrote:
> 
> 
> > On Aug 11, 2026, at 07:08, 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() via
> > alloc_gigantic_frozen_folio().
> > 
> > Additionally, hugetlb_cma_alloc_frozen_folio() previously attempted
> > allocation on hugetlb_cma[nid] without verifying if nid is included in
> > the caller's nodemask. Adding a node_isset(nid, *nodemask) check ensures
> > the initial preferred node allocation honors the memory policy / nodemask.
> > 
> > However, hugetlb_cma_alloc_frozen_folio() dereferences the nodemask in
> > node_isset(nid, *nodemask) and for_each_node_mask(node, *nodemask),
> > leading to a null pointer dereference kernel panic when nodemask is NULL.
> > 
> > Fix this by checking if nodemask is NULL in
> > hugetlb_cma_alloc_frozen_folio() and defaulting it to
> > cpuset_current_mems_allowed. Enclose the allocation attempts within
> > the cpuset seqcount retry loop so that if the cpuset changes concurrently
> > during allocation, the attempts are retried using the updated nodemask.
> > This ensures that the initial node check and fallback loop safely honor
> > the task's cpuset without violating cpuset constraints or causing NULL
> > pointer dereferences or unexpected allocation failures.
> > 
> > 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
> > 
> > Fixes: eb02f14c4a2b ("mm/hugetlb: allow overcommitting gigantic hugepages")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Sourav Panda <souravpanda@google.com>
> > ---
> > Changes in v6:
> > - Enclosed the CMA allocation attempts within the cpuset seqcount retry loop,
> >  retrying allocation upon cpuset mems_allowed updates to prevent unexpected
> >  allocation failures as suggested by Muchun Song.
> > - v5: https://lore.kernel.org/linux-mm/20260809043250.2917406-1-souravpanda@google.com/
> > - v4: https://lore.kernel.org/linux-mm/20260726072935.3513996-1-souravpanda@google.com/
> > - v3: https://lore.kernel.org/linux-mm/20260705175119.440599-1-souravpanda@google.com/
> > - 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_cma.c | 23 ++++++++++++++++++-----
> > 1 file changed, 18 insertions(+), 5 deletions(-)
> > 
> > diff --git a/mm/hugetlb_cma.c b/mm/hugetlb_cma.c
> > index 39344d6c78d8..9debf033d4fd 100644
> > --- a/mm/hugetlb_cma.c
> > +++ b/mm/hugetlb_cma.c
> > @@ -3,6 +3,7 @@
> > #include <linux/mm.h>
> > #include <linux/cma.h>
> > #include <linux/compiler.h>
> > +#include <linux/cpuset.h>
> > #include <linux/mm_inline.h>
> > 
> > #include <asm/page.h>
> > @@ -30,15 +31,27 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
> > 	int node;
> > 	struct folio *folio;
> > 	struct page *page = NULL;
> > + 	const nodemask_t *nmask;
> > + 	nodemask_t local_node_mask;
> > + 	unsigned int cpuset_mems_cookie;
> > 
> > 	if (!hugetlb_cma_size)
> > 		return NULL;
> > 
> > - 	if (hugetlb_cma[nid])
> > +		retry_cpuset:
> > + 	if (!nodemask) {
> > + 		cpuset_mems_cookie = read_mems_allowed_begin();
> > + 		local_node_mask = cpuset_current_mems_allowed;
> > + 		nmask = &local_node_mask;
> 
> I don't think we need a local_node_mask, nmask = &cpuset_current_mems_allowed
> is enough. nodemask_t could be a huge array, so it might consume a lot of memory
> on the stack. I don't think we need to do that unless we see a clear necessity.

But does not local_node_mask capture current snapshot for cpuset_current_mems_allowed
after first taking a reference via read_mems_allowed_begin() which ensures that nmask
has a constant value there after. Otherwise nmask could just change value by the time
it actually gets used.

> 
> Otherwise, LGTM.
> 
> Thanks.
> 
> > + 	} else {
> > + 		nmask = nodemask;
> > + 	}
> > +
> > + 	if (hugetlb_cma[nid] && node_isset(nid, *nmask))
> > 		page = cma_alloc_frozen_compound(hugetlb_cma[nid], order);
> > 
> > 	if (!page && !(gfp_mask & __GFP_THISNODE)) {
> > - 	for_each_node_mask(node, *nodemask) {
> > + 	for_each_node_mask(node, *nmask) {
> > 		if (node == nid || !hugetlb_cma[node])
> > 			continue;
> > 
> > @@ -48,8 +61,12 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
> > 		}
> > 	}
> > 
> > - 	if (!page)
> > + 	if (!page) {
> > + 		if (!nodemask &&
> > + 		    unlikely(read_mems_allowed_retry(cpuset_mems_cookie)))
> > + 			goto retry_cpuset;
> > 		return NULL;
> > + 	}
> > 
> > 	folio = page_folio(page);
> > 	folio_set_hugetlb_cma(folio);
> > -- 
> > 2.55.0
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6] mm/hugetlb_cma: Fix null nodemask dereference in hugetlb_cma_alloc_frozen_folio
  2026-08-11  5:26   ` Anshuman Khandual
@ 2026-08-11  5:45     ` Sourav Panda
  2026-08-11  6:52       ` Anshuman Khandual
  2026-08-11  6:26     ` Muchun Song
  1 sibling, 1 reply; 7+ messages in thread
From: Sourav Panda @ 2026-08-11  5:45 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: Muchun Song, osalvador, akpm, usama.arif, shakeel.butt,
	wangkefeng.wang, david, surenb, fvdl, gthelen, hannes, riel, sj,
	vbabka, mhocko, bjackman, zi.yan, linux-mm, linux-kernel

On Mon, Aug 10, 2026 at 10:26 PM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
> On Tue, Aug 11, 2026 at 10:48:31AM +0800, Muchun Song wrote:
> >
> >
> > > On Aug 11, 2026, at 07:08, 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() via
> > > alloc_gigantic_frozen_folio().
> > >
> > > Additionally, hugetlb_cma_alloc_frozen_folio() previously attempted
> > > allocation on hugetlb_cma[nid] without verifying if nid is included in
> > > the caller's nodemask. Adding a node_isset(nid, *nodemask) check ensures
> > > the initial preferred node allocation honors the memory policy / nodemask.
> > >
> > > However, hugetlb_cma_alloc_frozen_folio() dereferences the nodemask in
> > > node_isset(nid, *nodemask) and for_each_node_mask(node, *nodemask),
> > > leading to a null pointer dereference kernel panic when nodemask is NULL.
> > >
> > > Fix this by checking if nodemask is NULL in
> > > hugetlb_cma_alloc_frozen_folio() and defaulting it to
> > > cpuset_current_mems_allowed. Enclose the allocation attempts within
> > > the cpuset seqcount retry loop so that if the cpuset changes concurrently
> > > during allocation, the attempts are retried using the updated nodemask.
> > > This ensures that the initial node check and fallback loop safely honor
> > > the task's cpuset without violating cpuset constraints or causing NULL
> > > pointer dereferences or unexpected allocation failures.
> > >
> > > 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
> > >
> > > Fixes: eb02f14c4a2b ("mm/hugetlb: allow overcommitting gigantic hugepages")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Sourav Panda <souravpanda@google.com>
> > > ---
> > > Changes in v6:
> > > - Enclosed the CMA allocation attempts within the cpuset seqcount retry loop,
> > >  retrying allocation upon cpuset mems_allowed updates to prevent unexpected
> > >  allocation failures as suggested by Muchun Song.
> > > - v5: https://lore.kernel.org/linux-mm/20260809043250.2917406-1-souravpanda@google.com/
> > > - v4: https://lore.kernel.org/linux-mm/20260726072935.3513996-1-souravpanda@google.com/
> > > - v3: https://lore.kernel.org/linux-mm/20260705175119.440599-1-souravpanda@google.com/
> > > - 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_cma.c | 23 ++++++++++++++++++-----
> > > 1 file changed, 18 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/mm/hugetlb_cma.c b/mm/hugetlb_cma.c
> > > index 39344d6c78d8..9debf033d4fd 100644
> > > --- a/mm/hugetlb_cma.c
> > > +++ b/mm/hugetlb_cma.c
> > > @@ -3,6 +3,7 @@
> > > #include <linux/mm.h>
> > > #include <linux/cma.h>
> > > #include <linux/compiler.h>
> > > +#include <linux/cpuset.h>
> > > #include <linux/mm_inline.h>
> > >
> > > #include <asm/page.h>
> > > @@ -30,15 +31,27 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
> > >     int node;
> > >     struct folio *folio;
> > >     struct page *page = NULL;
> > > +   const nodemask_t *nmask;
> > > +   nodemask_t local_node_mask;
> > > +   unsigned int cpuset_mems_cookie;
> > >
> > >     if (!hugetlb_cma_size)
> > >             return NULL;
> > >
> > > -   if (hugetlb_cma[nid])
> > > +           retry_cpuset:
> > > +   if (!nodemask) {
> > > +           cpuset_mems_cookie = read_mems_allowed_begin();
> > > +           local_node_mask = cpuset_current_mems_allowed;
> > > +           nmask = &local_node_mask;
> >
> > I don't think we need a local_node_mask, nmask = &cpuset_current_mems_allowed
> > is enough. nodemask_t could be a huge array, so it might consume a lot of memory
> > on the stack. I don't think we need to do that unless we see a clear necessity.
>
> But does not local_node_mask capture current snapshot for cpuset_current_mems_allowed
> after first taking a reference via read_mems_allowed_begin() which ensures that nmask
> has a constant value there after. Otherwise nmask could just change value by the time
> it actually gets used.
>
Hi,

Apologies for the poor netiquette of rushing out v7 so quickly (I just
saw your message after sending v7)! I am heading out on a long
vacation starting this Thursday, so I wanted to get an updated version
out for review before leaving.

Regarding local_node_mask: I thought the seqcount mechanism
(read_mems_allowed_begin() / read_mems_allowed_retry()) guarantees
correctness here without needing a stack copy. If
current->mems_allowed changes at any point while nmask is being used
(even during allocation) and the allocation fails,
read_mems_allowed_retry() detects the sequence change and jumps back
to retry_cpuset to retry with the updated nodemask.

I've incorporated this into v7 (Sorry again - for sending it so fast):
https://lore.kernel.org/linux-mm/20260810230844.3778931-1-souravpanda@google.com/

Thanks,
Sourav

> >
> > Otherwise, LGTM.
> >
> > Thanks.
> >
> > > +   } else {
> > > +           nmask = nodemask;
> > > +   }
> > > +
> > > +   if (hugetlb_cma[nid] && node_isset(nid, *nmask))
> > >             page = cma_alloc_frozen_compound(hugetlb_cma[nid], order);
> > >
> > >     if (!page && !(gfp_mask & __GFP_THISNODE)) {
> > > -   for_each_node_mask(node, *nodemask) {
> > > +   for_each_node_mask(node, *nmask) {
> > >             if (node == nid || !hugetlb_cma[node])
> > >                     continue;
> > >
> > > @@ -48,8 +61,12 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
> > >             }
> > >     }
> > >
> > > -   if (!page)
> > > +   if (!page) {
> > > +           if (!nodemask &&
> > > +               unlikely(read_mems_allowed_retry(cpuset_mems_cookie)))
> > > +                   goto retry_cpuset;
> > >             return NULL;
> > > +   }
> > >
> > >     folio = page_folio(page);
> > >     folio_set_hugetlb_cma(folio);
> > > --
> > > 2.55.0
> >


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6] mm/hugetlb_cma: Fix null nodemask dereference in hugetlb_cma_alloc_frozen_folio
  2026-08-11  5:26   ` Anshuman Khandual
  2026-08-11  5:45     ` Sourav Panda
@ 2026-08-11  6:26     ` Muchun Song
  2026-08-11  7:04       ` Anshuman Khandual
  1 sibling, 1 reply; 7+ messages in thread
From: Muchun Song @ 2026-08-11  6:26 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: Sourav Panda, osalvador, akpm, usama.arif, shakeel.butt,
	wangkefeng.wang, david, surenb, fvdl, gthelen, hannes, riel, sj,
	vbabka, mhocko, bjackman, zi.yan, linux-mm, linux-kernel



> On Aug 11, 2026, at 13:26, Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> 
> On Tue, Aug 11, 2026 at 10:48:31AM +0800, Muchun Song wrote:
>> 
>> 
>>> On Aug 11, 2026, at 07:08, 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() via
>>> alloc_gigantic_frozen_folio().
>>> 
>>> Additionally, hugetlb_cma_alloc_frozen_folio() previously attempted
>>> allocation on hugetlb_cma[nid] without verifying if nid is included in
>>> the caller's nodemask. Adding a node_isset(nid, *nodemask) check ensures
>>> the initial preferred node allocation honors the memory policy / nodemask.
>>> 
>>> However, hugetlb_cma_alloc_frozen_folio() dereferences the nodemask in
>>> node_isset(nid, *nodemask) and for_each_node_mask(node, *nodemask),
>>> leading to a null pointer dereference kernel panic when nodemask is NULL.
>>> 
>>> Fix this by checking if nodemask is NULL in
>>> hugetlb_cma_alloc_frozen_folio() and defaulting it to
>>> cpuset_current_mems_allowed. Enclose the allocation attempts within
>>> the cpuset seqcount retry loop so that if the cpuset changes concurrently
>>> during allocation, the attempts are retried using the updated nodemask.
>>> This ensures that the initial node check and fallback loop safely honor
>>> the task's cpuset without violating cpuset constraints or causing NULL
>>> pointer dereferences or unexpected allocation failures.
>>> 
>>> 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
>>> 
>>> Fixes: eb02f14c4a2b ("mm/hugetlb: allow overcommitting gigantic hugepages")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Sourav Panda <souravpanda@google.com>
>>> ---
>>> Changes in v6:
>>> - Enclosed the CMA allocation attempts within the cpuset seqcount retry loop,
>>> retrying allocation upon cpuset mems_allowed updates to prevent unexpected
>>> allocation failures as suggested by Muchun Song.
>>> - v5: https://lore.kernel.org/linux-mm/20260809043250.2917406-1-souravpanda@google.com/
>>> - v4: https://lore.kernel.org/linux-mm/20260726072935.3513996-1-souravpanda@google.com/
>>> - v3: https://lore.kernel.org/linux-mm/20260705175119.440599-1-souravpanda@google.com/
>>> - 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_cma.c | 23 ++++++++++++++++++-----
>>> 1 file changed, 18 insertions(+), 5 deletions(-)
>>> 
>>> diff --git a/mm/hugetlb_cma.c b/mm/hugetlb_cma.c
>>> index 39344d6c78d8..9debf033d4fd 100644
>>> --- a/mm/hugetlb_cma.c
>>> +++ b/mm/hugetlb_cma.c
>>> @@ -3,6 +3,7 @@
>>> #include <linux/mm.h>
>>> #include <linux/cma.h>
>>> #include <linux/compiler.h>
>>> +#include <linux/cpuset.h>
>>> #include <linux/mm_inline.h>
>>> 
>>> #include <asm/page.h>
>>> @@ -30,15 +31,27 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
>>> int node;
>>> struct folio *folio;
>>> struct page *page = NULL;
>>> +  const nodemask_t *nmask;
>>> +  nodemask_t local_node_mask;
>>> +  unsigned int cpuset_mems_cookie;
>>> 
>>> if (!hugetlb_cma_size)
>>> return NULL;
>>> 
>>> -  if (hugetlb_cma[nid])
>>> + retry_cpuset:
>>> +  if (!nodemask) {
>>> +  cpuset_mems_cookie = read_mems_allowed_begin();
>>> +  local_node_mask = cpuset_current_mems_allowed;
>>> +  nmask = &local_node_mask;
>> 
>> I don't think we need a local_node_mask, nmask = &cpuset_current_mems_allowed
>> is enough. nodemask_t could be a huge array, so it might consume a lot of memory
>> on the stack. I don't think we need to do that unless we see a clear necessity.
> 
> But does not local_node_mask capture current snapshot for cpuset_current_mems_allowed
> after first taking a reference via read_mems_allowed_begin() which ensures that nmask
> has a constant value there after. Otherwise nmask could just change value by the time
> it actually gets used.

However, this value can change while you are copying it, so a snapshot itself is not
a consistent value. Right?

> 
>> 
>> Otherwise, LGTM.
>> 
>> Thanks.
>> 
>>> +  } else {
>>> +  nmask = nodemask;
>>> +  }
>>> +
>>> +  if (hugetlb_cma[nid] && node_isset(nid, *nmask))
>>> page = cma_alloc_frozen_compound(hugetlb_cma[nid], order);
>>> 
>>> if (!page && !(gfp_mask & __GFP_THISNODE)) {
>>> -  for_each_node_mask(node, *nodemask) {
>>> +  for_each_node_mask(node, *nmask) {
>>> if (node == nid || !hugetlb_cma[node])
>>> continue;
>>> 
>>> @@ -48,8 +61,12 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
>>> }
>>> }
>>> 
>>> -  if (!page)
>>> +  if (!page) {
>>> +  if (!nodemask &&
>>> +      unlikely(read_mems_allowed_retry(cpuset_mems_cookie)))
>>> +  goto retry_cpuset;
>>> return NULL;
>>> +  }
>>> 
>>> folio = page_folio(page);
>>> folio_set_hugetlb_cma(folio);
>>> -- 
>>> 2.55.0




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6] mm/hugetlb_cma: Fix null nodemask dereference in hugetlb_cma_alloc_frozen_folio
  2026-08-11  5:45     ` Sourav Panda
@ 2026-08-11  6:52       ` Anshuman Khandual
  0 siblings, 0 replies; 7+ messages in thread
From: Anshuman Khandual @ 2026-08-11  6:52 UTC (permalink / raw)
  To: Sourav Panda
  Cc: Muchun Song, osalvador, akpm, usama.arif, shakeel.butt,
	wangkefeng.wang, david, surenb, fvdl, gthelen, hannes, riel, sj,
	vbabka, mhocko, bjackman, zi.yan, linux-mm, linux-kernel

On Mon, Aug 10, 2026 at 10:45:44PM -0700, Sourav Panda wrote:
> On Mon, Aug 10, 2026 at 10:26 PM Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
> >
> > On Tue, Aug 11, 2026 at 10:48:31AM +0800, Muchun Song wrote:
> > >
> > >
> > > > On Aug 11, 2026, at 07:08, 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() via
> > > > alloc_gigantic_frozen_folio().
> > > >
> > > > Additionally, hugetlb_cma_alloc_frozen_folio() previously attempted
> > > > allocation on hugetlb_cma[nid] without verifying if nid is included in
> > > > the caller's nodemask. Adding a node_isset(nid, *nodemask) check ensures
> > > > the initial preferred node allocation honors the memory policy / nodemask.
> > > >
> > > > However, hugetlb_cma_alloc_frozen_folio() dereferences the nodemask in
> > > > node_isset(nid, *nodemask) and for_each_node_mask(node, *nodemask),
> > > > leading to a null pointer dereference kernel panic when nodemask is NULL.
> > > >
> > > > Fix this by checking if nodemask is NULL in
> > > > hugetlb_cma_alloc_frozen_folio() and defaulting it to
> > > > cpuset_current_mems_allowed. Enclose the allocation attempts within
> > > > the cpuset seqcount retry loop so that if the cpuset changes concurrently
> > > > during allocation, the attempts are retried using the updated nodemask.
> > > > This ensures that the initial node check and fallback loop safely honor
> > > > the task's cpuset without violating cpuset constraints or causing NULL
> > > > pointer dereferences or unexpected allocation failures.
> > > >
> > > > 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
> > > >
> > > > Fixes: eb02f14c4a2b ("mm/hugetlb: allow overcommitting gigantic hugepages")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Sourav Panda <souravpanda@google.com>
> > > > ---
> > > > Changes in v6:
> > > > - Enclosed the CMA allocation attempts within the cpuset seqcount retry loop,
> > > >  retrying allocation upon cpuset mems_allowed updates to prevent unexpected
> > > >  allocation failures as suggested by Muchun Song.
> > > > - v5: https://lore.kernel.org/linux-mm/20260809043250.2917406-1-souravpanda@google.com/
> > > > - v4: https://lore.kernel.org/linux-mm/20260726072935.3513996-1-souravpanda@google.com/
> > > > - v3: https://lore.kernel.org/linux-mm/20260705175119.440599-1-souravpanda@google.com/
> > > > - 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_cma.c | 23 ++++++++++++++++++-----
> > > > 1 file changed, 18 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/mm/hugetlb_cma.c b/mm/hugetlb_cma.c
> > > > index 39344d6c78d8..9debf033d4fd 100644
> > > > --- a/mm/hugetlb_cma.c
> > > > +++ b/mm/hugetlb_cma.c
> > > > @@ -3,6 +3,7 @@
> > > > #include <linux/mm.h>
> > > > #include <linux/cma.h>
> > > > #include <linux/compiler.h>
> > > > +#include <linux/cpuset.h>
> > > > #include <linux/mm_inline.h>
> > > >
> > > > #include <asm/page.h>
> > > > @@ -30,15 +31,27 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
> > > >     int node;
> > > >     struct folio *folio;
> > > >     struct page *page = NULL;
> > > > +   const nodemask_t *nmask;
> > > > +   nodemask_t local_node_mask;
> > > > +   unsigned int cpuset_mems_cookie;
> > > >
> > > >     if (!hugetlb_cma_size)
> > > >             return NULL;
> > > >
> > > > -   if (hugetlb_cma[nid])
> > > > +           retry_cpuset:
> > > > +   if (!nodemask) {
> > > > +           cpuset_mems_cookie = read_mems_allowed_begin();
> > > > +           local_node_mask = cpuset_current_mems_allowed;
> > > > +           nmask = &local_node_mask;
> > >
> > > I don't think we need a local_node_mask, nmask = &cpuset_current_mems_allowed
> > > is enough. nodemask_t could be a huge array, so it might consume a lot of memory
> > > on the stack. I don't think we need to do that unless we see a clear necessity.
> >
> > But does not local_node_mask capture current snapshot for cpuset_current_mems_allowed
> > after first taking a reference via read_mems_allowed_begin() which ensures that nmask
> > has a constant value there after. Otherwise nmask could just change value by the time
> > it actually gets used.
> >
> Hi,
> 
> Apologies for the poor netiquette of rushing out v7 so quickly (I just
> saw your message after sending v7)! I am heading out on a long
> vacation starting this Thursday, so I wanted to get an updated version
> out for review before leaving.

Not a problem.

> 
> Regarding local_node_mask: I thought the seqcount mechanism
> (read_mems_allowed_begin() / read_mems_allowed_retry()) guarantees
> correctness here without needing a stack copy. If
> current->mems_allowed changes at any point while nmask is being used
> (even during allocation) and the allocation fails,
> read_mems_allowed_retry() detects the sequence change and jumps back
> to retry_cpuset to retry with the updated nodemask.

read_mems_allowed_begin() --> read_mems_allowed_retry() --> Retry seems
to be sufficient for retrying allocation again if and when the nodemask
changes while causing an allocation failure.

But is there a possibility for the loop inside for_each_node_mask() to
detect a changed node mask and then successfully allocate from there ?
I guess that is semantically correct as the allocation happened from
task's cpuset_current_mems_allowed which might have changed afterwards.

Dropping the local variable here seems right.

> 
> I've incorporated this into v7 (Sorry again - for sending it so fast):
> https://lore.kernel.org/linux-mm/20260810230844.3778931-1-souravpanda@google.com/
> 
> Thanks,
> Sourav
> 
> > >
> > > Otherwise, LGTM.
> > >
> > > Thanks.
> > >
> > > > +   } else {
> > > > +           nmask = nodemask;
> > > > +   }
> > > > +
> > > > +   if (hugetlb_cma[nid] && node_isset(nid, *nmask))
> > > >             page = cma_alloc_frozen_compound(hugetlb_cma[nid], order);
> > > >
> > > >     if (!page && !(gfp_mask & __GFP_THISNODE)) {
> > > > -   for_each_node_mask(node, *nodemask) {
> > > > +   for_each_node_mask(node, *nmask) {
> > > >             if (node == nid || !hugetlb_cma[node])
> > > >                     continue;
> > > >
> > > > @@ -48,8 +61,12 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
> > > >             }
> > > >     }
> > > >
> > > > -   if (!page)
> > > > +   if (!page) {
> > > > +           if (!nodemask &&
> > > > +               unlikely(read_mems_allowed_retry(cpuset_mems_cookie)))
> > > > +                   goto retry_cpuset;
> > > >             return NULL;
> > > > +   }
> > > >
> > > >     folio = page_folio(page);
> > > >     folio_set_hugetlb_cma(folio);
> > > > --
> > > > 2.55.0
> > >


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6] mm/hugetlb_cma: Fix null nodemask dereference in hugetlb_cma_alloc_frozen_folio
  2026-08-11  6:26     ` Muchun Song
@ 2026-08-11  7:04       ` Anshuman Khandual
  0 siblings, 0 replies; 7+ messages in thread
From: Anshuman Khandual @ 2026-08-11  7:04 UTC (permalink / raw)
  To: Muchun Song
  Cc: Sourav Panda, osalvador, akpm, usama.arif, shakeel.butt,
	wangkefeng.wang, david, surenb, fvdl, gthelen, hannes, riel, sj,
	vbabka, mhocko, bjackman, zi.yan, linux-mm, linux-kernel

On Tue, Aug 11, 2026 at 02:26:44PM +0800, Muchun Song wrote:
> 
> 
> > On Aug 11, 2026, at 13:26, Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> > 
> > On Tue, Aug 11, 2026 at 10:48:31AM +0800, Muchun Song wrote:
> >> 
> >> 
> >>> On Aug 11, 2026, at 07:08, 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() via
> >>> alloc_gigantic_frozen_folio().
> >>> 
> >>> Additionally, hugetlb_cma_alloc_frozen_folio() previously attempted
> >>> allocation on hugetlb_cma[nid] without verifying if nid is included in
> >>> the caller's nodemask. Adding a node_isset(nid, *nodemask) check ensures
> >>> the initial preferred node allocation honors the memory policy / nodemask.
> >>> 
> >>> However, hugetlb_cma_alloc_frozen_folio() dereferences the nodemask in
> >>> node_isset(nid, *nodemask) and for_each_node_mask(node, *nodemask),
> >>> leading to a null pointer dereference kernel panic when nodemask is NULL.
> >>> 
> >>> Fix this by checking if nodemask is NULL in
> >>> hugetlb_cma_alloc_frozen_folio() and defaulting it to
> >>> cpuset_current_mems_allowed. Enclose the allocation attempts within
> >>> the cpuset seqcount retry loop so that if the cpuset changes concurrently
> >>> during allocation, the attempts are retried using the updated nodemask.
> >>> This ensures that the initial node check and fallback loop safely honor
> >>> the task's cpuset without violating cpuset constraints or causing NULL
> >>> pointer dereferences or unexpected allocation failures.
> >>> 
> >>> 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
> >>> 
> >>> Fixes: eb02f14c4a2b ("mm/hugetlb: allow overcommitting gigantic hugepages")
> >>> Cc: stable@vger.kernel.org
> >>> Signed-off-by: Sourav Panda <souravpanda@google.com>
> >>> ---
> >>> Changes in v6:
> >>> - Enclosed the CMA allocation attempts within the cpuset seqcount retry loop,
> >>> retrying allocation upon cpuset mems_allowed updates to prevent unexpected
> >>> allocation failures as suggested by Muchun Song.
> >>> - v5: https://lore.kernel.org/linux-mm/20260809043250.2917406-1-souravpanda@google.com/
> >>> - v4: https://lore.kernel.org/linux-mm/20260726072935.3513996-1-souravpanda@google.com/
> >>> - v3: https://lore.kernel.org/linux-mm/20260705175119.440599-1-souravpanda@google.com/
> >>> - 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_cma.c | 23 ++++++++++++++++++-----
> >>> 1 file changed, 18 insertions(+), 5 deletions(-)
> >>> 
> >>> diff --git a/mm/hugetlb_cma.c b/mm/hugetlb_cma.c
> >>> index 39344d6c78d8..9debf033d4fd 100644
> >>> --- a/mm/hugetlb_cma.c
> >>> +++ b/mm/hugetlb_cma.c
> >>> @@ -3,6 +3,7 @@
> >>> #include <linux/mm.h>
> >>> #include <linux/cma.h>
> >>> #include <linux/compiler.h>
> >>> +#include <linux/cpuset.h>
> >>> #include <linux/mm_inline.h>
> >>> 
> >>> #include <asm/page.h>
> >>> @@ -30,15 +31,27 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
> >>> int node;
> >>> struct folio *folio;
> >>> struct page *page = NULL;
> >>> +  const nodemask_t *nmask;
> >>> +  nodemask_t local_node_mask;
> >>> +  unsigned int cpuset_mems_cookie;
> >>> 
> >>> if (!hugetlb_cma_size)
> >>> return NULL;
> >>> 
> >>> -  if (hugetlb_cma[nid])
> >>> + retry_cpuset:
> >>> +  if (!nodemask) {
> >>> +  cpuset_mems_cookie = read_mems_allowed_begin();
> >>> +  local_node_mask = cpuset_current_mems_allowed;
> >>> +  nmask = &local_node_mask;
> >> 
> >> I don't think we need a local_node_mask, nmask = &cpuset_current_mems_allowed
> >> is enough. nodemask_t could be a huge array, so it might consume a lot of memory
> >> on the stack. I don't think we need to do that unless we see a clear necessity.
> > 
> > But does not local_node_mask capture current snapshot for cpuset_current_mems_allowed
> > after first taking a reference via read_mems_allowed_begin() which ensures that nmask
> > has a constant value there after. Otherwise nmask could just change value by the time
> > it actually gets used.
> 
> However, this value can change while you are copying it, so a snapshot itself is not
> a consistent value. Right?

Yes it can change while copying, although it stays consistent for the for loop
later but that might not be necessary after all. As mentioned earlier it seems
OK drop the local variable here.

> 
> > 
> >> 
> >> Otherwise, LGTM.
> >> 
> >> Thanks.
> >> 
> >>> +  } else {
> >>> +  nmask = nodemask;
> >>> +  }
> >>> +
> >>> +  if (hugetlb_cma[nid] && node_isset(nid, *nmask))
> >>> page = cma_alloc_frozen_compound(hugetlb_cma[nid], order);
> >>> 
> >>> if (!page && !(gfp_mask & __GFP_THISNODE)) {
> >>> -  for_each_node_mask(node, *nodemask) {
> >>> +  for_each_node_mask(node, *nmask) {
> >>> if (node == nid || !hugetlb_cma[node])
> >>> continue;
> >>> 
> >>> @@ -48,8 +61,12 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
> >>> }
> >>> }
> >>> 
> >>> -  if (!page)
> >>> +  if (!page) {
> >>> +  if (!nodemask &&
> >>> +      unlikely(read_mems_allowed_retry(cpuset_mems_cookie)))
> >>> +  goto retry_cpuset;
> >>> return NULL;
> >>> +  }
> >>> 
> >>> folio = page_folio(page);
> >>> folio_set_hugetlb_cma(folio);
> >>> -- 
> >>> 2.55.0
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-08-11  7:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-10 23:08 [PATCH v6] mm/hugetlb_cma: Fix null nodemask dereference in hugetlb_cma_alloc_frozen_folio Sourav Panda
2026-08-11  2:48 ` Muchun Song
2026-08-11  5:26   ` Anshuman Khandual
2026-08-11  5:45     ` Sourav Panda
2026-08-11  6:52       ` Anshuman Khandual
2026-08-11  6:26     ` Muchun Song
2026-08-11  7:04       ` Anshuman Khandual

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox