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 2FECBCF042B for ; Wed, 9 Oct 2024 00:50:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 656636B00D5; Tue, 8 Oct 2024 20:50:09 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 608C16B00D7; Tue, 8 Oct 2024 20:50:09 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4A5716B00D8; Tue, 8 Oct 2024 20:50:09 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 2812E6B00D5 for ; Tue, 8 Oct 2024 20:50:09 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 95D551A0DE4 for ; Wed, 9 Oct 2024 00:50:05 +0000 (UTC) X-FDA: 82652232096.03.8A752C7 Received: from mail-wm1-f48.google.com (mail-wm1-f48.google.com [209.85.128.48]) by imf09.hostedemail.com (Postfix) with ESMTP id 87366140004 for ; Wed, 9 Oct 2024 00:50:06 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=yvsNf7xI; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf09.hostedemail.com: domain of jannh@google.com designates 209.85.128.48 as permitted sender) smtp.mailfrom=jannh@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1728434857; 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=P5xBkJBsHSVqAt+axJMzODbqA3UYAyFBsJSa47B8HB4=; b=Lm38r6LYVo4RZuLyC4KbdhDVsjGQ8EhrLuVVHwMqMe4MVDy6iee4YQnD7BNv6cYMmsVIzU 1hnbnULT1wpbpeQhvHTdtbid12mSx2fvEm+9s1bDcDPL00/T3ZGPjvE+/phYzbb3d03zpH UWxu5tmSGuPXpR4yJmrZyPmnh2cvJKA= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728434857; a=rsa-sha256; cv=none; b=wCmiiu/xr9EwCT8h9ONwzVdyOR/7stCDdo4jO7ERdzy1UO3XDwaT1UT5Di9dRZm+DQj4Uh BoTMlZ59qqdFtfFyHEaSw8E8utS4GQ+9VMhoFWFweyiU1xYgqfag3T19TxwklVfewz6gu+ b3TUaKtPpI3TBZgYImOhtyQf/SMw9LA= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=yvsNf7xI; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf09.hostedemail.com: domain of jannh@google.com designates 209.85.128.48 as permitted sender) smtp.mailfrom=jannh@google.com Received: by mail-wm1-f48.google.com with SMTP id 5b1f17b1804b1-43111d23e29so8475e9.1 for ; Tue, 08 Oct 2024 17:50:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1728435005; x=1729039805; darn=kvack.org; 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=P5xBkJBsHSVqAt+axJMzODbqA3UYAyFBsJSa47B8HB4=; b=yvsNf7xIVVoMejAI0HM2wULkKJ85YIh8Uk21NkQkLC/YVr2A/2ZV1cVZohkW1MRUGj WqOK12af2aRHOp0xi7IXrL0i/rnfj9PcA05WVjSIrJS8oDK31Zw+WXaav7Nk9K2ptzVZ eCoRRiGq/Gn5tU3Kq6cq2JZIebBa0jj3PQSJCWBmbZV/RvHo/X0vDj06X4kJU0VYCocF mv1vSGnkxQczpuyU+9UjkiOC127QMPgcfWnTfoszjy5J2H6hlxrGBjqL2r5G3QdfVJk+ 9z5a6YBVCHAHyNUMV5EFSw0PygPsf+omy5ZiRWMawz0b/k2K1NffmbU9QlumGZrl026i maGg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728435005; x=1729039805; 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=P5xBkJBsHSVqAt+axJMzODbqA3UYAyFBsJSa47B8HB4=; b=R2azY+8BostFlE5Nw8RSWNI2nPBbCLTtfX6p+lvnVObsI/MBUvWE2K4rU7hsUe4mBC T1NJfEyohR/S0/N5VSmBPvN9b0U1eJFQ0JGpA7b3nywhT2xWs0xRLbBJ3LhWSKl/UD1x ZGMqcRalMPAAcdMrWiBMdNgS+QNa8XF9Dh1N6jLep1Ev92wmuxnbGP+pqu/5x4IpdIKo GHGD4nXOwTrX/nCdOOWidmdewLjg07VGoxRoyVM1G4PceelxZnVGymLBWeR2u3B9YWPc U98Z9R+JmEEF5KqzBh5/4KPW98EVVoArERqRAg/TsG0QXf/z/PIHWxhAckQ63M79cJ0u kx6Q== X-Forwarded-Encrypted: i=1; AJvYcCVuuiPbs0yFlf7meCbkFn+gVjjpMHg8dl88FN331AuR7BgQwEtnoeYmoHRlnEWr49pD/NeSSoxDaQ==@kvack.org X-Gm-Message-State: AOJu0YxHkDcBu6nqfPJvDU2QbxIOImIb1eTf8WJRK6QkjSeRozWMbapX qg9PzVYQfEElqmzVQKaKO3u1uqXJ4gaSujNAGctM45OofwhE0d+XheGpvXmJ8PNFjXnw9NfP01D I4Z/2ffWcW29XHYnOrPpeOhsVnJp2B/uGK90r X-Google-Smtp-Source: AGHT+IE8i1WoerH4tKasIfRlN/mHeEmDePbStRVFDY5RZAcV3AkdGjoAzKlazO6sInZgo+PAJdXEmCaNDOM+Ho3MuYA= X-Received: by 2002:a05:600c:cc3:b0:42c:9e35:cde6 with SMTP id 5b1f17b1804b1-43058cee8e2mr2199905e9.2.1728435004265; Tue, 08 Oct 2024 17:50:04 -0700 (PDT) MIME-Version: 1.0 References: <20241007-move_normal_pmd-vs-collapse-fix-2-v1-1-5ead9631f2ea@google.com> In-Reply-To: From: Jann Horn Date: Wed, 9 Oct 2024 02:49:26 +0200 Message-ID: Subject: Re: [PATCH] mm/mremap: Fix move_normal_pmd/retract_page_tables race To: Joel Fernandes Cc: akpm@linux-foundation.org, david@redhat.com, linux-mm@kvack.org, willy@infradead.org, hughd@google.com, lorenzo.stoakes@oracle.com, linux-kernel@vger.kernel.org, stable@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 87366140004 X-Stat-Signature: um68y3k8knpy1xx9kpqknbnzp6wexguc X-Rspam-User: X-HE-Tag: 1728435006-587000 X-HE-Meta: U2FsdGVkX1+E+hBmaFFSNwvHO0Fq8C0kJFX7D4+F/i/bsfFHhBOWIr+kFzteV22FwurU5Z74uNU7jPzyCPeZBLvhkgRa3tjgps+cuxpkr4Ja+fwY0uR66fuKkUiBk6l//wW/BgPciJPvpi9wC0Yr+8NCgNyk2SezOQwLY+F/0k24gYIHS4LUYwFC4ypAD8WdX9Ydwt5Jj0aviU97dwV1Xde7Y3TnglBuozMUNrpWLBAtZ8/tl8/USXiVgGgF2KI0CwFGVaLFwibZTw6rnf/BcR3kPcroSSOSuaDVkXt+5KAAxrBp059dxJNJ3L+rqNYtqEI8INVC1uKy3dWSeiePVXJ1geiIrOq6M5Vy1lFoy2Ra0LUXRj6+fjoKgpPmtnhYrU8hfRd2p0XDBnhqFwcWD1j+uxGaflrIXonheGdvSxkTXt5vbgLcJdJN/4vO4E8QGs3ftI+RUbv240H/4XQ9918zVdXT42gc8Y0NZ2c176EGIW/Noxkq2jTcdKA4eAQf53V9k0Wdu3Sx+cV2e0foxufMmgvLjhzHU4QTCy1SegyYWrGtrUK6vdsSA/XRZe99PNxuzfoZB9zk1hoRWrHOg/n2wDs+glHz9Va3TARZzJiXpxS05XOx8opH/bw+L3VSysLAhPbnlMczQ1GevFUvXBLSU6y2Skkmj9KtGq6FkAPV8AhyxGP1XcEnSXNsXyglMocF02rJEjgSLU5QT5LslBF5mezDK0CL2+pNe6R553nZ4ymQKLDSNZfZdjhKuLjAwKCcxaM3p+YIKhCXxn8u6XsVhHU1X8XyExah0BDJ6rTPrwvohHk/B01l4xbmgwyaO2Ddru8DHnAUj6bic3iBmEhDc02FUFLj1edhM4WMnhYX+B5F8FL9sQ9E7gF+xZzWtrsFIgW2fDVS9Fh9VbcQe5RGW6BocqBIA2uIs4AUQ/bUyEE2YrwTqe5RJ/OKXOJOTUmu9E4k3P5Tfg9Rn1i yvT1ts7S 16OkTSqeSp0gwaHv/D08VUwMqbxt/eTySt9JPiWo6+V0si/Pdbg5Nm1wQhReGEzYcMZiES1g4pinvE93ZVSwKaYf62FCvuFQRTrECd3RSAgoqJulSOuVOlQuKqibCBiHbowfUr/gsW8LppIOW/ASP2I+H+Pl13ew5qrufPa3IvqNGqKq1Vo5N5/9HqIybgUg6XJEMwy++6j2DuNIrp2b/80nNfcaUlJGfbaijIWe3cUyIHF+ZAadNe8PUVoQ+sEfTASqoThTwJx/QqkQ0g7RyZIXBwdl83yinNEMqjv6U1nqsPUWtMGfKL9br4KKW2MBkzSLS 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: List-Subscribe: List-Unsubscribe: On Wed, Oct 9, 2024 at 1:58=E2=80=AFAM Joel Fernandes wrote: > On Mon, Oct 7, 2024 at 5:42=E2=80=AFPM Jann Horn wrote= : > Not to overthink it, but do you have any insight into why copy_vma() > only requires the rmap lock under this condition? > > *need_rmap_locks =3D (new_vma->vm_pgoff <=3D vma->vm_pgoff); > > Could a collapse still occur when need_rmap_locks is false, > potentially triggering the bug you described? My assumption is no, but > I wanted to double-check. Ah, that code is a bit confusing. There are actually two circumstances under which we take rmap locks, and that condition only captures (part of) the first one: 1. when we might move PTEs against rmap traversal order (we need the lock so that concurrent rmap traversal can't miss the PTEs) 2. when we move page tables (otherwise concurrent rmap traversal could race with page table changes) If you look at the four callsites of move_pgt_entry(), you can see that its parameter "need_rmap_locks" sometimes comes from the caller's "need_rmap_locks" variable (in the HPAGE_PUD and HPAGE_PMD cases), but other times it is just hardcoded to true (in the NORMAL_PUD and NORMAL_PMD cases). So move_normal_pmd() always holds rmap locks. (This code would probably be a bit clearer if we moved the rmap locking into the helpers move_{normal,huge}_{pmd,pud} and got rid of the helper move_pgt_entry()...) (Also, note that when undoing the PTE moves with the second move_page_tables() call, the "need_rmap_locks" parameter to move_page_tables() is hardcoded to true.) > The patch looks good to me overall. I was also curious if > move_normal_pud() would require a similar change, though I=E2=80=99m incl= ined > to think that path doesn't lead to a bug. Yeah, there is no path that would remove PUD entries pointing to page tables through the rmap, that's a special PMD entry thing. (Well, at least not in non-hugetlb code, I haven't looked at hugetlb in a long time - but hugetlb has an entirely separate codepath for moving page tables, move_hugetlb_page_tables().)