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 5203CC47422 for ; Thu, 18 Jan 2024 23:17:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C20F66B009B; Thu, 18 Jan 2024 18:17:56 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id BD05F6B009D; Thu, 18 Jan 2024 18:17:56 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ABFD16B009E; Thu, 18 Jan 2024 18:17:56 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 9F8EB6B009B for ; Thu, 18 Jan 2024 18:17:56 -0500 (EST) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 4B8541406F6 for ; Thu, 18 Jan 2024 23:17:56 +0000 (UTC) X-FDA: 81693996552.14.68EEC01 Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) by imf13.hostedemail.com (Postfix) with ESMTP id 8EDAF2000D for ; Thu, 18 Jan 2024 23:17:54 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="tLw6L/H3"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf13.hostedemail.com: domain of axelrasmussen@google.com designates 209.85.221.50 as permitted sender) smtp.mailfrom=axelrasmussen@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1705619874; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=JX1cr9tFTgKQC91coVo6egCQq+AbCsqUmfHm6o6nXyI=; b=KHvT0YC9JvXVBKlxlWQCDJtogbRTOrG+lftvf6uGyNccK6izVKH8VBVH9WLnrdRoFC4uYj BI2wwcW0fuWHSD5MVlueOz4/7o9+lX7Bq2yL/2IBWYqYZVzXqAUFPpPasFQGt9tUEE5zNz oGtR6FD0P4mNR6bonQhr/FcEmx0JoKY= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="tLw6L/H3"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf13.hostedemail.com: domain of axelrasmussen@google.com designates 209.85.221.50 as permitted sender) smtp.mailfrom=axelrasmussen@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1705619874; a=rsa-sha256; cv=none; b=XIsuhqKgmiG5oVbrifDaKgiKGPjH4/ErkDaY3kqSvPnMsAWcaI4XqfyixLiGUXSe99zneO Z944pReEhOHvgeMixJWyzHRHHfdvu7B5ou7SjMATLZ3/uA1g9z0YXeP8sgT+zzQL755/1I 9bA3EdTzmDo88ui16g/JpkX9Gnsf6f8= Received: by mail-wr1-f50.google.com with SMTP id ffacd0b85a97d-3376d424a79so143581f8f.1 for ; Thu, 18 Jan 2024 15:17:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1705619873; x=1706224673; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=JX1cr9tFTgKQC91coVo6egCQq+AbCsqUmfHm6o6nXyI=; b=tLw6L/H3fBWeVLz7zh5Hyg5e1ZxrTR6G4Ch3WMeJPbhwKVya2/AkWAduC+LjZho3t7 dU8XK7klXjJhPWjmjkogZZKLFMgeBP9dcbA4GezON6trbHmbZi7VQpeL+TXt2eJt20GF xrpzq9W5vr0HPfQHsRNhzTvZOqC2JvcaiiQgC0OYrDVKcjEsekFIa2DeBD6IIeonDT95 gq6c9NSimeTtbDrQr9ZLKtvGKduRGz5q1aW+4fGn5oPrgIQ9UO6n4A2Ysej7NzmF3jnn OmDrFK6Bmemr06+xvvuiVEoCFenzfLSxn+Mne2b2VYdMWHdRDoO2dPRn+XsW8t4SgOhm jyUg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705619873; x=1706224673; h=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=JX1cr9tFTgKQC91coVo6egCQq+AbCsqUmfHm6o6nXyI=; b=p5qRvqkXMyuob4J2/0Txz+EMzrcqgwUK0wM0wk3CPH7S3sPfaAEJ/KPJ9M0WWnZvKe CquuHLzVMicE9i60HUiLyAGCk6BPBJsZNnLHrPVfGkeAIPNlpcaTchutHWDhrUlVBu0m ba1ood9LVrnPwOaQqOuvZ+TTqetatU4gQUvNkkhJ+ze3GZxUCR1g91GzgthbcOAUhuUL nv0RbdA4uIuES/VR3PEpt8KX1w+dDt1kZWD/DVzs4njEpIQRZeENOWTJ9UoBgW+DyLeO pu1nuUIsjlimDrLPvMCkwpYwh+4Vh0T/bvz74JzME5S+BmUEdwviOONGvU93b5MTDIah aeyQ== X-Gm-Message-State: AOJu0YyHsHONzwA3RqOED/oM07yq3hDwaAhCkx23PDJoUO1el761+1sT qWa09hCfO31S0Q6chAsE/h5TMusJGFfF+muT0O48t+9kOhPGEiqFe2HU9xrATTf/K8k4Em7C+Zg DS1TwjB5GL6LE6cGiOmFKujrQ/BQddkPA2Cea X-Google-Smtp-Source: AGHT+IFrf1n4DM5FcXfriPORx1Gk3A+zegn/pC7Y+8wHv5e0r7W2T2bLq0C/Rut+m5UwIUopSxI59mFpK7qxQZKfXss= X-Received: by 2002:a5d:6709:0:b0:337:c521:8404 with SMTP id o9-20020a5d6709000000b00337c5218404mr796654wru.0.1705619872980; Thu, 18 Jan 2024 15:17:52 -0800 (PST) MIME-Version: 1.0 References: <20240117223729.1444522-1-lokeshgidra@google.com> <20240118135941.c7795d52881f486aa21aeea8@linux-foundation.org> In-Reply-To: <20240118135941.c7795d52881f486aa21aeea8@linux-foundation.org> From: Axel Rasmussen Date: Thu, 18 Jan 2024 15:17:14 -0800 Message-ID: Subject: Re: [PATCH] userfaultfd: fix mmap_changing checking in mfill_atomic_hugetlb To: Andrew Morton Cc: Lokesh Gidra , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, selinux@vger.kernel.org, surenb@google.com, kernel-team@android.com, aarcange@redhat.com, peterx@redhat.com, david@redhat.com, bgeffon@google.com, willy@infradead.org, jannh@google.com, kaleshsingh@google.com, ngeoffray@google.com Content-Type: multipart/alternative; boundary="000000000000a75b6a060f4092d9" X-Rspam-User: X-Stat-Signature: 6u5fs3odys1o6684bo7faqptksis8p4k X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 8EDAF2000D X-HE-Tag: 1705619874-206584 X-HE-Meta: U2FsdGVkX1/+15Yuuj9aMT3TdwjPXiPsdAt1SAWnbrmqA45XA8NQ2uzpElCLWyrsnT1jYOYOqh4maGcaoWEWxJJEEbMPqKNU5oCfwDhOwjqBTLHwnXtTmXryDX+W8gBjwpvQHsXPBFeR2pV4KyX1wAaq9QuwMMa3zEaAp3bEb/x6EdtumLL8noSHwU3iQsbmCVX0rmJAtJvmUPnr+F/aUR0L+gOwUE4CEZfevlYePmd7er6VhasA1Lh4NjAivKfgXmF8hL/e7s51nzVAGu9h3J8i4audN4DDF4b+CseWPvhY6sgV/Fec6kKvUMqClDQGk1ewIz0LqgmAwZN2zMtNmsK1x2Wnd0dktZjflm/ZIqQe1uFyLItgVayIo/dArc3Xqt+KB4VhaZcVDbiTVcWjRXqiXRFp5AEaXYyHW1lQVaRoHwmrSbmjOyUPHD+L1uKLdl1aJCV1S5JnpwxYvdkVTaOL5a4Uhi+5KN9dgRyZlVaH4FHnOHs1lvRM729puF/31PfqwtUt9r3of4PJNVFZTx1singso1Rkypa3l8VC2ZUGDm8rL3PzTqB8KAPalkjVCb3li+3ChalfggUkNlbmlKfUDN2Q8ZzOBLabw+H1QSTsWKf2lBuKKQHmRyMQnePfMvU0jKeP+VN1Dr13EYDVbUC0BuWjWRKc0Qm62u8/ARh8Z9zP6cLWS5MSqUjxVJwaJda3KlpkMgLgkz6ElFMGCfGHQSsDH//vJ88/UfEJ/0xSV87C7wqkW1EQFVJfn0keRWb3JvJdzZJ5pIWhx6kYjlcmRCP4WvJYgzOb/Wju3Nmb+bKySB6dIcHB9eNCIm+ES69I9bXn1TG2DM4UwI/KrxWdskySiJhEEW2rFp9YugmynV4gpo5Z0oHVS4+CQ/gwQArpkuctJgFWl6heKvGES2mtIw4aNEtqM8x/OQ0wdmlo5ZNUFQQrNXl6BF9lL65N0fqXsGfQKslFUyfFKUx O8xXcZeM BEHtjrr4OpRut1UlSJyM8DgT4S+o0+8q7DGFynWeWnQq3gMRnoyINZ+JrFjPPkVdWuOomRtKBk3Ii5GJ5Q0Ib2nMhIaiA3CY11JUcM7IMuiNEzExYYZGe6W4qQAIqc+qjI0zlfiCx2cIKW0+mcVuFMLoeBl82OerOjo4hS0E36bs3b7DfYJo/avV1nQmnaIlKtove+vjcuL1JJDQgBOElS/sh7jcRJIzylwqBypM6ONfce+BxffmlyAu7SsnQZugfgPQHT23P+2ga8aj2VCCcECtfzXd4qsBXm9DI0Cg34meKg1u98y7UIizVE58l/judGcuOxk8HoohnOJlTlEz3asnLMNaOuWixLQOOb+DKie/AXfq1AsLDb8h2et5b0RUttXYojro8l6rGvHh1zNM9AMtUXLnwtF820Aynq2/hRkki4/Tpq91pjK1zLyzLTSxSJEobT2L2Dd6c5b69Stah5OMi9g== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000948, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: --000000000000a75b6a060f4092d9 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Jan 18, 2024 at 1:59=E2=80=AFPM Andrew Morton wrote: > On Wed, 17 Jan 2024 14:37:29 -0800 Lokesh Gidra > wrote: > > > In mfill_atomic_hugetlb(), mmap_changing isn't being checked > > again if we drop mmap_lock and reacquire it. When the lock is not held, > > mmap_changing could have been incremented. This is also inconsistent > > with the behavior in mfill_atomic(). > The change looks reasonable to me. I'm not sure I can conclusively say there isn't some other mechanism specific to hugetlbfs which means this isn't needed, though. > > Thanks. Could you and reviewers please consider > > - what might be the userspace-visible runtime effects? > > - Should the fix be backported into earlier kernels? > > - A suitable Fixes: target? > Hmm, 60d4d2d2b40e4 added __mcopy_atomic_hugetlb without this. But, at that point in history, none of the other functions had mmap_changing either. So, I think the right Fixes: target is df2cc96e77011 ("userfaultfd: prevent non-cooperative events vs mcopy_atomic races") ? It seems to have missed the hugetlb path. This was introduced in 4.18. Based on that commit's message, essentially what can happen if the race "succeeds" is, memory can be accessed without userfaultfd being notified of this fact. Depending on what userfaultfd is being used for, from userspace's perspective this can appear like memory corruption for example. So, based on that it seems to me reasonable to backport this to stable kernels (4.19+). --000000000000a75b6a060f4092d9 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Thu, Jan 18, 2024 at 1:59=E2=80=AF= PM Andrew Morton <akpm@linu= x-foundation.org> wrote:
On Wed, 17 Jan 2024 14:37:29 -0800 Lokesh Gidra <lokeshgidra@google.com= > wrote:

> In mfill_atomic_hugetlb(), mmap_changing isn't being checked
> again if we drop mmap_lock and reacquire it. When the lock is not held= ,
> mmap_changing could have been incremented. This is also inconsistent > with the behavior in mfill_atomic().

The change looks reasonable to me. I'm not sure I can conclusively s= ay there isn't some other mechanism specific to hugetlbfs which=C2=A0me= ans this isn't needed, though.
=C2=A0

Thanks. Could you and reviewers please consider

- what might be the userspace-visible runtime effects?

- Should the fix be backported into earlier kernels?

- A suitable Fixes: target?

Hmm,=C2=A06= 0d4d2d2b40e4 added __mcopy_atomic_hugetlb without this. But, at that point = in history, none of the other functions had mmap_changing either.

So, I think the right Fixes: target is=C2=A0df2cc96e77011 (= "userfaultfd: prevent non-cooperative events vs mcopy_atomic races&quo= t;) ? It seems to have missed the hugetlb path. This was introduced in 4.18= .

Based on that commit's message, essentially = what can happen if the race "succeeds" is, memory can be accessed= without userfaultfd being notified of this fact. Depending on what userfau= ltfd is being used for, from userspace's=C2=A0perspective this can appe= ar like memory corruption for example. So, based on that it seems to me rea= sonable to backport this to stable kernels (4.19+).
--000000000000a75b6a060f4092d9--