The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Muchun Song <muchun.song@linux.dev>
To: Sourav Panda <souravpanda@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	osalvador@suse.de, david@kernel.org, surenb@google.com,
	fvdl@google.com, gthelen@google.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/hugetlb: Fix null pointer dereference of nodemask in hugetlb_cma_alloc_frozen_folio
Date: Sat, 4 Jul 2026 11:04:38 +0800	[thread overview]
Message-ID: <CA20002F-CF0B-4E27-B5F8-EE1D17EDE6F4@linux.dev> (raw)
In-Reply-To: <CANruzcR+uQPisER8KDFH3x4C-O22xvUwQEgv2=TQ=MHooO11Gw@mail.gmail.com>



> 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 :)



  reply	other threads:[~2026-07-04  3:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2026-07-04  3:53       ` Sourav Panda
2026-07-04  4:06         ` Muchun Song

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=CA20002F-CF0B-4E27-B5F8-EE1D17EDE6F4@linux.dev \
    --to=muchun.song@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=fvdl@google.com \
    --cc=gthelen@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=osalvador@suse.de \
    --cc=souravpanda@google.com \
    --cc=surenb@google.com \
    /path/to/YOUR_REPLY

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

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