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 C0145E7719E for ; Mon, 13 Jan 2025 10:30:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 529C46B007B; Mon, 13 Jan 2025 05:30:21 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4D8B06B0083; Mon, 13 Jan 2025 05:30:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 379C76B0085; Mon, 13 Jan 2025 05:30:21 -0500 (EST) 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 17B5C6B007B for ; Mon, 13 Jan 2025 05:30:21 -0500 (EST) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id BFB71141E09 for ; Mon, 13 Jan 2025 10:30:20 +0000 (UTC) X-FDA: 83002059000.27.19B80E3 Received: from mail-wm1-f46.google.com (mail-wm1-f46.google.com [209.85.128.46]) by imf27.hostedemail.com (Postfix) with ESMTP id BDFBE40012 for ; Mon, 13 Jan 2025 10:30:18 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=IjrFu3ym; spf=pass (imf27.hostedemail.com: domain of aliceryhl@google.com designates 209.85.128.46 as permitted sender) smtp.mailfrom=aliceryhl@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=1736764218; 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=Q6BcFT4T+We0MNxDCiiTwafBoXlHK/RucDnwCAebGy8=; b=LEzo5VL+6YcuS1eHWeSfkTmvvMRW8E1Uzr6ZA6CabfmsoPHcrkKsPvGaFeFmXr/BSxDX4R +0EkZ+/sA35/7+kaU3IPCgrQ0yne8e8J+C4csQcvzha4cmA1pqHEWQ/vAw0MQX396DCHsO wgXWH19CVSc+XFR7Eqpd6RGT4STlzFY= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736764218; a=rsa-sha256; cv=none; b=TwMum+iO3blzV5XT5X2dCz8JkbpdmiztpGQXliSxhuLE3OWsLLUn6QtmNhtO1W/+9UgUHy FZHvXUy4NZVF/UPVA5fQDaPWzYH3nnLeBeItcinPAGyUAzJvNkKEiSg1PMhDzVBaAJrdHR AniclBqXKlBERaCseVUUbPRHVWnUfpQ= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=IjrFu3ym; spf=pass (imf27.hostedemail.com: domain of aliceryhl@google.com designates 209.85.128.46 as permitted sender) smtp.mailfrom=aliceryhl@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-wm1-f46.google.com with SMTP id 5b1f17b1804b1-43621d27adeso28076235e9.2 for ; Mon, 13 Jan 2025 02:30:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1736764217; x=1737369017; 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=Q6BcFT4T+We0MNxDCiiTwafBoXlHK/RucDnwCAebGy8=; b=IjrFu3ymhywGgSpw5iGtxN0EHwL/X8EMOX9mvFSsxqU2R3sN7Vg0+QLVtHlM5HXcbl wbmv+hGWMJbjhrE9XjjsSJ1r8xOaS3nxoXTpJHO6uLg/QJ4ezcl1wV61O8iSgQGj3KIN 3Yk0qsSAIC1EzTyf8efjRGWZbrsws0f3M3ErU4tbWVE3lE7UqLMLT6V/jmmZo+V50yiI Dc5lPClHFkAl41sgXiPBEzbhieJfgNKlKgNmjqygMqqWL0uIMsZa7sdbCkfm+QQP3DW5 sbfcVKBkzD224f6DBvYBIDID82f/ErZAKmZ6Y3mPkUy6mB0w2uNWMDm1NnJ1RNMPVGYC 1M3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736764217; x=1737369017; 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=Q6BcFT4T+We0MNxDCiiTwafBoXlHK/RucDnwCAebGy8=; b=MGj3YvIsrTuSPNrfWv3/DHkNMzttsqSeogPnR69xMLY+tpa8ak5WcxCzFKWLd9MAK0 UyCWcuhyPjJcFoqOAK15q6kraY6rHbdWQs60SmpD5PcfmKaHLtsKtgT5sNeDiET27pio rBYmMsTC0ilYfQDSZpE0zVGxWl8TUgyDAgbBZylxX6y0NLtQbf2UY0NIe3Ah/Dg1oL8U lHB4kPZogO7zKhB4D7/vEafnBO3y0RhYyS1OJ/zM+0kpOOpkFg71F8j0eL1fuSTI/LAA FsZM91bnlhl/FHxJdf1NA9nYVaYhduY5cKmDi5XNQVMAdGKuy++rtHpDqN4JP7qgJWFR hs3g== X-Forwarded-Encrypted: i=1; AJvYcCWdPIUj99pOSXYEnO/9OXqskXBTCEwSE6m6XK+PDTOnQt6Eh9UzqSzqKvCmITzvFlOBkZ4mGhrmwQ==@kvack.org X-Gm-Message-State: AOJu0Yw+E16QsVzWoayi/AWUd+uyS74cyYK3/6tExjcxkzzGwlTYY1Nn KdeTSh6uJ1Lp7Zz00yMDVxlWaujWJ08jV0yfBC7leX4wBvc6GKhDU/Rb/0KoRsefGogUZvBLP3l r2MS9OIq/tE2zyXO6YkJnESG3gljRfa7aVqgH X-Gm-Gg: ASbGnctLJ+nUEXktVhd0FCnZuEKm7Ghyaes20qhBLg03DUOBx51EOBaDyIu/gshVrbH Hit0mD9eXETlmByDz2r9THvIIjrxi4Uk5mqSflPCXK11bUPLk+d5hPruDIQnVUDR1wXRe X-Google-Smtp-Source: AGHT+IFWb2oyTOjJX/o3HSNnXeHPflZbUYxMxvifF7/xMk5dQsb+1KcZXFEGg1ps+RkKlLpxr9haE/A/djl1U0bHtYo= X-Received: by 2002:a05:600c:354e:b0:431:5aea:95f with SMTP id 5b1f17b1804b1-436e26a175cmr218244085e9.16.1736764217229; Mon, 13 Jan 2025 02:30:17 -0800 (PST) MIME-Version: 1.0 References: <20241211-vma-v11-0-466640428fc3@google.com> <20241211-vma-v11-8-466640428fc3@google.com> In-Reply-To: From: Alice Ryhl Date: Mon, 13 Jan 2025 11:30:05 +0100 X-Gm-Features: AbW1kvZmBQ72-6O2pqLwyAyR2WjjcEyxesG4j5T1ajEwgzoqW67phRCjjvS6z7U Message-ID: Subject: Re: [PATCH v11 8/8] task: rust: rework how current is accessed To: Boqun Feng Cc: Miguel Ojeda , Matthew Wilcox , Lorenzo Stoakes , Vlastimil Babka , John Hubbard , "Liam R. Howlett" , Andrew Morton , Greg Kroah-Hartman , Arnd Bergmann , Christian Brauner , Jann Horn , Suren Baghdasaryan , Alex Gaynor , Gary Guo , =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= , Benno Lossin , Andreas Hindborg , Trevor Gross , linux-kernel@vger.kernel.org, linux-mm@kvack.org, rust-for-linux@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: BDFBE40012 X-Rspam-User: X-Rspamd-Server: rspam07 X-Stat-Signature: 1wtaz94apmpfdkegf5njk3aawzzgs1j1 X-HE-Tag: 1736764218-660292 X-HE-Meta: U2FsdGVkX1/Sfxb4iKOec7yOK/zlUukvZfBZx8FWWkAwKvCl4Lf1Qsltr8dRzjj+RCdrf5oUqsvSI06w8nmbunwh7k1h/DbpWZ9dkExPd/bYXZNxktZuYhknNVBRrZTUtN/ettL2K2Y4dEUy1zu16lntaRd/5P8GCRxmlIljXcORKBoCnVI9uAdljn8NEJp9HuikRZ7aS/9r2Vv48H7YfksW7eSXNGtqqDHzCm//tDIgxbXCQTj+GmEPubL2VNOWNolKgU0c+chvYEe1TB8kNyJh3ZPG7RbRsoKsgrZek1nVbYAIWQUNfhnRMQT2UvBucuY2Dyo4DU2mvacvZHIr4GzwbHVF6OUqAEh3P1nS1GpfJf+ENvUJDjCEJn+w27fY3ql2SaC7MfhQ/S3snt5bgCwLY75Vj7/dGEeoAYlXMdbE/Lf9dORF53vHSy3m1T4t0pQgEfx8uSsK7jAv3UAiQOL/tPnkgu30xSjEqxgfmJwCfRZE4R75qul5onm3zcO8j2ZLRaBITvufkhadlJ5K3V9Kq5kQ/mERBocPYH7LPuLj30iDv8xeErPIbdZmd2Gvi1cgTm8q6xttjb0js7L5yq1+KJFo3LZVJRRLbMACzlGyXogwbeMsjgTv16lBe67WAHvu+rR/k39fUnaO6pr66ctpymFhZrxfMBucuWgHS/KIqsyJqcGEyLjWBVKgQ3RybDs3xZj78Wpa+4Uvng+TVND3PZ5FK74sM412CucZFk4v2UoxJjUfsTd6gGunKSrb8zh/btI7cmAf7Mzw2+fLhEsfL+n3qb3n+NhbaCbFc3XfwUPGKlv5osb86u5IcXtXo5rpCB93ZVcW1VOkFWByTocTZDJMWZ3jymTMQ25kLeuvQgTNiJj2eW6VnfccmnfnCjL0qcycgbHV7gLJ/V9DlGNDg2yS6Hxrgpr4RFk9Ou4WIFtgMsnFNYTzs/1bFfnvwV62Y0k+/SPCHJAMfSH kMvoJsav 8vazKmLz0n3j64HSc4xwhYLqBYtv5SJD0B6fCotCvnvVsylUL3tRzVSaxCHFuWwr2VNmuwGX898Ge4NBGGGqlMAsUxufNX692fF5XkXLpW1r7WPlwS51NgE2zfat9WJnVaY+mb3cTrk+xEtRfe1aRDr4Q6IKDNFjl8/QOUFRO+LmUXH5dn0pTvFxvrCxEtE7lqBvHXPsK2+nEqTsQcrgXvLCQiCmMLGPdnxlLZ5POkx5cI9dBATxynttnwbdYGjb25g37MrOnbX+GTlhl+fZaGbaVhD5N/IpXt7Tysg1EJAfVX2xFma0pnT7+e4Snf/aOz6r1lyk8YEUkmAX/S7Cc8NIjGA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.382247, 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 Tue, Dec 17, 2024 at 12:40=E2=80=AFAM Boqun Feng = wrote: > > On Wed, Dec 11, 2024 at 10:37:12AM +0000, Alice Ryhl wrote: > > Introduce a new type called `CurrentTask` that lets you perform various > > operations that are only safe on the `current` task. Use the new type t= o > > provide a way to access the current mm without incrementing its > > refcount. > > > > With this change, you can write stuff such as > > > > let vma =3D current!().mm().lock_vma_under_rcu(addr); > > > > without incrementing any refcounts. > > > > This replaces the existing abstractions for accessing the current pid > > namespace. With the old approach, every field access to current involve= s > > both a macro and a unsafe helper function. The new approach simplifies > > that to a single safe function on the `CurrentTask` type. This makes it > > less heavy-weight to add additional current accessors in the future. > > > > That said, creating a `CurrentTask` type like the one in this patch > > requires that we are careful to ensure that it cannot escape the curren= t > > task or otherwise access things after they are freed. To do this, I > > declared that it cannot escape the current "task context" where I > > defined a "task context" as essentially the region in which `current` > > remains unchanged. So e.g., release_task() or begin_new_exec() would > > leave the task context. > > > > If a userspace thread returns to userspace and later makes another > > syscall, then I consider the two syscalls to be different task contexts= . > > This allows values stored in that task to be modified between syscalls, > > even if they're guaranteed to be immutable during a syscall. > > > > Ensuring correctness of `CurrentTask` is slightly tricky if we also wan= t > > the ability to have a safe `kthread_use_mm()` implementation in Rust. T= o > > support that safely, there are two patterns we need to ensure are safe: > > > > // Case 1: current!() called inside the scope. > > let mm; > > kthread_use_mm(some_mm, || { > > mm =3D current!().mm(); > > }); > > drop(some_mm); > > mm.do_something(); // UAF > > > > and: > > > > // Case 2: current!() called before the scope. > > let mm; > > let task =3D current!(); > > kthread_use_mm(some_mm, || { > > mm =3D task.mm(); > > }); > > drop(some_mm); > > mm.do_something(); // UAF > > > > The existing `current!()` abstraction already natively prevents the > > first case: The `&CurrentTask` would be tied to the inner scope, so the > > borrow-checker ensures that no reference derived from it can escape the > > scope. > > > > Fixing the second case is a bit more tricky. The solution is to > > essentially pretend that the contents of the scope execute on an > > different thread, which means that only thread-safe types can cross the > > boundary. Since `CurrentTask` is marked `NotThreadSafe`, attempts to > > move it to another thread will fail, and this includes our fake pretend > > thread boundary. > > > > This has the disadvantage that other types that aren't thread-safe for > > reasons unrelated to `current` also cannot be moved across the > > `kthread_use_mm()` boundary. I consider this an acceptable tradeoff. > > > > Cc: Christian Brauner > > Signed-off-by: Alice Ryhl > > --- > > rust/kernel/mm.rs | 22 ---- > > rust/kernel/task.rs | 284 ++++++++++++++++++++++++++++++--------------= -------- > > 2 files changed, 167 insertions(+), 139 deletions(-) > > > > diff --git a/rust/kernel/mm.rs b/rust/kernel/mm.rs > > index 50f4861ae4b9..f7d1079391ef 100644 > > --- a/rust/kernel/mm.rs > > +++ b/rust/kernel/mm.rs > > @@ -142,28 +142,6 @@ fn deref(&self) -> &MmWithUser { > > > > // These methods are safe to call even if `mm_users` is zero. > > impl Mm { > > - /// Call `mmgrab` on `current.mm`. > > - #[inline] > > - pub fn mmgrab_current() -> Option> { > > - // SAFETY: It's safe to get the `mm` field from current. > > - let mm =3D unsafe { > > - let current =3D bindings::get_current(); > > - (*current).mm > > - }; > > - > > - if mm.is_null() { > > - return None; > > - } > > - > > - // SAFETY: The value of `current->mm` is guaranteed to be null= or a valid `mm_struct`. We > > - // just checked that it's not null. Furthermore, the returned = `&Mm` is valid only for the > > - // duration of this function, and `current->mm` will stay vali= d for that long. > > - let mm =3D unsafe { Mm::from_raw(mm) }; > > - > > - // This increments the refcount using `mmgrab`. > > - Some(ARef::from(mm)) > > - } > > - > > This is removed because of no user? If so, maybe don't introduce this at > all in the earlier patch of this series? The rest looks good to me. I guess I can drop the temporary introduction of this. It's here due to the history of this series where originally it only had mmgrab_current, and Binder would use that. But with this patch, you can use CurrentTask::mm() + ARef::from() to do the same thing. For Binder, the difference doesn't matter, but the latter is more powerful as you can access the current task's mm_struct without incrementing refcounts. Alice