From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-out.aladdin-rd.ru (mail-out.aladdin-rd.ru [91.199.251.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DBA213594C for ; Mon, 27 Jan 2025 15:02:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.199.251.16 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737990159; cv=none; b=p0PKCG0H0mRDU2TEikn6BWieKCcv6iJhQpkRLoe2ePPqEgEKhzw3SOOzY4PKvXLk2yO2lc6hJiXjU1qkRHYm4XJF2pkIPtadIA3226VExvvm3eok+onH+7B8HVjkMdCDYtXr5l44aMT8fQiSSlBPowj87HOg20lZ4vClX3Wa+GM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737990159; c=relaxed/simple; bh=h/XFX4xfdAmTSyH8XTy+U36jJ6/rr62SRXECWNYW6Os=; h=From:To:CC:Subject:Date:Message-ID:References:In-Reply-To: Content-Type:MIME-Version; b=CkGBQnLwBzlVnwAIBIEo0iurDELI2nxHppasi9diqbhga4H0VD/8+YyeE6gmp4uyj1MCsFfu7pQj9h1nDllKvO4t1IEfztkpvDnG+jb/u+iBWykmeodt+yHvPKnRWzE51bxkCKPGmaGqTWfdK1Mo6AS506S1vCvEtsoAVd6rx6g= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=aladdin.ru; spf=pass smtp.mailfrom=aladdin.ru; arc=none smtp.client-ip=91.199.251.16 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=aladdin.ru Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=aladdin.ru From: =?koi8-r?B?5NXMz9cg5MHOycnM?= To: Lorenzo Stoakes CC: Andrew Morton , "Liam R. Howlett" , Vlastimil Babka , Jann Horn , "Matthew Wilcox (Oracle)" , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , "lvc-project@linuxtesting.org" , "stable@vger.kernel.com" Subject: RE: [PATCH] mm/vma: Fix hugetlb accounting error in copy_vma() Thread-Topic: [PATCH] mm/vma: Fix hugetlb accounting error in copy_vma() Thread-Index: AQHbcMhQSVzPYt8UC0WR3XfsBPQhEbMqgLMAgAA1bSA= Date: Mon, 27 Jan 2025 15:02:26 +0000 Message-ID: References: <20250127143201.45453-1-d.dulov@aladdin.ru> <83645f1b-cede-455c-abc0-6f105199eee9@lucifer.local> In-Reply-To: <83645f1b-cede-455c-abc0-6f105199eee9@lucifer.local> Accept-Language: ru-RU, en-US Content-Language: ru-RU X-MS-Has-Attach: X-MS-TNEF-Correlator: Content-Type: text/plain; charset="koi8-r" Content-Transfer-Encoding: quoted-printable Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Thank you for the feedback! I'll check the suggested solution ASAP. -----Original Message----- From: Lorenzo Stoakes [mailto:lorenzo.stoakes@oracle.com]=20 Sent: Monday, January 27, 2025 5:47 PM To: =E4=D5=CC=CF=D7 =E4=C1=CE=C9=C9=CC Cc: Andrew Morton ; Liam R. Howlett ; Vlastimil Babka ; Jann Horn ; Matthew Wilcox (Oracle) ; linux-mm@kvack.org; linu= x-kernel@vger.kernel.org; lvc-project@linuxtesting.org; stable@vger.kernel.= com Subject: Re: [PATCH] mm/vma: Fix hugetlb accounting error in copy_vma() Thanks for the report. On Mon, Jan 27, 2025 at 05:32:01PM +0300, Daniil Dulov wrote: > In copy_vma() allocation of maple tree nodes may fail. Since page account= ing > takes place at the close() operation for hugetlb, it is called at the err= or > path against the new_vma to account pages of the vma that was not success= fully > copied and that shares the page_counter with the original vma. Then, when= the > process is being terminated, vm_ops->close() is called once again against= the > original vma, which results in a page_counter underflow. This seems like a bug in hugetlb. I really hate the solution here, it's hacky and assumes only these fields a= re meaningful for 'close twice' scenarios. We now use vma_close(), which assigns vma->vm_ops to vma_dummy_vm_ops, mean= ing no further close() invocations can occur. If hugetlb is _still_ choosing to internally invoke this, it seems like it should have some if (vma->vm_ops =3D=3D hugetlb_vm_ops) { ... } check first= ? That way it'll account for the closing twice issue. Can you easily repro in order to check a solution like that fixes your prob= lem? I don't see why it shouldn't > > page_counter underflow: -1024 nr_pages=3D1024 > WARNING: CPU: 1 PID: 1086 at mm/page_counter.c:55 page_counter_cancel+0xd= 6/0x130 mm/page_counter.c:55 > Modules linked in: > CPU: 1 PID: 1086 Comm: syz-executor200 Not tainted 6.1.108-syzkaller-0007= 8-g9ce77c16947b #0 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/0= 1/2014 > Call Trace: > > page_counter_uncharge+0x2e/0x70 mm/page_counter.c:158 > hugetlb_cgroup_uncharge_counter+0xd2/0x420 mm/hugetlb_cgroup.c:430 > hugetlb_vm_op_close+0x435/0x700 mm/hugetlb.c:4886 > remove_vma+0x84/0x130 mm/mmap.c:140 > exit_mmap+0x32f/0x7a0 mm/mmap.c:3249 > __mmput+0x11e/0x430 kernel/fork.c:1199 > mmput+0x61/0x70 kernel/fork.c:1221 > exit_mm kernel/exit.c:565 [inline] > do_exit+0xa4a/0x2790 kernel/exit.c:858 > do_group_exit+0xd0/0x2a0 kernel/exit.c:1021 > __do_sys_exit_group kernel/exit.c:1032 [inline] > __se_sys_exit_group kernel/exit.c:1030 [inline] > __x64_sys_exit_group+0x3a/0x50 kernel/exit.c:1030 > do_syscall_x64 arch/x86/entry/common.c:51 [inline] > do_syscall_64+0x35/0x80 arch/x86/entry/common.c:81 > entry_SYSCALL_64_after_hwframe+0x6e/0xd8 > > > Since there is no sense in vm accounting for a bad copy of vma, set vm_st= art > to be equal vm_end and vm_pgoff to be equal 0. Previously, a similar issu= e > has been fixed in __split_vma() in the same way [1]. > > [1]: https://lore.kernel.org/all/20220719201523.3561958-1-Liam.Howlett@or= acle.com/T/ Understood that we do this elsewhere, I think equally we should not do this there either! :) > > Found by Linux Verification Center (linuxtesting.org) with Syzkaller. > > Fixes: d4af56c5c7c6 ("mm: start tracking VMAs with maple tree") > Cc: stable@vger.kernel.com > Signed-off-by: Daniil Dulov > --- > mm/vma.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mm/vma.c b/mm/vma.c > index bb2119e5a0d0..dbc68b7cd0ec 100644 > --- a/mm/vma.c > +++ b/mm/vma.c > @@ -1772,6 +1772,9 @@ struct vm_area_struct *copy_vma(struct vm_area_stru= ct **vmap, > return new_vma; > > out_vma_link: > + /* Avoid vm accounting in close() operation */ > + new_vma->vm_start =3D new_vma->vm_end; > + new_vma->vm_pgoff =3D 0; > vma_close(new_vma); > > if (new_vma->vm_file) > -- > 2.34.1 > >