On 09/21/22 01:48, HORIGUCHI NAOYA(堀口 直也) wrote:
> On Fri, Sep 16, 2022 at 02:46:38PM -0700, Mike Kravetz wrote:
> > v1 -> v2
> > - Fixed up head page in error path of __prep_compound_gigantic_page as
> > discovered by Miaohe Lin.
> > - Updated link to Matthew's Allocate and free frozen pages series.
> > - Rebased on next-20220916
> >
> > mm/hugetlb.c | 102 +++++++++++++++++++--------------------------------
> > 1 file changed, 38 insertions(+), 64 deletions(-)
>
> Hello Mike,
>
> I accidentally found a NULL pointer dereference when testing the latest
> mm-unstable, which seems to be caused (or exposed?) by this patch
> (I confirmed that it disappeared by reverting this patch).
>
> It's reproduced by doing like `sysctl vm.nr_hugepages=1000000` to allocate
> hugepages as much as possible.
>
> Could you check that this patch is related to the issue?
>
> Thanks,
> Naoya Horiguchi
>
> ---
> [ 25.634476] BUG: kernel NULL pointer dereference, address: 0000000000000034
> [ 25.635980] #PF: supervisor write access in kernel mode
> [ 25.637283] #PF: error_code(0x0002) - not-present page
> [ 25.638365] PGD 0 P4D 0
> [ 25.638906] Oops: 0002 [#1] PREEMPT SMP PTI
> [ 25.639779] CPU: 4 PID: 819 Comm: sysctl Tainted: G E N 6.0.0-rc3-v6.0-rc1-220920-1758-1398-g2b3f5+ #12
> [ 25.641928] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1.fc35 04/01/2014
> [ 25.643727] RIP: 0010:alloc_buddy_huge_page.isra.0+0x8c/0x140
> [ 25.645071] Code: fe ff 41 83 fc 01 0f 84 54 94 8b 00 41 bc 01 00 00 00 44 89 f7 4c 89 f9 44 89 ea 89 de e8 7c b9 fe ff 48 89 c7 b8 01 00 00 00 <f0> 0f b1 6f 34 66 90 83 f8 01 75 c5 48 85 ff 74 52 65 48 ff 05 03
> [ 25.649006] RSP: 0018:ffffaa7181fffc18 EFLAGS: 00010286
> [ 25.650215] RAX: 0000000000000001 RBX: 0000000000000009 RCX: 0000000000000009
> [ 25.651672] RDX: ffffffffae3b6df0 RSI: ffffffffae8f7ce0 RDI: 0000000000000000
> [ 25.653115] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000c01
> [ 25.654579] R10: 0000000000000f90 R11: 0000000000000000 R12: 0000000000000002
> [ 25.656176] R13: 0000000000000000 R14: 0000000000346cca R15: ffffffffae8f7ce0
> [ 25.657637] FS: 00007f9252f2a740(0000) GS:ffff98cebbc00000(0000) knlGS:0000000000000000
> [ 25.659292] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 25.660469] CR2: 0000000000000034 CR3: 000000014924c004 CR4: 0000000000170ee0
> [ 25.661928] Call Trace:
> [ 25.662469] <TASK>
> [ 25.662927] alloc_fresh_huge_page+0x16f/0x1d0
> [ 25.663859] alloc_pool_huge_page+0x6d/0xb0
> [ 25.664734] __nr_hugepages_store_common+0x189/0x3e0
> [ 25.665764] ? __do_proc_doulongvec_minmax+0x31f/0x340
> [ 25.666832] hugetlb_sysctl_handler_common+0xbf/0xd0
> [ 25.667861] ? hugetlb_register_node+0xe0/0xe0
> [ 25.668786] proc_sys_call_handler+0x196/0x2b0
> [ 25.669724] vfs_write+0x29b/0x3a0
> [ 25.670454] ksys_write+0x4f/0xd0
> [ 25.671153] do_syscall_64+0x3b/0x90
> [ 25.671909] entry_SYSCALL_64_after_hwframe+0x63/0xcd
> [ 25.672958] RIP: 0033:0x7f9252d3e727
> [ 25.673712] Code: 0b 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
> [ 25.677470] RSP: 002b:00007ffcdf9904a8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> [ 25.679002] RAX: ffffffffffffffda RBX: 000055c6ae683210 RCX: 00007f9252d3e727
> [ 25.680456] RDX: 0000000000000006 RSI: 000055c6ae683250 RDI: 0000000000000003
> [ 25.681910] RBP: 000055c6ae685380 R08: 0000000000000003 R09: 0000000000000077
> [ 25.683373] R10: 000000000000006b R11: 0000000000000246 R12: 0000000000000006
> [ 25.684824] R13: 0000000000000006 R14: 0000000000000006 R15: 00007f9252df59e0
> [ 25.686293] </TASK>
Hello Naoya,
My bad for an obvious mistake! Note this change in the patch,
> @@ -1951,7 +1953,21 @@ static struct page *alloc_buddy_huge_page(struct hstate *h,
> gfp_mask |= __GFP_RETRY_MAYFAIL;
> if (nid == NUMA_NO_NODE)
> nid = numa_mem_id();
> +retry:
> page = __alloc_pages(gfp_mask, order, nid, nmask);
> +
> + /* Freeze head page */
> + if (!page_ref_freeze(page, 1)) {
> + __free_pages(page, order);
> + if (retry) { /* retry once */
> + retry = false;
> + goto retry;
> + }
> + /* WOW! twice in a row. */
> + pr_warn("HugeTLB head page unexpected inflated ref count\n");
> + page = NULL;
> + }
> +
> if (page)
> __count_vm_event(HTLB_BUDDY_PGALLOC);
> else
It does not check for a successful return from __alloc_pages before trying to
freeze 'page'. It should obviously check for page == NULL before trying to
freeze with something like this.
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index db90658db171..a092dd639687 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1961,7 +1961,7 @@ static struct page *alloc_buddy_huge_page(struct hstate *h,
page = __alloc_pages(gfp_mask, order, nid, nmask);
/* Freeze head page */
- if (!page_ref_freeze(page, 1)) {
+ if (page && !page_ref_freeze(page, 1)) {
__free_pages(page, order);
if (retry) { /* retry once */
retry = false;
I will send out a v3 (my) tomorrow.
--
Mike Kravetz
