* [PATCH] mm/hugetlb: Fix null pointer dereference of nodemask in hugetlb_cma_alloc_frozen_folio @ 2026-07-02 21:57 Sourav Panda 2026-07-02 23:24 ` Andrew Morton 0 siblings, 1 reply; 6+ messages in thread From: Sourav Panda @ 2026-07-02 21:57 UTC (permalink / raw) To: muchun.song, osalvador, akpm Cc: david, surenb, fvdl, gthelen, souravpanda, linux-mm, linux-kernel alloc_buddy_hugetlb_folio_with_mpol() can pass a NULL nodemask to hugetlb_cma_alloc_frozen_folio() as a fallback to allocate from all nodes. In this case, hugetlb_cma_alloc_frozen_folio() blindly dereferences it in for_each_node_mask(), leading to a null pointer dereference. Fix this by checking if nodemask is NULL and defaulting to node_states[N_MEMORY] if it is. Signed-off-by: Sourav Panda <souravpanda@google.com> --- mm/hugetlb_cma.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mm/hugetlb_cma.c b/mm/hugetlb_cma.c index 39344d6c78d8..563e11e607e5 100644 --- a/mm/hugetlb_cma.c +++ b/mm/hugetlb_cma.c @@ -34,6 +34,9 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask, if (!hugetlb_cma_size) return NULL; + if (!nodemask) + nodemask = &node_states[N_MEMORY]; + if (hugetlb_cma[nid]) page = cma_alloc_frozen_compound(hugetlb_cma[nid], order); -- 2.55.0.rc0.799.gd6f94ed593-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/hugetlb: Fix null pointer dereference of nodemask in hugetlb_cma_alloc_frozen_folio 2026-07-02 21:57 [PATCH] mm/hugetlb: Fix null pointer dereference of nodemask in hugetlb_cma_alloc_frozen_folio Sourav Panda @ 2026-07-02 23:24 ` Andrew Morton 2026-07-03 20:04 ` Sourav Panda 0 siblings, 1 reply; 6+ messages in thread From: Andrew Morton @ 2026-07-02 23:24 UTC (permalink / raw) To: Sourav Panda Cc: muchun.song, osalvador, david, surenb, fvdl, gthelen, linux-mm, linux-kernel On Thu, 2 Jul 2026 21:57:13 +0000 Sourav Panda <souravpanda@google.com> wrote: > alloc_buddy_hugetlb_folio_with_mpol() can pass a NULL nodemask to > hugetlb_cma_alloc_frozen_folio() as a fallback to allocate from all > nodes. In this case, hugetlb_cma_alloc_frozen_folio() blindly > dereferences it in for_each_node_mask(), leading to a null pointer > dereference. > oh. > Fix this by checking if nodemask is NULL and defaulting to > node_states[N_MEMORY] if it is. > > --- a/mm/hugetlb_cma.c > +++ b/mm/hugetlb_cma.c > @@ -34,6 +34,9 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask, > if (!hugetlb_cma_size) > return NULL; > > + if (!nodemask) > + nodemask = &node_states[N_MEMORY]; > + > if (hugetlb_cma[nid]) > page = cma_alloc_frozen_compound(hugetlb_cma[nid], order); It is possible to hit this with any known testcase? If not, why not. I smell the smell of dead code somewhere? Sashiko said things: https://sashiko.dev/#/patchset/20260702215713.627941-1-souravpanda@google.com Ackerly's "mm: hugetlb: move mpol interpretation out of alloc_buddy_hugetlb_folio_with_mpol()" made big changes to alloc_buddy_hugetlb_folio_with_mpol(): https://lore.kernel.org/20260702-hugetlb-open-up-v4-2-d53cefcccf34@google.com. If this bug is real then it would be better to stage your fix ahead of Ackerly's series. Possibly with a cc:stable. Then I can redo Ackerly's patch on top and we'll need to check that this bug isn't reintroduced. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/hugetlb: Fix null pointer dereference of nodemask in hugetlb_cma_alloc_frozen_folio 2026-07-02 23:24 ` Andrew Morton @ 2026-07-03 20:04 ` Sourav Panda 2026-07-04 3:04 ` Muchun Song 0 siblings, 1 reply; 6+ messages in thread From: Sourav Panda @ 2026-07-03 20:04 UTC (permalink / raw) To: Andrew Morton Cc: muchun.song, osalvador, david, surenb, fvdl, gthelen, linux-mm, linux-kernel On Thu, Jul 2, 2026 at 4:24 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > On Thu, 2 Jul 2026 21:57:13 +0000 Sourav Panda <souravpanda@google.com> wrote: > > > alloc_buddy_hugetlb_folio_with_mpol() can pass a NULL nodemask to > > hugetlb_cma_alloc_frozen_folio() as a fallback to allocate from all > > nodes. In this case, hugetlb_cma_alloc_frozen_folio() blindly > > dereferences it in for_each_node_mask(), leading to a null pointer > > dereference. > > > > oh. > > > Fix this by checking if nodemask is NULL and defaulting to > > node_states[N_MEMORY] if it is. > > > > --- a/mm/hugetlb_cma.c > > +++ b/mm/hugetlb_cma.c > > @@ -34,6 +34,9 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask, > > if (!hugetlb_cma_size) > > return NULL; > > > > + if (!nodemask) > > + nodemask = &node_states[N_MEMORY]; > > + > > if (hugetlb_cma[nid]) > > page = cma_alloc_frozen_compound(hugetlb_cma[nid], order); > > It is possible to hit this with any known testcase? > > If not, why not. I smell the smell of dead code somewhere? > Currently, there are no selftests in the tree that trigger this specific MPOL_PREFERRED_MANY fallback path, which is why it has gone unnoticed. The bug only triggers under a specific topology: Multiple NUMA nodes where gigantic hugepages are backed by CMA, but only on a subset of the nodes. 1. The Reproducer: #include <stdio.h> #include <stdlib.h> #include <numaif.h> #include <numa.h> #include <sys/mman.h> #include <unistd.h> #include <string.h> #define GIGANTIC_PAGE_SIZE (1UL << 30) // 1GB int main(void) { void *ptr; unsigned long nodemask = 1; // Preferred Node 0 int ret; /* Allocate 1GB gigantic hugepage area without reserving */ ptr = mmap(NULL, GIGANTIC_PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB | MAP_HUGE_1GB | MAP_NORESERVE, -1, 0); if (ptr == MAP_FAILED) { perror("mmap failed"); return 1; } /* Set MPOL_PREFERRED_MANY policy (mode 5) restricted to node 0 */ ret = mbind(ptr, GIGANTIC_PAGE_SIZE, 5 /* MPOL_PREFERRED_MANY */, &nodemask, sizeof(nodemask) * 8, 0); if (ret < 0) { perror("mbind"); return 1; } /* Trigger page fault */ memset(ptr, 0, GIGANTIC_PAGE_SIZE); printf("PASS\n"); return 0; } 2. Triggering the panic in QEMU: CONFIG_CMA=y CONFIG_HUGETLB_CMA=y Then, boot a VM with two NUMA nodes, restricting CMA to Node 1, and run the reproducer (+pdpe1gb is required). vng -v \ --cpus 2 \ --memory 8G \ --user=root \ --qemu-opts=" \ -cpu max,+pdpe1gb \ -object memory-backend-ram,id=mem0,size=4G \ -numa node,nodeid=0,memdev=mem0 \ -object memory-backend-ram,id=mem1,size=4G \ -numa node,nodeid=1,memdev=mem1" \ --append "hugetlb_cma=1:1G hugetlb_cma_only=1 default_hugepagesz=1G hugepagesz=1G hugepages=0" Inside the VM: gcc reproducer.c -lnuma -o reproducer echo 1 > /proc/sys/vm/nr_overcommit_hugepages ./reproducer 3. Outcome: [ 33.586151] BUG: kernel NULL pointer dereference, address: 0000000000000000 [ 33.586512] #PF: supervisor read access in kernel mode [ 33.586680] #PF: error_code(0x0000) - not-present page [ 33.586879] PGD 1023c7067 P4D 0 [ 33.587668] Oops: Oops: 0000 [#1] SMP NOPTI [ 33.588265] CPU: 0 UID: 0 PID: 336 Comm: reproducer Not tainted 7.1.0-virtme #5 PREEMPT(lazy) ... [ 33.588960] RIP: 0010:hugetlb_cma_alloc_frozen_folio+0x75/0x120 ... [ 33.592469] Call Trace: [ 33.592672] <TASK> [ 33.592853] only_alloc_fresh_hugetlb_folio.isra.0+0x2c/0x160 [ 33.593127] alloc_surplus_hugetlb_folio+0x6d/0x100 [ 33.593284] alloc_hugetlb_folio+0x3c5/0x660 [ 33.593430] hugetlb_no_page+0x3d9/0x650 ... > > Sashiko said things: > https://sashiko.dev/#/patchset/20260702215713.627941-1-souravpanda@google.com > Sashiko is right to highlight the following: By defaulting to node_states[N_MEMORY] instead of cpuset_current_mems_allowed, it appears the allocation loop could search all system nodes, breaking NUMA isolation for containers. I will apply &cpuset_current_mems_allowed instead of &node_states[N_MEMORY] in v2. The other issue Sashiko highlighted is also valid (but unrelated to this patch) and I can bundle a fix for it in v2. Essentially the preferred nid needs to be extended with a node_isset(nid, *nodemask) check. > > Ackerly's "mm: hugetlb: move mpol interpretation out of > alloc_buddy_hugetlb_folio_with_mpol()" made big changes to > alloc_buddy_hugetlb_folio_with_mpol(): > https://lore.kernel.org/20260702-hugetlb-open-up-v4-2-d53cefcccf34@google.com. > > If this bug is real then it would be better to stage your fix ahead of > Ackerly's series. Possibly with a cc:stable. Then I can redo > Ackerly's patch on top and we'll need to check that this bug isn't > reintroduced. > Makes sense! Thanks for connecting the dots :) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/hugetlb: Fix null pointer dereference of nodemask in hugetlb_cma_alloc_frozen_folio 2026-07-03 20:04 ` Sourav Panda @ 2026-07-04 3:04 ` Muchun Song 2026-07-04 3:53 ` Sourav Panda 0 siblings, 1 reply; 6+ messages in thread From: Muchun Song @ 2026-07-04 3:04 UTC (permalink / raw) To: Sourav Panda Cc: Andrew Morton, osalvador, david, surenb, fvdl, gthelen, linux-mm, linux-kernel > On Jul 4, 2026, at 04:04, Sourav Panda <souravpanda@google.com> wrote: > > On Thu, Jul 2, 2026 at 4:24 PM Andrew Morton <akpm@linux-foundation.org> wrote: >> >> On Thu, 2 Jul 2026 21:57:13 +0000 Sourav Panda <souravpanda@google.com> wrote: >> >>> alloc_buddy_hugetlb_folio_with_mpol() can pass a NULL nodemask to >>> hugetlb_cma_alloc_frozen_folio() as a fallback to allocate from all >>> nodes. In this case, hugetlb_cma_alloc_frozen_folio() blindly >>> dereferences it in for_each_node_mask(), leading to a null pointer >>> dereference. >>> >> >> oh. >> >>> Fix this by checking if nodemask is NULL and defaulting to >>> node_states[N_MEMORY] if it is. >>> >>> --- a/mm/hugetlb_cma.c >>> +++ b/mm/hugetlb_cma.c >>> @@ -34,6 +34,9 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask, >>> if (!hugetlb_cma_size) >>> return NULL; >>> >>> + if (!nodemask) >>> + nodemask = &node_states[N_MEMORY]; >>> + >>> if (hugetlb_cma[nid]) >>> page = cma_alloc_frozen_compound(hugetlb_cma[nid], order); >> >> It is possible to hit this with any known testcase? >> >> If not, why not. I smell the smell of dead code somewhere? >> > > Currently, there are no selftests in the tree that trigger this > specific MPOL_PREFERRED_MANY fallback path, which is why it has gone > unnoticed. > > The bug only triggers under a specific topology: Multiple NUMA nodes > where gigantic hugepages are backed by CMA, but only on a subset of > the nodes. > > 1. The Reproducer: > #include <stdio.h> > #include <stdlib.h> > #include <numaif.h> > #include <numa.h> > #include <sys/mman.h> > #include <unistd.h> > #include <string.h> > > #define GIGANTIC_PAGE_SIZE (1UL << 30) // 1GB > > int main(void) { > void *ptr; > unsigned long nodemask = 1; // Preferred Node 0 > int ret; > > /* Allocate 1GB gigantic hugepage area without reserving */ > ptr = mmap(NULL, GIGANTIC_PAGE_SIZE, PROT_READ | PROT_WRITE, > MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB | > MAP_HUGE_1GB | MAP_NORESERVE, -1, 0); > > if (ptr == MAP_FAILED) { > perror("mmap failed"); > return 1; > } > > /* Set MPOL_PREFERRED_MANY policy (mode 5) restricted to node 0 */ > ret = mbind(ptr, GIGANTIC_PAGE_SIZE, 5 /* MPOL_PREFERRED_MANY */, > &nodemask, sizeof(nodemask) * 8, 0); > > if (ret < 0) { > perror("mbind"); > return 1; > } > > /* Trigger page fault */ > memset(ptr, 0, GIGANTIC_PAGE_SIZE); > > printf("PASS\n"); > return 0; > } > > 2. Triggering the panic in QEMU: > CONFIG_CMA=y > CONFIG_HUGETLB_CMA=y > > Then, boot a VM with two NUMA nodes, restricting CMA to Node 1, and > run the reproducer (+pdpe1gb is required). > > vng -v \ > --cpus 2 \ > --memory 8G \ > --user=root \ > --qemu-opts=" \ > -cpu max,+pdpe1gb \ > -object memory-backend-ram,id=mem0,size=4G \ > -numa node,nodeid=0,memdev=mem0 \ > -object memory-backend-ram,id=mem1,size=4G \ > -numa node,nodeid=1,memdev=mem1" \ > --append "hugetlb_cma=1:1G hugetlb_cma_only=1 > default_hugepagesz=1G hugepagesz=1G hugepages=0" > > Inside the VM: > gcc reproducer.c -lnuma -o reproducer > echo 1 > /proc/sys/vm/nr_overcommit_hugepages > ./reproducer > > 3. Outcome: > > [ 33.586151] BUG: kernel NULL pointer dereference, address: > 0000000000000000 > [ 33.586512] #PF: supervisor read access in kernel mode > [ 33.586680] #PF: error_code(0x0000) - not-present page > [ 33.586879] PGD 1023c7067 P4D 0 > [ 33.587668] Oops: Oops: 0000 [#1] SMP NOPTI > [ 33.588265] CPU: 0 UID: 0 PID: 336 Comm: reproducer Not tainted > 7.1.0-virtme #5 PREEMPT(lazy) > ... > [ 33.588960] RIP: 0010:hugetlb_cma_alloc_frozen_folio+0x75/0x120 > ... > [ 33.592469] Call Trace: > [ 33.592672] <TASK> > [ 33.592853] only_alloc_fresh_hugetlb_folio.isra.0+0x2c/0x160 > [ 33.593127] alloc_surplus_hugetlb_folio+0x6d/0x100 > [ 33.593284] alloc_hugetlb_folio+0x3c5/0x660 > [ 33.593430] hugetlb_no_page+0x3d9/0x650 > ... > >> >> Sashiko said things: >> https://sashiko.dev/#/patchset/20260702215713.627941-1-souravpanda@google.com >> > > Sashiko is right to highlight the following: By defaulting to > node_states[N_MEMORY] instead of > cpuset_current_mems_allowed, it appears the allocation loop could search > all system nodes, breaking NUMA isolation for containers. > > I will apply &cpuset_current_mems_allowed instead of > &node_states[N_MEMORY] in v2. Hi Sourav, Thanks for your report. However, I suggest fixing it in alloc_fresh_hugetlb_folio(), because we also need a right node_mask for alloc_contig_frozen_pages() case. Muchun, Thanks. > > The other issue Sashiko highlighted is also valid (but unrelated to > this patch) and I can bundle a fix for it in v2. > > Essentially the preferred nid needs to be extended with a > node_isset(nid, *nodemask) check. > >> >> Ackerly's "mm: hugetlb: move mpol interpretation out of >> alloc_buddy_hugetlb_folio_with_mpol()" made big changes to >> alloc_buddy_hugetlb_folio_with_mpol(): >> https://lore.kernel.org/20260702-hugetlb-open-up-v4-2-d53cefcccf34@google.com. >> >> If this bug is real then it would be better to stage your fix ahead of >> Ackerly's series. Possibly with a cc:stable. Then I can redo >> Ackerly's patch on top and we'll need to check that this bug isn't >> reintroduced. >> > > Makes sense! Thanks for connecting the dots :) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/hugetlb: Fix null pointer dereference of nodemask in hugetlb_cma_alloc_frozen_folio 2026-07-04 3:04 ` Muchun Song @ 2026-07-04 3:53 ` Sourav Panda 2026-07-04 4:06 ` Muchun Song 0 siblings, 1 reply; 6+ messages in thread From: Sourav Panda @ 2026-07-04 3:53 UTC (permalink / raw) To: Muchun Song Cc: Andrew Morton, osalvador, david, surenb, fvdl, gthelen, linux-mm, linux-kernel On Fri, Jul 3, 2026 at 8:05 PM Muchun Song <muchun.song@linux.dev> wrote: > > > > > On Jul 4, 2026, at 04:04, Sourav Panda <souravpanda@google.com> wrote: > > > > On Thu, Jul 2, 2026 at 4:24 PM Andrew Morton <akpm@linux-foundation.org> wrote: > >> > >> On Thu, 2 Jul 2026 21:57:13 +0000 Sourav Panda <souravpanda@google.com> wrote: > >> > >>> alloc_buddy_hugetlb_folio_with_mpol() can pass a NULL nodemask to > >>> hugetlb_cma_alloc_frozen_folio() as a fallback to allocate from all > >>> nodes. In this case, hugetlb_cma_alloc_frozen_folio() blindly > >>> dereferences it in for_each_node_mask(), leading to a null pointer > >>> dereference. > >>> > >> > >> oh. > >> > >>> Fix this by checking if nodemask is NULL and defaulting to > >>> node_states[N_MEMORY] if it is. > >>> > >>> --- a/mm/hugetlb_cma.c > >>> +++ b/mm/hugetlb_cma.c > >>> @@ -34,6 +34,9 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask, > >>> if (!hugetlb_cma_size) > >>> return NULL; > >>> > >>> + if (!nodemask) > >>> + nodemask = &node_states[N_MEMORY]; > >>> + > >>> if (hugetlb_cma[nid]) > >>> page = cma_alloc_frozen_compound(hugetlb_cma[nid], order); > >> > >> It is possible to hit this with any known testcase? > >> > >> If not, why not. I smell the smell of dead code somewhere? > >> > > > > Currently, there are no selftests in the tree that trigger this > > specific MPOL_PREFERRED_MANY fallback path, which is why it has gone > > unnoticed. > > > > The bug only triggers under a specific topology: Multiple NUMA nodes > > where gigantic hugepages are backed by CMA, but only on a subset of > > the nodes. > > > > 1. The Reproducer: > > #include <stdio.h> > > #include <stdlib.h> > > #include <numaif.h> > > #include <numa.h> > > #include <sys/mman.h> > > #include <unistd.h> > > #include <string.h> > > > > #define GIGANTIC_PAGE_SIZE (1UL << 30) // 1GB > > > > int main(void) { > > void *ptr; > > unsigned long nodemask = 1; // Preferred Node 0 > > int ret; > > > > /* Allocate 1GB gigantic hugepage area without reserving */ > > ptr = mmap(NULL, GIGANTIC_PAGE_SIZE, PROT_READ | PROT_WRITE, > > MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB | > > MAP_HUGE_1GB | MAP_NORESERVE, -1, 0); > > > > if (ptr == MAP_FAILED) { > > perror("mmap failed"); > > return 1; > > } > > > > /* Set MPOL_PREFERRED_MANY policy (mode 5) restricted to node 0 */ > > ret = mbind(ptr, GIGANTIC_PAGE_SIZE, 5 /* MPOL_PREFERRED_MANY */, > > &nodemask, sizeof(nodemask) * 8, 0); > > > > if (ret < 0) { > > perror("mbind"); > > return 1; > > } > > > > /* Trigger page fault */ > > memset(ptr, 0, GIGANTIC_PAGE_SIZE); > > > > printf("PASS\n"); > > return 0; > > } > > > > 2. Triggering the panic in QEMU: > > CONFIG_CMA=y > > CONFIG_HUGETLB_CMA=y > > > > Then, boot a VM with two NUMA nodes, restricting CMA to Node 1, and > > run the reproducer (+pdpe1gb is required). > > > > vng -v \ > > --cpus 2 \ > > --memory 8G \ > > --user=root \ > > --qemu-opts=" \ > > -cpu max,+pdpe1gb \ > > -object memory-backend-ram,id=mem0,size=4G \ > > -numa node,nodeid=0,memdev=mem0 \ > > -object memory-backend-ram,id=mem1,size=4G \ > > -numa node,nodeid=1,memdev=mem1" \ > > --append "hugetlb_cma=1:1G hugetlb_cma_only=1 > > default_hugepagesz=1G hugepagesz=1G hugepages=0" > > > > Inside the VM: > > gcc reproducer.c -lnuma -o reproducer > > echo 1 > /proc/sys/vm/nr_overcommit_hugepages > > ./reproducer > > > > 3. Outcome: > > > > [ 33.586151] BUG: kernel NULL pointer dereference, address: > > 0000000000000000 > > [ 33.586512] #PF: supervisor read access in kernel mode > > [ 33.586680] #PF: error_code(0x0000) - not-present page > > [ 33.586879] PGD 1023c7067 P4D 0 > > [ 33.587668] Oops: Oops: 0000 [#1] SMP NOPTI > > [ 33.588265] CPU: 0 UID: 0 PID: 336 Comm: reproducer Not tainted > > 7.1.0-virtme #5 PREEMPT(lazy) > > ... > > [ 33.588960] RIP: 0010:hugetlb_cma_alloc_frozen_folio+0x75/0x120 > > ... > > [ 33.592469] Call Trace: > > [ 33.592672] <TASK> > > [ 33.592853] only_alloc_fresh_hugetlb_folio.isra.0+0x2c/0x160 > > [ 33.593127] alloc_surplus_hugetlb_folio+0x6d/0x100 > > [ 33.593284] alloc_hugetlb_folio+0x3c5/0x660 > > [ 33.593430] hugetlb_no_page+0x3d9/0x650 > > ... > > > >> > >> Sashiko said things: > >> https://sashiko.dev/#/patchset/20260702215713.627941-1-souravpanda@google.com > >> > > > > Sashiko is right to highlight the following: By defaulting to > > node_states[N_MEMORY] instead of > > cpuset_current_mems_allowed, it appears the allocation loop could search > > all system nodes, breaking NUMA isolation for containers. > > > > I will apply &cpuset_current_mems_allowed instead of > > &node_states[N_MEMORY] in v2. > > Hi Sourav, > > Thanks for your report. However, I suggest fixing it in alloc_fresh_hugetlb_folio(), > because we also need a right node_mask for alloc_contig_frozen_pages() case. > > Muchun, > Thanks. Hi Muchun, Thanks for the review. I completely agree with you :) Here is the planned diff for v2: diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 571212b80835..ab5deba4f7a1 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -1865,6 +1865,9 @@ static struct folio *alloc_fresh_hugetlb_folio(struct hstate *h, { struct folio *folio; + if (!nmask) + nmask = &cpuset_current_mems_allowed; + folio = only_alloc_fresh_hugetlb_folio(h, gfp_mask, nid, nmask, NULL); if (folio) hugetlb_vmemmap_optimize_folio(h, 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); > > > > > The other issue Sashiko highlighted is also valid (but unrelated to > > this patch) and I can bundle a fix for it in v2. > > > > Essentially the preferred nid needs to be extended with a > > node_isset(nid, *nodemask) check. > > > >> > >> Ackerly's "mm: hugetlb: move mpol interpretation out of > >> alloc_buddy_hugetlb_folio_with_mpol()" made big changes to > >> alloc_buddy_hugetlb_folio_with_mpol(): > >> https://lore.kernel.org/20260702-hugetlb-open-up-v4-2-d53cefcccf34@google.com. > >> > >> If this bug is real then it would be better to stage your fix ahead of > >> Ackerly's series. Possibly with a cc:stable. Then I can redo > >> Ackerly's patch on top and we'll need to check that this bug isn't > >> reintroduced. > >> > > > > Makes sense! Thanks for connecting the dots :) > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/hugetlb: Fix null pointer dereference of nodemask in hugetlb_cma_alloc_frozen_folio 2026-07-04 3:53 ` Sourav Panda @ 2026-07-04 4:06 ` Muchun Song 0 siblings, 0 replies; 6+ messages in thread From: Muchun Song @ 2026-07-04 4:06 UTC (permalink / raw) To: Sourav Panda Cc: Andrew Morton, osalvador, david, surenb, fvdl, gthelen, linux-mm, linux-kernel > On Jul 4, 2026, at 11:53, Sourav Panda <souravpanda@google.com> wrote: > > On Fri, Jul 3, 2026 at 8:05 PM Muchun Song <muchun.song@linux.dev> wrote: >> >> >> >>> On Jul 4, 2026, at 04:04, Sourav Panda <souravpanda@google.com> wrote: >>> >>> On Thu, Jul 2, 2026 at 4:24 PM Andrew Morton <akpm@linux-foundation.org> wrote: >>>> >>>> On Thu, 2 Jul 2026 21:57:13 +0000 Sourav Panda <souravpanda@google.com> wrote: >>>> >>>>> alloc_buddy_hugetlb_folio_with_mpol() can pass a NULL nodemask to >>>>> hugetlb_cma_alloc_frozen_folio() as a fallback to allocate from all >>>>> nodes. In this case, hugetlb_cma_alloc_frozen_folio() blindly >>>>> dereferences it in for_each_node_mask(), leading to a null pointer >>>>> dereference. >>>>> >>>> >>>> oh. >>>> >>>>> Fix this by checking if nodemask is NULL and defaulting to >>>>> node_states[N_MEMORY] if it is. >>>>> >>>>> --- a/mm/hugetlb_cma.c >>>>> +++ b/mm/hugetlb_cma.c >>>>> @@ -34,6 +34,9 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask, >>>>> if (!hugetlb_cma_size) >>>>> return NULL; >>>>> >>>>> + if (!nodemask) >>>>> + nodemask = &node_states[N_MEMORY]; >>>>> + >>>>> if (hugetlb_cma[nid]) >>>>> page = cma_alloc_frozen_compound(hugetlb_cma[nid], order); >>>> >>>> It is possible to hit this with any known testcase? >>>> >>>> If not, why not. I smell the smell of dead code somewhere? >>>> >>> >>> Currently, there are no selftests in the tree that trigger this >>> specific MPOL_PREFERRED_MANY fallback path, which is why it has gone >>> unnoticed. >>> >>> The bug only triggers under a specific topology: Multiple NUMA nodes >>> where gigantic hugepages are backed by CMA, but only on a subset of >>> the nodes. >>> >>> 1. The Reproducer: >>> #include <stdio.h> >>> #include <stdlib.h> >>> #include <numaif.h> >>> #include <numa.h> >>> #include <sys/mman.h> >>> #include <unistd.h> >>> #include <string.h> >>> >>> #define GIGANTIC_PAGE_SIZE (1UL << 30) // 1GB >>> >>> int main(void) { >>> void *ptr; >>> unsigned long nodemask = 1; // Preferred Node 0 >>> int ret; >>> >>> /* Allocate 1GB gigantic hugepage area without reserving */ >>> ptr = mmap(NULL, GIGANTIC_PAGE_SIZE, PROT_READ | PROT_WRITE, >>> MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB | >>> MAP_HUGE_1GB | MAP_NORESERVE, -1, 0); >>> >>> if (ptr == MAP_FAILED) { >>> perror("mmap failed"); >>> return 1; >>> } >>> >>> /* Set MPOL_PREFERRED_MANY policy (mode 5) restricted to node 0 */ >>> ret = mbind(ptr, GIGANTIC_PAGE_SIZE, 5 /* MPOL_PREFERRED_MANY */, >>> &nodemask, sizeof(nodemask) * 8, 0); >>> >>> if (ret < 0) { >>> perror("mbind"); >>> return 1; >>> } >>> >>> /* Trigger page fault */ >>> memset(ptr, 0, GIGANTIC_PAGE_SIZE); >>> >>> printf("PASS\n"); >>> return 0; >>> } >>> >>> 2. Triggering the panic in QEMU: >>> CONFIG_CMA=y >>> CONFIG_HUGETLB_CMA=y >>> >>> Then, boot a VM with two NUMA nodes, restricting CMA to Node 1, and >>> run the reproducer (+pdpe1gb is required). >>> >>> vng -v \ >>> --cpus 2 \ >>> --memory 8G \ >>> --user=root \ >>> --qemu-opts=" \ >>> -cpu max,+pdpe1gb \ >>> -object memory-backend-ram,id=mem0,size=4G \ >>> -numa node,nodeid=0,memdev=mem0 \ >>> -object memory-backend-ram,id=mem1,size=4G \ >>> -numa node,nodeid=1,memdev=mem1" \ >>> --append "hugetlb_cma=1:1G hugetlb_cma_only=1 >>> default_hugepagesz=1G hugepagesz=1G hugepages=0" >>> >>> Inside the VM: >>> gcc reproducer.c -lnuma -o reproducer >>> echo 1 > /proc/sys/vm/nr_overcommit_hugepages >>> ./reproducer >>> >>> 3. Outcome: >>> >>> [ 33.586151] BUG: kernel NULL pointer dereference, address: >>> 0000000000000000 >>> [ 33.586512] #PF: supervisor read access in kernel mode >>> [ 33.586680] #PF: error_code(0x0000) - not-present page >>> [ 33.586879] PGD 1023c7067 P4D 0 >>> [ 33.587668] Oops: Oops: 0000 [#1] SMP NOPTI >>> [ 33.588265] CPU: 0 UID: 0 PID: 336 Comm: reproducer Not tainted >>> 7.1.0-virtme #5 PREEMPT(lazy) >>> ... >>> [ 33.588960] RIP: 0010:hugetlb_cma_alloc_frozen_folio+0x75/0x120 >>> ... >>> [ 33.592469] Call Trace: >>> [ 33.592672] <TASK> >>> [ 33.592853] only_alloc_fresh_hugetlb_folio.isra.0+0x2c/0x160 >>> [ 33.593127] alloc_surplus_hugetlb_folio+0x6d/0x100 >>> [ 33.593284] alloc_hugetlb_folio+0x3c5/0x660 >>> [ 33.593430] hugetlb_no_page+0x3d9/0x650 >>> ... >>> >>>> >>>> Sashiko said things: >>>> https://sashiko.dev/#/patchset/20260702215713.627941-1-souravpanda@google.com >>>> >>> >>> Sashiko is right to highlight the following: By defaulting to >>> node_states[N_MEMORY] instead of >>> cpuset_current_mems_allowed, it appears the allocation loop could search >>> all system nodes, breaking NUMA isolation for containers. >>> >>> I will apply &cpuset_current_mems_allowed instead of >>> &node_states[N_MEMORY] in v2. >> >> Hi Sourav, >> >> Thanks for your report. However, I suggest fixing it in alloc_fresh_hugetlb_folio(), >> because we also need a right node_mask for alloc_contig_frozen_pages() case. >> >> Muchun, >> Thanks. > > Hi Muchun, Hi, > > Thanks for the review. I completely agree with you :) > > Here is the planned diff for v2: > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index 571212b80835..ab5deba4f7a1 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -1865,6 +1865,9 @@ static struct folio > *alloc_fresh_hugetlb_folio(struct hstate *h, > { > struct folio *folio; > > + if (!nmask) > + nmask = &cpuset_current_mems_allowed; > + > folio = only_alloc_fresh_hugetlb_folio(h, gfp_mask, nid, nmask, NULL); > if (folio) > hugetlb_vmemmap_optimize_folio(h, 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); LGTM. Please send a v2 version. Thanks for your work. > >> >>> >>> The other issue Sashiko highlighted is also valid (but unrelated to >>> this patch) and I can bundle a fix for it in v2. >>> >>> Essentially the preferred nid needs to be extended with a >>> node_isset(nid, *nodemask) check. >>> >>>> >>>> Ackerly's "mm: hugetlb: move mpol interpretation out of >>>> alloc_buddy_hugetlb_folio_with_mpol()" made big changes to >>>> alloc_buddy_hugetlb_folio_with_mpol(): >>>> https://lore.kernel.org/20260702-hugetlb-open-up-v4-2-d53cefcccf34@google.com. >>>> >>>> If this bug is real then it would be better to stage your fix ahead of >>>> Ackerly's series. Possibly with a cc:stable. Then I can redo >>>> Ackerly's patch on top and we'll need to check that this bug isn't >>>> reintroduced. >>>> >>> >>> Makes sense! Thanks for connecting the dots :) >> >> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-04 4:07 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-02 21:57 [PATCH] mm/hugetlb: Fix null pointer dereference of nodemask in hugetlb_cma_alloc_frozen_folio Sourav Panda 2026-07-02 23:24 ` Andrew Morton 2026-07-03 20:04 ` Sourav Panda 2026-07-04 3:04 ` Muchun Song 2026-07-04 3:53 ` Sourav Panda 2026-07-04 4:06 ` Muchun Song
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox