The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [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