From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-170.mta0.migadu.com (out-170.mta0.migadu.com [91.218.175.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CF2623815FA for ; Sat, 4 Jul 2026 04:07:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.170 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783138026; cv=none; b=BHs62WwNf0MdzvrdvEDCsP69TaIJyfiMU1fS7oNRU23HqXR6a0jkuolpAbcYY1UlElivo5wIG/BXWFIMoxWGLlS0Bpo9ETOv/A0u/NMd1BxSp3dbsrZaW2yVdUezrkT6zHpBP+okXQxEO4b6cJ1AxhQ47Tp+v6/nsl3eNYHmTyk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783138026; c=relaxed/simple; bh=niosObCxYAEcfbXPssoFHuw9okgxbmuiLw+d/w2a4m4=; h=Content-Type:Mime-Version:Subject:From:In-Reply-To:Date:Cc: Message-Id:References:To; b=gq6yxKOEZxbE/jikInjT8rsF5cEsv0HI//vVAHTSONp2/i59ilStxVzM/J5vaiZpUVeclorRBYDUMotiQWXirnsGY57jB2ylCz1sN3ptZboWeeei6mJ50y0s38GvIBwwr75DcGawLMHR2dzO88H+9chWaztp3TBC1HvcIy6xRWM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=vFxjSyhd; arc=none smtp.client-ip=91.218.175.170 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="vFxjSyhd" Content-Type: text/plain; charset=utf-8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1783138018; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vO176FQV9ceY4FDmJoLUkoKYNJ33TI8Fa25LDwf4kHU=; b=vFxjSyhdIMNtyRLvcDEPPiG52orQi6gItBGyYbXVdZFm3Yhd6+bs95F3ieu768mVarYn9G pO4yW+N5PtvCG062sGJwYg+TM9fpzzgg/3jxEYaxAAefSIfMzuZyKmnR+qUEGhPq7B9VaH 9Y7oNzPly4ykJ5kxKuUkc6RDUqSZKAs= Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.600.51.1.1\)) Subject: Re: [PATCH] mm/hugetlb: Fix null pointer dereference of nodemask in hugetlb_cma_alloc_frozen_folio X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Muchun Song In-Reply-To: Date: Sat, 4 Jul 2026 12:06:01 +0800 Cc: Andrew Morton , osalvador@suse.de, david@kernel.org, surenb@google.com, fvdl@google.com, gthelen@google.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Message-Id: <9B0D2555-ED82-4D65-8A83-7F960EFCE601@linux.dev> References: <20260702215713.627941-1-souravpanda@google.com> <20260702162415.142571f4d8c43aa4b95b9f7c@linux-foundation.org> To: Sourav Panda X-Migadu-Flow: FLOW_OUT > On Jul 4, 2026, at 11:53, Sourav Panda wrote: >=20 > On Fri, Jul 3, 2026 at 8:05=E2=80=AFPM Muchun Song = wrote: >>=20 >>=20 >>=20 >>> On Jul 4, 2026, at 04:04, Sourav Panda = wrote: >>>=20 >>> On Thu, Jul 2, 2026 at 4:24=E2=80=AFPM Andrew Morton = wrote: >>>>=20 >>>> On Thu, 2 Jul 2026 21:57:13 +0000 Sourav Panda = wrote: >>>>=20 >>>>> 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. >>>>>=20 >>>>=20 >>>> oh. >>>>=20 >>>>> Fix this by checking if nodemask is NULL and defaulting to >>>>> node_states[N_MEMORY] if it is. >>>>>=20 >>>>> --- 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; >>>>>=20 >>>>> + if (!nodemask) >>>>> + nodemask =3D &node_states[N_MEMORY]; >>>>> + >>>>> if (hugetlb_cma[nid]) >>>>> page =3D cma_alloc_frozen_compound(hugetlb_cma[nid], = order); >>>>=20 >>>> It is possible to hit this with any known testcase? >>>>=20 >>>> If not, why not. I smell the smell of dead code somewhere? >>>>=20 >>>=20 >>> Currently, there are no selftests in the tree that trigger this >>> specific MPOL_PREFERRED_MANY fallback path, which is why it has = gone >>> unnoticed. >>>=20 >>> 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. >>>=20 >>> 1. The Reproducer: >>> #include >>> #include >>> #include >>> #include >>> #include >>> #include >>> #include >>>=20 >>> #define GIGANTIC_PAGE_SIZE (1UL << 30) // 1GB >>>=20 >>> int main(void) { >>> void *ptr; >>> unsigned long nodemask =3D 1; // Preferred Node 0 >>> int ret; >>>=20 >>> /* Allocate 1GB gigantic hugepage area without reserving */ >>> ptr =3D mmap(NULL, GIGANTIC_PAGE_SIZE, PROT_READ | PROT_WRITE, >>> MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB | >>> MAP_HUGE_1GB | MAP_NORESERVE, -1, 0); >>>=20 >>> if (ptr =3D=3D MAP_FAILED) { >>> perror("mmap failed"); >>> return 1; >>> } >>>=20 >>> /* Set MPOL_PREFERRED_MANY policy (mode 5) restricted to node = 0 */ >>> ret =3D mbind(ptr, GIGANTIC_PAGE_SIZE, 5 /* = MPOL_PREFERRED_MANY */, >>> &nodemask, sizeof(nodemask) * 8, 0); >>>=20 >>> if (ret < 0) { >>> perror("mbind"); >>> return 1; >>> } >>>=20 >>> /* Trigger page fault */ >>> memset(ptr, 0, GIGANTIC_PAGE_SIZE); >>>=20 >>> printf("PASS\n"); >>> return 0; >>> } >>>=20 >>> 2. Triggering the panic in QEMU: >>> CONFIG_CMA=3Dy >>> CONFIG_HUGETLB_CMA=3Dy >>>=20 >>> Then, boot a VM with two NUMA nodes, restricting CMA to Node 1, and >>> run the reproducer (+pdpe1gb is required). >>>=20 >>> vng -v \ >>> --cpus 2 \ >>> --memory 8G \ >>> --user=3Droot \ >>> --qemu-opts=3D" \ >>> -cpu max,+pdpe1gb \ >>> -object memory-backend-ram,id=3Dmem0,size=3D4G \ >>> -numa node,nodeid=3D0,memdev=3Dmem0 \ >>> -object memory-backend-ram,id=3Dmem1,size=3D4G \ >>> -numa node,nodeid=3D1,memdev=3Dmem1" \ >>> --append "hugetlb_cma=3D1:1G hugetlb_cma_only=3D1 >>> default_hugepagesz=3D1G hugepagesz=3D1G hugepages=3D0" >>>=20 >>> Inside the VM: >>> gcc reproducer.c -lnuma -o reproducer >>> echo 1 > /proc/sys/vm/nr_overcommit_hugepages >>> ./reproducer >>>=20 >>> 3. Outcome: >>>=20 >>> [ 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] >>> [ 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 >>> ... >>>=20 >>>>=20 >>>> Sashiko said things: >>>> = https://sashiko.dev/#/patchset/20260702215713.627941-1-souravpanda@google.= com >>>>=20 >>>=20 >>> 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. >>>=20 >>> I will apply &cpuset_current_mems_allowed instead of >>> &node_states[N_MEMORY] in v2. >>=20 >> Hi Sourav, >>=20 >> 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. >>=20 >> Muchun, >> Thanks. >=20 > Hi Muchun, Hi, >=20 > Thanks for the review. I completely agree with you :) >=20 > Here is the planned diff for v2: >=20 > 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; >=20 > + if (!nmask) > + nmask =3D &cpuset_current_mems_allowed; > + > folio =3D 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; >=20 > - if (hugetlb_cma[nid]) > + if (hugetlb_cma[nid] && node_isset(nid, *nodemask)) > page =3D cma_alloc_frozen_compound(hugetlb_cma[nid], = order); LGTM. Please send a v2 version. Thanks for your work. >=20 >>=20 >>>=20 >>> The other issue Sashiko highlighted is also valid (but unrelated to >>> this patch) and I can bundle a fix for it in v2. >>>=20 >>> Essentially the preferred nid needs to be extended with a >>> node_isset(nid, *nodemask) check. >>>=20 >>>>=20 >>>> 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. >>>>=20 >>>> 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. >>>>=20 >>>=20 >>> Makes sense! Thanks for connecting the dots :) >>=20 >>=20