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 94EBFC2BA4C for ; Wed, 26 Jan 2022 16:53:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 173216B0073; Wed, 26 Jan 2022 11:53:21 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 122466B0074; Wed, 26 Jan 2022 11:53:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 010FB6B0075; Wed, 26 Jan 2022 11:53:20 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0140.hostedemail.com [216.40.44.140]) by kanga.kvack.org (Postfix) with ESMTP id E6DAD6B0073 for ; Wed, 26 Jan 2022 11:53:20 -0500 (EST) Received: from smtpin31.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id A4CD9181CA747 for ; Wed, 26 Jan 2022 16:53:20 +0000 (UTC) X-FDA: 79073033760.31.DA60605 Received: from mail-ed1-f42.google.com (mail-ed1-f42.google.com [209.85.208.42]) by imf01.hostedemail.com (Postfix) with ESMTP id 5830840008 for ; Wed, 26 Jan 2022 16:53:20 +0000 (UTC) Received: by mail-ed1-f42.google.com with SMTP id l5so86442edv.3 for ; Wed, 26 Jan 2022 08:53:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=yMCR+EXKge9hen5RQObBZwVGVHUPBvemW8pcttH5ZO0=; b=IkEvYfQOI3Zx6Igawua9mn3i1veqQWdgKp+06ExpqDtWBuxvZnD5pf/aTUqEuWCQYk hwmyWiaeTyPG+nmwAmcJexeZNAaa9xL/16MAlgrV8nAkwFLaugBZpz9m9EVgSb3eZ1Np JsdwzpYxEGeKAkJLyVfbn8o/L/Xk5J8V4zmXx8GPhat3rDHBqN0UlQ2bAShJ4qHQ+2dD bmfeqMx+NKz+a/HhobeGxqhOCnYZCujYcUD/oSOD/EqgReWsUoF3wZacE49E56qVclG/ rNL4RWkPcUFsUtASUdnFj88X6fg6T7XNI72GPIgPACYoY9RVDEvh7B6j9yMgHLE76Dc9 yDww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=yMCR+EXKge9hen5RQObBZwVGVHUPBvemW8pcttH5ZO0=; b=1BJFU6e+JHKO2meF1+MnmuHREgxvJcJS8kErmhjbN4XIn8av8EjClQeTg6HrB0PBEO TijfWyGEXMoWjrRiyjMbZAydsPn/70Ye8o9LeO0GzBWx3BlJP5ae2mzsv99c9Ukwcjuu GbrV1W8cqJUwkp+KcAcRpMg0y9M1lL9IkqQHm6zO3R6u1ZLKnJ7MBBqSq3K548j1GXMd Y4d03/7VvDIQNr3E4u/MHvHcxWyzAtAovQNtcAletPXuYelcMYL4WJd9qvnzuSLvkF1C X4epAGmqsnWW1/3KQsc1fKtuiTdIYxRN6uSCIfJwYMz38VSdxnIbwdHIUobaZEUUHCoj 7sdA== X-Gm-Message-State: AOAM5310QtVIwg20x+nKpVyvDVMHvIgfy/5DGUhmr1WozNL7FRcuMkSi zB3aiApokmVHs+cdKhIIVs3g3/L+ACD/jRl5dMA= X-Google-Smtp-Source: ABdhPJzivxlBNUz+a7RinXUcIWuV4Xgj19ciAH9xKeP04EYHcdAtwIWVijN24TiR/G/c7rVd/n4BnKZJiXLdcesNIOU= X-Received: by 2002:a05:6402:11c9:: with SMTP id j9mr25438514edw.385.1643215998924; Wed, 26 Jan 2022 08:53:18 -0800 (PST) MIME-Version: 1.0 References: <20220120202805.3369-1-shy828301@gmail.com> <5b4e2c29-8f1a-5a68-d243-a30467cc02d4@redhat.com> In-Reply-To: <5b4e2c29-8f1a-5a68-d243-a30467cc02d4@redhat.com> From: Yang Shi Date: Wed, 26 Jan 2022 08:53:06 -0800 Message-ID: Subject: Re: [v2 PATCH] fs/proc: task_mmu.c: don't read mapcount for migration entry To: David Hildenbrand Cc: Jann Horn , "Kirill A. Shutemov" , Matthew Wilcox , Andrew Morton , Linux MM , Linux Kernel Mailing List , stable Content-Type: text/plain; charset="UTF-8" X-Rspam-User: nil X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 5830840008 X-Stat-Signature: x9xt8m7mbb44pwmj6u15e8cnopupu4wm Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=IkEvYfQO; spf=pass (imf01.hostedemail.com: domain of shy828301@gmail.com designates 209.85.208.42 as permitted sender) smtp.mailfrom=shy828301@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-HE-Tag: 1643216000-936203 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, Jan 26, 2022 at 3:57 AM David Hildenbrand wrote: > > On 26.01.22 12:48, Jann Horn wrote: > > On Wed, Jan 26, 2022 at 12:38 PM David Hildenbrand wrote: > >> On 26.01.22 12:29, Jann Horn wrote: > >>> On Wed, Jan 26, 2022 at 11:51 AM David Hildenbrand wrote: > >>>> On 20.01.22 21:28, Yang Shi wrote: > >>>>> The syzbot reported the below BUG: > >>>>> > >>>>> kernel BUG at include/linux/page-flags.h:785! > > [...] > >>>>> RIP: 0010:PageDoubleMap include/linux/page-flags.h:785 [inline] > >>>>> RIP: 0010:__page_mapcount+0x2d2/0x350 mm/util.c:744 > > [...] > >>>> Does this point at the bigger issue that reading the mapcount without > >>>> having the page locked is completely unstable? > >>> > >>> (See also https://lore.kernel.org/all/CAG48ez0M=iwJu=Q8yUQHD-+eZDg6ZF8QCF86Sb=CN1petP=Y0Q@mail.gmail.com/ > >>> for context.) > >> > >> Thanks for the pointer. > >> > >>> > >>> I'm not sure what you mean by "unstable". Do you mean "the result is > >>> not guaranteed to still be valid when the call returns", "the result > >>> might not have ever been valid", or "the call might crash because the > >>> page's state as a compound page is unstable"? > >> > >> A little bit of everything :) > > [...] > >>> In case you mean "the result might not have ever been valid": > >>> Yes, even with this patch applied, in theory concurrent THP splits > >>> could cause us to count some page mappings twice. Arguably that's not > >>> entirely correct. > >> > >> Yes, the snapshot is not atomic and, thereby, unreliable. That what I > >> mostly meant as "unstable". > >> > >>> > >>> In case you mean "the call might crash because the page's state as a > >>> compound page could concurrently change": > >> > >> I think that's just a side-product of the snapshot not being "correct", > >> right? > > > > I guess you could see it that way? The way I look at it is that > > page_mapcount() is designed to return a number that's at least as high > > as the number of mappings (rarely higher due to races), and using > > page_mapcount() on an unlocked page is legitimate if you're fine with > > the rare double-counting of references. In my view, the problem here > > is: > > > > There are different types of references to "struct page" - some of > > them allow you to call page_mapcount(), some don't. And in particular, > > get_page() doesn't give you a reference that can be used with > > page_mapcount(), but locking a (real, non-migration) PTE pointing to > > the page does give you such a reference. > > I assume the point is that as long as the page cannot be unmapped > because you block it from getting unmapped (PT lock), the compound page > cannot get split. As long as the page cannot get unmapped from that page > table you should have at least a mapcount of 1. If you mean holding ptl could prevent THP from splitting, then it is not true since you may be in the middle of THP split just exactly like the race condition solved by this patch. Just page lock or elevated page refcount could serialize against THP split AFAIK. > > But yeah, using the mapcount of a page that is not even mapped > (migration entry) is clearly wrong. > > To summarize: reading the mapcount on an unlocked page will easily > return a wrong result and the result should not be relied upon. reading > the mapcount of a migration entry is dangerous and certainly wrong. Depends on your usecase. Some just want to get a snapshot, just like smaps, they don't care. > > -- > Thanks, > > David / dhildenb >