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 E28A8C77B7C for ; Tue, 24 Jun 2025 11:29:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7FE6F6B00A8; Tue, 24 Jun 2025 07:29:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7D6916B00A9; Tue, 24 Jun 2025 07:29:46 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 713466B00AA; Tue, 24 Jun 2025 07:29:46 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 60F966B00A8 for ; Tue, 24 Jun 2025 07:29:46 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 06F8780559 for ; Tue, 24 Jun 2025 11:29:46 +0000 (UTC) X-FDA: 83590074372.03.D9FDF09 Received: from mail-ed1-f41.google.com (mail-ed1-f41.google.com [209.85.208.41]) by imf10.hostedemail.com (Postfix) with ESMTP id 06A8AC0007 for ; Tue, 24 Jun 2025 11:29:43 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=bgwZJvlR; spf=pass (imf10.hostedemail.com: domain of nadav.amit@gmail.com designates 209.85.208.41 as permitted sender) smtp.mailfrom=nadav.amit@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1750764584; 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=cT0e8thdB741gKZwqm26adW2g3BYeo+7KpebTXcd1V4=; b=utfGZDrkIDPJECNpxsxDdYIE9MXI58n1+upAvSyfxKYEYRktnFn9obUglDiH+8wuPvvUG5 pgOWB179c5hLSTN6yBiW5iDEGVUjn8a1tgUpHlNOBMkAPDDUAXgHzWhKyxrXUJvwJs5m2f QcS9l+IJwCAl8QlLeMLAXTU+8rUu7iA= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=bgwZJvlR; spf=pass (imf10.hostedemail.com: domain of nadav.amit@gmail.com designates 209.85.208.41 as permitted sender) smtp.mailfrom=nadav.amit@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1750764584; a=rsa-sha256; cv=none; b=Xcoy9B6ofNFZS2WqVyKoqLhuh94m/agVE30nuHN/MSNKeCey2D/bbT7MIKh/vXOk3FAAQo 0v2FIMA7JfIpG7DRI/UQgcrE6HTnz6FZTYxaZFAXKlIZ9ypJ1Uf8H23Hw+keO/lkj6O993 9lGheNszvm8WRhlf2jxhwIKjXSpByco= Received: by mail-ed1-f41.google.com with SMTP id 4fb4d7f45d1cf-6077d0b9bbeso8824591a12.3 for ; Tue, 24 Jun 2025 04:29:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1750764582; x=1751369382; darn=kvack.org; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=cT0e8thdB741gKZwqm26adW2g3BYeo+7KpebTXcd1V4=; b=bgwZJvlRbvCaz2USJd2/WzFnavFXI9oJQsJJ3QYQRgjR5PXgtyBHz97VPQ7dBvgB/9 eRG0dZJSDa3bpzkFr4NbHmeRLmSwLGoGPrzbFTI+CAy9I54FDepKci7cJeJ/ll8Pwebl A3qvSplAygJ1mMVNu6YjV62Q2aXIwOl6RdBtbI6xwaT1QTuHcy2jDyBggegUHWR8fnue AdWDxjkf+ZJAO+aLAvu0wJpeE22fO5KWtH9WUuW6hD6ipWSA6AaGw6FH7f6dHzQNX/+M mz1TvV695BhAhWIG+uuM1YPbhf4XJD7c3uUb6nmeKezgZLvglgPa+yPYjBAQV6hMzgMS X+rg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1750764582; x=1751369382; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=cT0e8thdB741gKZwqm26adW2g3BYeo+7KpebTXcd1V4=; b=gHmwWrPR+NXW7MSxIN28AMaRoSoWiJ/1DtjvK6Hk0gDslRNdVXZI1pqST9nl9L746t sV1burCcAQAfFClchKs6IKPv8Ggwuz9xrGyeYH7dnLIgq8JbjI6EKgTezqjg9iiVSmBD BizG7v7lVzRa85s3wTiadb6OpEx3k4I+terDBEANNDW4hGxF5eNeNm2qmN6HCN5qgjJQ kKBdKdQThEZSUNzQleMM14lG+UHvhS38STEgSjIUHg9/cgDOvzIUVhgT2JezSzaVlWsw B2Vr9bu+ziuDLlGQcZODNeOxK96bcakJ/63UKV8I/n2pwnyqj7ci/ExuBgwSyceRuIk3 zerg== X-Forwarded-Encrypted: i=1; AJvYcCWxVh3HzN/EBlvH+aF3fPb/2UEL1q+IotDqY2C28r4Dvxdhqre9rRXSFVQCK8imlQGFd/wx0gis2w==@kvack.org X-Gm-Message-State: AOJu0YxgT8yJ8F6j2FvxdXEB1CG3Iw7OYFzSjmFjTuqO0s0Uzy7IaI1u W48eXZZ3pG0yFf3145lm1R3BqeGMuq4rELHOSBVs/jOjUUDU1diCnMpZ X-Gm-Gg: ASbGncvFXgmhMBgG3n+IdGZgmpy3/ZKgMpo7fnXs2Hk/8TTLjtB/5nj6VqrZEyunUy0 JYeA5h4LC8TQ+rEXPT4bGQNIdzPeZHNPMMRSp6u2hN5XBbcI67KtcpONpMXREXeh8gify9fb3Kd 3EGPfhklwXDKvuYOcOS1upw8aH1hXvC0HG1vG0mm+ARqHxvYIkxxGcW+HxzCz9/2BLOSDCtp+Eo b2O0jwgBAenWZ+x4/U4ggvVaAE11kQ1qS8PbQmcLs54FZwytapQvAMkPM0dMT2Z+kqbjIy+zOnb C1cT2vcAPo7B9cN/eR08TQdSEoVIl/2NOD+qARhqSr2vfKIskFMb/K1ju5Us87/RMExjUb6X0pw CQ9U= X-Google-Smtp-Source: AGHT+IGQvk8QZ+Fin/JWPu8cW0baEoe0DVTqf/iH6dwWkXfQMRKk1ZBhOly5xwow0t3UeqmTGHpNiw== X-Received: by 2002:a17:907:9725:b0:adb:45eb:7d0b with SMTP id a640c23a62f3a-ae0579821c1mr1673852866b.15.1750764581968; Tue, 24 Jun 2025 04:29:41 -0700 (PDT) Received: from smtpclient.apple ([209.35.226.87]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-ae0b5ab9b8esm28917966b.140.2025.06.24.04.29.39 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 24 Jun 2025 04:29:41 -0700 (PDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.600.51.1.1\)) Subject: Re: [PATCH v2] selftests/mm: Fix UFFDIO_API usage with proper two-step feature negotiation From: Nadav Amit In-Reply-To: <4fd18a1c-aba2-468a-881f-0507953f2904@redhat.com> Date: Tue, 24 Jun 2025 14:29:27 +0300 Cc: Li Wang , Andrew Morton , linux-kselftest@vger.kernel.org, "open list:MEMORY MANAGEMENT" , Peter Xu , Aruna Ramakrishna , Bagas Sanjaya , Catalin Marinas , Dave Hansen , Joey Gouly , Johannes Weiner , Keith Lucas , Ryan Roberts , Shuah Khan Content-Transfer-Encoding: quoted-printable Message-Id: <611F9598-A1A4-47B6-B37E-09BF7B4D17D0@gmail.com> References: <20250622081035.378164-1-liwang@redhat.com> <20250624042411.395285-1-liwang@redhat.com> <4fd18a1c-aba2-468a-881f-0507953f2904@redhat.com> To: David Hildenbrand X-Mailer: Apple Mail (2.3826.600.51.1.1) X-Rspamd-Server: rspam11 X-Rspam-User: X-Rspamd-Queue-Id: 06A8AC0007 X-Stat-Signature: b46j6a3jicdan8z6a49y8q5x7je9ian9 X-HE-Tag: 1750764583-225271 X-HE-Meta: U2FsdGVkX1+3kKVZxuTFQmXX6dOpCVk21VLd8QCBH/UTQil24eZlqPN8QbvTHD1wC1jwmDvvQqUfj5cizwI/1bHMlUPn2U2DBtbhg67mKAlFqbO9li9PhFJia2eNWlGilaAWMR474dvIWhR1dA6RHtwiSj+fHgPhtPl7J/q3qC739/7Ot3m4XyokPLFLuZFHAKNr6ge7dxZkh+SkUsE7s9opg64H2N32oEMTD8/FDRKUrPVUke0h1OqWbn+zqeyrRZ4abmxVsedE4jw2ikXfrBQqpyCI4rPTvn2qZF5xr6WveEfGQwqoo5aHM7PHdAyH0yscuDW6wZWWTNmgi+82zYvqNo2E8hl3zXKCfrIHkFOWgmuL+eG1Ms8xhA6RF6xFmVSOXYEyQsqWKEz192kOUVNuAbQQDRJpPT7ltLc9eqOsDHIEekI30o5CEF66dMHVwKjHdFtlJeYfjQtTjCW865KPsQcYeC8E8KXO+TjhmhKt67RRhOvcYqSmroZLbkhfQsEKw6QbB6MxBgakBnMcCcy/c1D3p43PBfQgaye5LQucr6N03HldSHU9tcJRiQ3uKvMlqqwl0CrO5OPVefohKyExUHCNKqLt7uNL8h0ebrvXq0aFKz/E1R5zR6SaSUXRzc9kY99gfAU+EQU0zuOOao9+AzyMmYcItXMYPE/dkFLgfu2FchpCVc3YISQJvlYq9nYVQGi1KpVhrxu+cC7P62In6mKAq1JPwxmNTA+1gMkujdfM3Wvh/c/EggwQeOcS2h/FIm+zpRobtvUOHTvZbRdFdrdnY88rqD9DZqR9tzV7FAUG28a01z5OKMYFopBNylfJZmw9pivnfK2B7aN9r1jrtV4WqCoi3oWlOUG5qjYc7IuCP2gI73xjCziakLhXJzyIvKdvuVSdVMpTNqZ8Y7ye3cmfBkz5ZjxStHFITEst5ZkhCTW6jJBCTdYTqpuJtnI9Z8YTKHRArtEMMrZ xcVGkV+z 7ghduLhutMYk0nZYZ2vvGgNRQunXJ2P6X6P0tmaDWtQsqKd1z1XvcWuZYmU/rNvgMA1cKsCwwbzFxeEJ4Xg1ozkEmYXOY0oq6ct4PbZP1NfuQKvU6XE55vA3gv5cHNZutLBkXvuYIQJR4j7dTZCq1izAo9hVgrN3wHCgIct7rFC+DHzYZROUdSVbP4+SF1sMpUz5gU3QzrgtXpiCwg5k9rorD04oCd7tRddT0y6LMHfASdQGieAQOUOW3mrKJ0jZ0EGSUhikKwEtjmFFqKqLhbvFBRltg2hZfqig6wb6x+ASIQmZP9BxHQnhkjf2SmKBGdv8oqCBlzgpH66hBZtv8Ns8ThX3jtpq0z5NkImaPdGrcHGV2DnclQ8fLABW2xpjmmRmXtOInRUDjtSaC/x3mc4becYHSaYNihMVqLQUZzxz5OgmJ89JneN0D2tCtEtKvxPoX6DfKSRZHEitYrjV2RBr+oLxaM6hjqEhBCXlDP3rvsLmFo8A+wCUWTkn4pTd4/Eh/okoS2XNEp50= 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 24 Jun 2025, at 11:22, David Hildenbrand wrote: >=20 > On 24.06.25 10:07, David Hildenbrand wrote: >>>=20 >> Is that actually required? >> The man page explicitly documents: >> " EINVAL A previous UFFDIO_API call already enabled one or = more >> features for this userfaultfd. Calling UFF=E2=80=90 >> DIO_API twice, the first time with no features set, is >> explicitly allowed as per the two-step feature >> detection handshake. >> " >> So if that doesn't work, something might be broken. >=20 > CCing Nadav and Peter: >=20 > Could it be that >=20 > commit 22e5fe2a2a279d9a6fcbdfb4dffe73821bef1c90 > Author: Nadav Amit > Date: Thu Sep 2 14:58:59 2021 -0700 >=20 > userfaultfd: prevent concurrent API initialization > userfaultfd assumes that the enabled features are set once and = never > changed after UFFDIO_API ioctl succeeded. > However, currently, UFFDIO_API can be called concurrently from = two > different threads, succeed on both threads and leave userfaultfd's > features in non-deterministic state. Theoretically, other uffd = operations > (ioctl's and page-faults) can be dispatched while adversely = affected by > such changes of features. > Moreover, the writes to ctx->state and ctx->features are not = ordered, > which can - theoretically, again - let userfaultfd_ioctl() think = that > userfaultfd API completed, while the features are still not = initialized. > To avoid races, it is arguably best to get rid of ctx->state. = Since there > are only 2 states, record the API initialization in ctx->features = as the > uppermost bit and remove ctx->state. >=20 > Accidentally broke the documented two-step handshake in the man page = where we > can avoid closing + reopening the fd? I agree the code is not correct (and my patch didn=E2=80=99t address = this issue), but I don=E2=80=99t see it broke it either. Unless I=E2=80=99m missing something the code before my patch, when uffdio_api.features =3D=3D 0, also set ctx->state to UFFD_STATE_RUNNING, = which meant another invocation would see (ctx->state !=3D UFFD_STATE_WAIT_API) = and fail. >=20 > Without testing, the following might fix it if I am right: >=20 > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index 22f4bf956ba1c..f03e7c980e1c5 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -1944,9 +1944,9 @@ static int userfaultfd_move(struct = userfaultfd_ctx *ctx, > static int userfaultfd_api(struct userfaultfd_ctx *ctx, > unsigned long arg) > { > + unsigned int new_features, old_features =3D 0; > struct uffdio_api uffdio_api; > void __user *buf =3D (void __user *)arg; > - unsigned int ctx_features; > int ret; > __u64 features; > @@ -1990,9 +1990,12 @@ static int userfaultfd_api(struct = userfaultfd_ctx *ctx, > goto out; > /* only enable the requested features for this uffd context */ > - ctx_features =3D uffd_ctx_features(features); > + new_features =3D uffd_ctx_features(features); > + /* allow two-step handshake */ > + if (userfaultfd_is_initialized(ctx)) > + old_features =3D UFFD_FEATURE_INITIALIZED; > ret =3D -EINVAL; > - if (cmpxchg(&ctx->features, 0, ctx_features) !=3D 0) > + if (cmpxchg(&ctx->features, old_features, new_features) !=3D = old_features) > goto err_out; > ret =3D 0; I am not sure it is right since you would return EINVAL in this case. It also looks a bit overly complicated - are you concerned about a race? My whole concern about race was that somebody would exploit it to overcome non-cooperative UFFD (IIRC). So perhaps just add a check for the case features if 0 and be done with it? Something like adding: ret =3D 0; if (ctx->features =3D=3D 0 && features =3D=3D 0) goto err_out; /* no error but copying of = uffdio_api required */ before enabling the requested features for this uffd context.