Re: [PATCH v5] mm/hugetlb_cma: Fix null nodemask dereference in hugetlb_cma_alloc_frozen_folio
Sourav Panda <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.kernel.mm |
|---|---|
| Message-ID | <CANruzcSySUn_gLEh9kxwvatSCn4hizcy6v2BkcMe0Jb=HoUOSw@mail.gmail.com> |
On Sat, Aug 8, 2026 at 11:58 PM Muchun Song <[email protected]> wrote: > > > > > On Aug 9, 2026, at 12:32, Sourav Panda <[email protected]> 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 (safely read using a seqcount retry loop). > > 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. > > > > 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: [email protected] > > Signed-off-by: Sourav Panda <[email protected]> > > --- > > Changes in v5: > > - Replaced defaulting nodemask to &node_states[N_MEMORY] with safely reading > > cpuset_current_mems_allowed using a seqcount retry loop, ensuring fallback > > allocations comply with task hardwall cpusets as suggested by Usama Arif. > > - v4: https://lore.kernel.org/linux-mm/[email protected]/ > > - v3: https://lore.kernel.org/linux-mm/[email protected]/ > > - v2: https://lore.kernel.org/linux-mm/[email protected]/ > > - v1: https://lore.kernel.org/linux-mm/[email protected]/ > > > > mm/hugetlb_cma.c | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/mm/hugetlb_cma.c b/mm/hugetlb_cma.c > > index 39344d6c78d8..3ae9347078e9 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,11 +31,23 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask, > > int node; > > struct folio *folio; > > struct page *page = NULL; > > + nodemask_t local_node_mask; > > > > if (!hugetlb_cma_size) > > return NULL; > > > > - if (hugetlb_cma[nid]) > > + if (!nodemask) { > > + unsigned int cpuset_mems_cookie; > > + > > + do { > > + cpuset_mems_cookie = read_mems_allowed_begin(); > > + local_node_mask = cpuset_current_mems_allowed; > > I think it only makes sense to move cma_alloc_frozen_compound() inside the > loop. Otherwise, during subsequent memory allocations, we could still face > unexpected allocation failures. > > Muchun, > Thanks. Hi Muchun! What do you think of the following diff? This way cma_alloc_frozen_compound is also protected from stale nodemasks. 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,6 +61,9 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask, } } + if (!page && !nodemask && unlikely(read_mems_allowed_retry(cpuset_mems_cookie))) + goto retry_cpuset; + if (!page) return NULL; If this looks good to you, I'll send out v6 with this change. With regards, Sourav Panda > > > + } while (read_mems_allowed_retry(cpuset_mems_cookie)); > > + > > + nodemask = &local_node_mask; > > + } > > + > > + 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 >