From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id E4D6FC74A5B for ; Thu, 30 Mar 2023 00:58:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 690D96B0072; Wed, 29 Mar 2023 20:58:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6408F6B0074; Wed, 29 Mar 2023 20:58:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 508AB6B0075; Wed, 29 Mar 2023 20:58:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 3A29C6B0072 for ; Wed, 29 Mar 2023 20:58:42 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 005A4ABF37 for ; Thu, 30 Mar 2023 00:58:41 +0000 (UTC) X-FDA: 80623754484.23.AED2C56 Received: from mail-ed1-f42.google.com (mail-ed1-f42.google.com [209.85.208.42]) by imf06.hostedemail.com (Postfix) with ESMTP id 2A35C180003 for ; Thu, 30 Mar 2023 00:58:39 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=WEXfDUGX; spf=pass (imf06.hostedemail.com: domain of zokeefe@google.com designates 209.85.208.42 as permitted sender) smtp.mailfrom=zokeefe@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1680137920; h=from:from:sender: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:dkim-signature; bh=su0gIjKrNXbtaF8LQ8xZK0mtSIWjgnLtGXQ4zlfGIYQ=; b=KhPJwOBn9HY9gHeZstPq0b2yufuOsdSe1fiPGz8XFS9OnXMgK9tYt6VBtyt50vF7dSmOM6 IDGgPnHiLouE5bXonZhMv+n6HNbQyoHKiVEFCq6/nbnK4lB6Y6mJkd6MyLS4qwHV3VWUDj v3x/EEMotCts9xaiMX+qWnA+Mph64i8= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=WEXfDUGX; spf=pass (imf06.hostedemail.com: domain of zokeefe@google.com designates 209.85.208.42 as permitted sender) smtp.mailfrom=zokeefe@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680137920; a=rsa-sha256; cv=none; b=v3A3sRfftTvNhOkvkyJLMMj/vZoDIQEI9vUZ9xI1zMpr4BKfBcVyqyyxJxjXJ5gkcclsE3 HJkcCWHZ0LMG3qS44RLXrnIZ095UjUw/0m3oicn2O/uWyQEm4s99gAKt+RFKn3hgsuZ+oE vWlfR1bxfXzlxvyKdZtNd4U1k6xvZ50= Received: by mail-ed1-f42.google.com with SMTP id cn12so70331229edb.4 for ; Wed, 29 Mar 2023 17:58:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1680137918; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=su0gIjKrNXbtaF8LQ8xZK0mtSIWjgnLtGXQ4zlfGIYQ=; b=WEXfDUGXYmXh859yKLvna3F3O6cnWUDpZexV58P6WEGdNfB5Nd4xfh3zdYamjP67bX 6EfYoqmHRSU4Vaoo15AYIf0CnX1lcS/BHfKE1WA150zf8OCJGmueOZD4iCwZRysujJfI rpL8b1EhGGmItvwfEs5V5ZK36C92Ae7tm/pkG6YaPVavMos4drXFJGrL6qguaSFhwUEE ltz1iJr2Bgd3zPjgIt7F5E4BGm7u5phTfDS+rEjoSnWhOGp/W9zJewhi/Cl3IxzBh377 AEYgIOneC29WK6ZuASSG4nBdWXYudwhG0Nk8fM+EnjS95zu2DJBZamrb5LEIuZRaOf8x 0CNA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680137918; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=su0gIjKrNXbtaF8LQ8xZK0mtSIWjgnLtGXQ4zlfGIYQ=; b=2lEu1kekw3OMOEPMk+hMwZpzE+b12bQwHruq3dWPpYJslFpc9OGwRGfHwMYhucfJa7 fbSWcUsvj1JxhVwtn9QYwsJxeJPBQv5BIFSqMXzUCCB6z+6pTbcwEgzLrdNfHEjpZ6hj SIbRZZoXBH/K28L7Y9QSD4WcO+bFbv2bmrWgMenFkyfyL2Bhg+VnPkB+dqG/ITd1nvWt Zg2KQt9KjgPwdqemMVzSeG5rv0+ZorqCE+eB2zGAq5Xy/9u9YP18ojuWuDqBXJWIoZ36 EC9sqg8ndV+SqUzILLKmQNZpY/XRoUk0YCmiZtxJwY+IvzgrHOH/jvriiP0BV7Hy1cK7 z6uA== X-Gm-Message-State: AAQBX9eEB5zeelEnWZyW2/ZYrGcvbPZAnEmQoBzzRaLwxDDLV8a16ML5 PZkNw6z1rUxsSSYBUKwVnOQGF1iczosI/gGkqK0w9Q== X-Google-Smtp-Source: AKy350b53e6+XRogiX+Qi8FwDcW8/73RPONebml/PIb/IzEz/4R5hJRzEC0HD1YmwJoodG2Cjd5GvTqDGOGIz6a/GDM= X-Received: by 2002:a17:907:d412:b0:930:310:abbf with SMTP id vi18-20020a170907d41200b009300310abbfmr10584206ejc.11.1680137918431; Wed, 29 Mar 2023 17:58:38 -0700 (PDT) MIME-Version: 1.0 References: <20230329145330.23191-1-ivan.orlov0322@gmail.com> <20230329145304.66add47ba9b9fafb71b1e13d@linux-foundation.org> In-Reply-To: From: "Zach O'Keefe" Date: Wed, 29 Mar 2023 17:58:01 -0700 Message-ID: Subject: Re: [PATCH] mm: khugepaged: Fix kernel BUG in hpage_collapse_scan_file To: Yang Shi Cc: Andrew Morton , Ivan Orlov , linux-mm@kvack.org, linux-kernel@vger.kernel.org, himadrispandya@gmail.com, skhan@linuxfoundation.org, linux-kernel-mentees@lists.linuxfoundation.org, syzbot+9578faa5475acb35fa50@syzkaller.appspotmail.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 2A35C180003 X-Stat-Signature: 4jqxed5b7xey4z53qirpsh8ocmd1krnf X-HE-Tag: 1680137919-732129 X-HE-Meta: U2FsdGVkX1+f5pJiFQAv1nd7hFYdCjjAeILNOAHuET3cCk7r8UknfoCV2PecnEnf5czcrMabDawbSLpLVdDvonF6KWLep/ZaR2yv8Se8Yc+F6v6zN9kfec8Oe0qIB3xZ+BLh2yjYgCJYDvvna3MnrnHsvx/KAScbLRTAouo9I86Qcl3Nr1Jhmz8CsJOjy+jXc/P0LBMn87Qnyxk98cHjn0iRFzJWEIlNorQEUE0IdcZ0sRr5zGdTea0/U8MOk7FDwcYiM2ReY/NDUBe+UjQ4DsHaZTs20kocHG+sR6/I6jk0xuZn7MVFWoI8agj5pPoBpinMGlzmTkLrkqx//pCaC8WfbEOxLWOI6mJ4xOjQnS0JYQUTRd7Vr0SwyOBJreXqmVukbgAjdfVsy6x5vAldhpVcg9CXYmQyZEos3FWC0XfDo0AekO+ohBSqyQRyKmmZc8ZrRu1Ze3eajzS1KaSSQIXUpiQ7LHH2Aj7+s2nQMEOF8WaFP2P+RtPQWlC+v0SgWd9NEAKwVcZ576qPYBcxpHFCs/+ccz4QrxYn6QNHdFnkTF0u6Kp0cdxyC9yax19BOtOVMWX9S+6l8kgNCMOYNd0RNOO/9vMsgdRurjF+XY6IijuWiTyjv8RzyMbotKMGybDQt734rysWIUPmSRQxrtXPpLCMBBTlTbt+gkIWAXYSv4fHpQJ0aFjjcSiBLRPfCHLgVwm0y/MBU3/pwaNTvcFRfypqvDNIKnYwf3ok0U0i5yi2HBYdT/0/PlUZcq6niTWoMZ30hxrd3YZtFzLawhM6bZKo5PA5wIHN3XtEgzEtvWi6Npmejl3x01g11vlRpBxpurUIx78sBFA6TIGfEIVxoP/Y7Oire6bJdQ60oY4X/07q48nlAWJOulkwtwIKS+T1ZmNxWgxxjmBjOJ6YCVwgpqI7hcCuy6CJ9Lx139Rmx9qAMP0dmgjYvcEius06WcO6D6ZzofA9CNAJ+G7 M2p/3cGR U1ndL1ejoIaWeh5gQy6KYVN79Oa+Gmm0/z5jtdxGV/vJW1PjJxpSx1d2k4lCgFpvCV/k52BU5cvL65N2xg+2DtQDUef8XZnBBj5OkiK1vfQSo7TgLYuUZzGiCI7dVYygC/LB8BV7CY0b+Bau6WgQkPQ8X5vED3DW4K8v0HvPWfYCl3PRy1KVTNb2f9X5l7Af2ZgWtylUtUsYL72jRPA1PuQTbRexLV9OpxBZ8gT4FbnjbZ1I+jwmjzIDYDMRlNPDPKhmWSdvwSl//drWOUKCu6ThqdB8nKjuA9LhB4d5cE8iiNWq4ZOGStiOBshkZB+QwLZ+hoZeq9j4ZJTRMDvdpK9bBrHhFjeP3ERWOp308tbVRih0fNzbFetB0IKJsl09UKzLkKLHrgP0wSo070d4Yo5MgifcaKlEhZD6tKh3rxSck6Sr9cCPf43ZRRigJ5vr8qVlYVn0wcNM0v+p9TXYfEMobuxtIqWZls8UEdc2vXRxtkvxUZVS4/J6NxYvQdGwtgju3dIQWGFwdui6Xa8jXecF8MXzg93bmj5j+ouUSSUHCEJfl2pS1oCHNFkmyCtiEa68J6q4ondBDSjkPxdQAH5vL5tIVGT2EhML7XjjGgdTNMfkIBBm7Rwe1xH/8mqOm1jU9wzdzyNeS+/zMx5OLlxw4zmtwSxywNy/q X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Mar 29, 2023 at 5:14=E2=80=AFPM Yang Shi wrot= e: > > On Wed, Mar 29, 2023 at 2:53=E2=80=AFPM Andrew Morton wrote: > > > > On Wed, 29 Mar 2023 18:53:30 +0400 Ivan Orlov wrote: > > > > > Syzkaller reported the following issue: > > > > > > ... > > > > > > The 'xas_store' call during page cache scanning can potentially > > > translate 'xas' into the error state (with the reproducer provided > > > by the syzkaller the error code is -ENOMEM). However, there are no > > > further checks after the 'xas_store', and the next call of 'xas_next' > > > at the start of the scanning cycle doesn't increase the xa_index, > > > and the issue occurs. > > > > > > This patch will add the xarray state error checking after the > > > 'xas_store' and the corresponding result error code. > > > > > > Tested via syzbot. > > > > > > Reported-by: syzbot+9578faa5475acb35fa50@syzkaller.appspotmail.com > > > Link: https://syzkaller.appspot.com/bug?id=3D7d6bb3760e026ece7524500f= e44fb024a0e959fc > > > Signed-off-by: Ivan Orlov > > > --- > > > mm/khugepaged.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > > > index 92e6f56a932d..4d9850d9ea7f 100644 > > > --- a/mm/khugepaged.c > > > +++ b/mm/khugepaged.c > > > @@ -55,6 +55,7 @@ enum scan_result { > > > SCAN_CGROUP_CHARGE_FAIL, > > > SCAN_TRUNCATED, > > > SCAN_PAGE_HAS_PRIVATE, > > > + SCAN_STORE_FAILED, > > > }; > > > > > > #define CREATE_TRACE_POINTS > > > @@ -1840,6 +1841,15 @@ static int collapse_file(struct mm_struct *mm,= unsigned long addr, > > > goto xa_locked; > > > } > > > xas_store(&xas, hpage); > > > + if (xas_error(&xas)) { > > > + /* revert shmem_charge performe= d > > > + * in the previous condition > > > + */ > > > + mapping->nrpages--; > > > + shmem_uncharge(mapping->host, 1= ); > > > + result =3D SCAN_STORE_FAILED; > > > + goto xa_locked; > > > + } > > > nr_none++; > > > continue; > > > } > > > > Needs this, I assume. > > > > --- a/include/trace/events/huge_memory.h~mm-khugepaged-fix-kernel-bug-i= n-hpage_collapse_scan_file-fix > > +++ a/include/trace/events/huge_memory.h > > @@ -36,7 +36,8 @@ > > EM( SCAN_ALLOC_HUGE_PAGE_FAIL, "alloc_huge_page_failed") = \ > > EM( SCAN_CGROUP_CHARGE_FAIL, "ccgroup_charge_failed") = \ > > EM( SCAN_TRUNCATED, "truncated") = \ > > - EMe(SCAN_PAGE_HAS_PRIVATE, "page_has_private") = \ > > + EM( SCAN_PAGE_HAS_PRIVATE, "page_has_private") = \ > > + EMe(SCAN_STORE_FAILED, "store_failed") > > I'm a little bit reluctant to make the error code list longer, can we > just return SCAN_FAIL? IIUC this issue should happen very rarely, > maybe not worth a new error code. > > Basically the rollback approach makes sense to me. IIRC Zach was > looking into the same problem, loop him in. He may share some > thoughts. Thanks Yang, appreciate being brought into the loop. One of the things I plan to do during paternity leave is update my email filters so I don't miss things like this. Coincidentally, Hugh also just brought this to my attention. Looks to be the syzkaller report posted a few weeks ago[1]. Given there are two series munging with this path right now (or were), I was trying to find time to first review said series, then post a fix on top, if necessary (or it could have been incorporated into David Stevens' "mm/khugepaged: fix khugepaged+shmem races" series). But, I'm perennially behind and haven't been able to find time to do those reviews, and so my "fix" attempt has sat. Thanks, Ivan, for picking up the slack. So, I did test this patch with the syzbot reproducer, and everything looked good :) Thank you. I have similar reservations about increasing the error code list longer, unless there is opportunity to combine other failure sites under a common umbrella. For example, I was debating if a SCAN_OOM error was worthy of inclusion, which we could use in __collapse_huge_page_swapin() on VM_FAULT_OOM. I personally went the route of saying, "no, just use SCAN_FAIL". There also ought to be some comments, somewhere (either in code, or commit description) about why this is the only xas_store() site that deserves special error handling. I was planning on suggesting to sprinkle in a few VM_BUG_ON()'s after some of these sites, with a comment, just in case the implementation of xarray changes and operations which previously didn't require allocating memory now do so. At least to me, it took work to sort it out, so I don't think it's obvious. Now, as mentioned, I'm headed on paternity leave starting Friday, until July 12. So, if there is a v2, I'm likely to miss it, and even Cc'ing me isn't likely to get a response :) As such, feel free to have my Tested-by: Zach O'Keefe now, since I've validated it works. My understanding is that no other callsites need attention, so I believe this bug is "fixed" -- all that remains is dealing with the error codes, comments, assertions, etc. Thanks again, Ivan, Best, Zach [1] https://lore.kernel.org/linux-mm/000000000000226a6105f6954b47@google.co= m/ > > > > #undef EM > > #undef EMe > > _ > > > >