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 98095E77183 for ; Mon, 16 Dec 2024 23:40:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2DFE66B00AA; Mon, 16 Dec 2024 18:40:47 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2679A6B00AD; Mon, 16 Dec 2024 18:40:47 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 109246B00AE; Mon, 16 Dec 2024 18:40:47 -0500 (EST) 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 E30C76B00AA for ; Mon, 16 Dec 2024 18:40:46 -0500 (EST) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 8EA67AE403 for ; Mon, 16 Dec 2024 23:40:46 +0000 (UTC) X-FDA: 82902442350.20.E468105 Received: from mail-qt1-f174.google.com (mail-qt1-f174.google.com [209.85.160.174]) by imf24.hostedemail.com (Postfix) with ESMTP id 60BFC18000F for ; Mon, 16 Dec 2024 23:40:40 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=fznBYNcr; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf24.hostedemail.com: domain of boqun.feng@gmail.com designates 209.85.160.174 as permitted sender) smtp.mailfrom=boqun.feng@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1734392416; a=rsa-sha256; cv=none; b=OTvPsCkQJENdezrahzWdNZ0nXa/B9BAk0XvmawVfSF9Psv4VREaxeYNEpe4plf7EaWGWQl l5ZVaVz5v0Q+kooofu4kyxsvaT00yoKoL+2gNE0EtUdCVXXImwpmqHv0bNkI0swVJQAgYX wdDGkq4L/zX5+1PipE9U2HzdwrCzSq8= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=fznBYNcr; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf24.hostedemail.com: domain of boqun.feng@gmail.com designates 209.85.160.174 as permitted sender) smtp.mailfrom=boqun.feng@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1734392416; 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=7V08DEnZfpKxBekBIJyGIOyXsH+kw7fY8zsFNeDXVqE=; b=pfAo3tWd2QdKMQ+CzKaNVIMJDnjYzBtkBaNAG9HCn8ArcXfUeNLwM790mH79MwnS7HyZXn LJyPdtPgXCkF5Bun0uywbQzw72CoZKe6DsWaiGk/JcZz00hCDMuMwgHHXWf4Z5/b15sGBl 1wQidkKntlVlxy2If5yt9ciBDyfOTp0= Received: by mail-qt1-f174.google.com with SMTP id d75a77b69052e-46785fbb949so53618571cf.3 for ; Mon, 16 Dec 2024 15:40:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1734392443; x=1734997243; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:feedback-id:from:to:cc:subject:date :message-id:reply-to; bh=7V08DEnZfpKxBekBIJyGIOyXsH+kw7fY8zsFNeDXVqE=; b=fznBYNcrCaJysvo+aUQHHUUd+IlFb5dSV3zA92ceNFaulnjhaJICN4QB14THK1k1Kh GSNqYXSKRxEW3erOgiA4iGL5qShHcdAErlTzdHEy3w1vQyVstF1weAKj4n4LN4tfV6hm UXSduFQGD2FcATltJzIPAFycYp4Tgsdp2h3i5dQ+7TZO0WWHS0iAQxhGMLa9eyUzhQpb Qb6YQNlku545SGRUI4s78RGRt+owV5cGrM8Bn1kmdgpM+9V1lw/BsqhxoUTo4BRnlzNX 9ZD+BLSMDuSZXN+AprJY9OMMM9Y2V91FPePcctWcQYeAycbP6q/VQ2e0mkDPCVfXclss bV0Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734392443; x=1734997243; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:feedback-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=7V08DEnZfpKxBekBIJyGIOyXsH+kw7fY8zsFNeDXVqE=; b=VXmg+g0JvNrOcdwdHxrR76ZdJEeFnNt0HzL2cG3nGOnIPAUuxtPqZwNCR6ZtEA52Hi FMUdPg8FIv4zkPg/BxPYleAIbb7KbFdUuEbPFDMcutSG6wvqiY5bHUkjJFNSpnugpeNj Hd8lf4kXx9arUmyqCjl6IPkUmYgDDNK+h5ZakHpC/bbZKPS9v6WYsl7TqldVLxxKz5NZ yDJNBT9g59VSyhG0PzpxVLfF7psci1SgL1PjzSuLEVYaFpzO3uU3Yy2Z24U52BcmEv5o +3eszbpJyUUdMSkvCSE36WsC/uNv3njzaMUXS6JKf7MAAfdwmL3pwDuj0yx2nDjBa/Ks B/WQ== X-Forwarded-Encrypted: i=1; AJvYcCUyHvbNAltayJCw093jeDzYw0EYcmiRjFM3U9bfT7jYhNMhFquXtOAem47JM1MIGknB0AtSc/WJuA==@kvack.org X-Gm-Message-State: AOJu0Yw2GOqfCLi/qrAkSOocQQV2Z5FxGDsVGkxoGb/DVSNidC8ua8+W Om8kXmqSor/4zq2An1xDWAzWZqpbuV3AOD/0Yv+1hI1HHo/6bReD X-Gm-Gg: ASbGncu3C1gAs9RxFAUsXSVAipGlplJSJolcpyop3bU80VV+ik3ZGmalVhY/t6Doro1 +zQo0bn4GW6PMM8BANfXI8mYygKazebWf2iG2S66pvwMx2ohOx1JlIYEpvbYurPFg3RD1M+qHyx hb9xRqsCtH0aE+v3VlcpLBAvN1p7JMl2PWN5++1lX7eFH2X0M9khnVIQ4Ne09GlZ/evVuvS7mRo dHNV9cHy+TLK2/AHi3ZYt0+Hshgf5f52yrBRJ1V9xmqJ5BzR3bMH601WxsBpJmKRGtr77tjNbQM YwN5TiU5DYiIHWN5T24EkvhvhLE4yrLb1F23Yk9MRbkrcxo= X-Google-Smtp-Source: AGHT+IE+R/i05xOcYuGx3CfeQWjqyk51P8QEx7sJ9GSjo27w65ZKSBhtkEi/1BOtkUVqsIGrFOr2LQ== X-Received: by 2002:ad4:5443:0:b0:6d8:a39e:32a4 with SMTP id 6a1803df08f44-6dc8ca8ef8cmr211818766d6.25.1734392443062; Mon, 16 Dec 2024 15:40:43 -0800 (PST) Received: from fauth-a2-smtp.messagingengine.com (fauth-a2-smtp.messagingengine.com. [103.168.172.201]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6dccd257075sm32856486d6.35.2024.12.16.15.40.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 16 Dec 2024 15:40:42 -0800 (PST) Received: from phl-compute-04.internal (phl-compute-04.phl.internal [10.202.2.44]) by mailfauth.phl.internal (Postfix) with ESMTP id A31161200043; Mon, 16 Dec 2024 18:40:41 -0500 (EST) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-04.internal (MEProxy); Mon, 16 Dec 2024 18:40:41 -0500 X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefuddrleeggddufecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdpuffr tefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnth hsucdlqddutddtmdenucfjughrpeffhffvvefukfhfgggtuggjsehttdertddttddvnecu hfhrohhmpeeuohhquhhnucfhvghnghcuoegsohhquhhnrdhfvghnghesghhmrghilhdrtg homheqnecuggftrfgrthhtvghrnhephedugfduffffteeutddvheeuveelvdfhleelieev tdeguefhgeeuveeiudffiedvnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpe hmrghilhhfrhhomhepsghoqhhunhdomhgvshhmthhprghuthhhphgvrhhsohhnrghlihht hidqieelvdeghedtieegqddujeejkeehheehvddqsghoqhhunhdrfhgvnhhgpeepghhmrg hilhdrtghomhesfhhigihmvgdrnhgrmhgvpdhnsggprhgtphhtthhopedvfedpmhhouggv pehsmhhtphhouhhtpdhrtghpthhtoheprghlihgtvghrhihhlhesghhoohhglhgvrdgtoh hmpdhrtghpthhtohepohhjvggurgeskhgvrhhnvghlrdhorhhgpdhrtghpthhtohepfihi lhhlhiesihhnfhhrrgguvggrugdrohhrghdprhgtphhtthhopehlohhrvghniihordhsth horghkvghssehorhgrtghlvgdrtghomhdprhgtphhtthhopehvsggrsghkrgesshhushgv rdgtiidprhgtphhtthhopehjhhhusggsrghrugesnhhvihguihgrrdgtohhmpdhrtghpth htoheplhhirghmrdhhohiflhgvthhtsehorhgrtghlvgdrtghomhdprhgtphhtthhopegr khhpmheslhhinhhugidqfhhouhhnuggrthhiohhnrdhorhhgpdhrtghpthhtohepghhrvg hgkhhhsehlihhnuhigfhhouhhnuggrthhiohhnrdhorhhg X-ME-Proxy: Feedback-ID: iad51458e:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 16 Dec 2024 18:40:41 -0500 (EST) Date: Mon, 16 Dec 2024 15:40:40 -0800 From: Boqun Feng To: Alice Ryhl 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 , =?iso-8859-1?Q?Bj=F6rn?= Roy Baron , Benno Lossin , Andreas Hindborg , Trevor Gross , linux-kernel@vger.kernel.org, linux-mm@kvack.org, rust-for-linux@vger.kernel.org Subject: Re: [PATCH v11 8/8] task: rust: rework how current is accessed Message-ID: References: <20241211-vma-v11-0-466640428fc3@google.com> <20241211-vma-v11-8-466640428fc3@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20241211-vma-v11-8-466640428fc3@google.com> X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 60BFC18000F X-Stat-Signature: iy6c8knmqijoy31ngmni9pg5f9hsb1eq X-Rspam-User: X-HE-Tag: 1734392440-998790 X-HE-Meta: U2FsdGVkX18T+l5c148wz2cINW/6syhFXlNzQXoTOJVg9a/asIpjLaOvZ6ZIOr0kEm4JdBG0D9fNvIMxvAk8wGKhc1e3iA9D0nC8Hxkp9El3MdnJu8g7SwlQi6aadjDHCgDmxCDQzQ7xjfU+3BajhRTmFXHin5MKRg50MIvSNczT81n32UyOrFfafEhDPxGN3+/715TMIMX9JYhsz1Q3IcmRLWnxYYqNoDbi+MFw14hn2SDPAWDLWEY3s0W63nPNDsquJZcfob1VRnB7PkF5qIl96HvIWm6JS29pjnjZCdVN+bAT0vLW0uNcwFSrvM/pfuKCAs5sZlMK+Jw5DyXbMCwpIB0E5bB9vHo6IfGbGo8fIOCclZM3dJFU8L7UUJTCXSCDzVqerxv8B60rEnzC0eNlJ9dxQhuRIfRjABxwgQifCZMYtZLZKWVcFJ0mDiHJE41Emfemvl33S/Kj7qm290FgidTO8SZuo7KfsIEcuubtnP891obxR5VYHBs8SNvjDjkGbHs0Emc7qtHK5JHeL5BneifshJnh7SGZCDAeZdf7vlwn8bNoF8CauDGFiVOIz6s1BbGh90sIpHkztkZSC2gS1sN8xtJACI5neWGQ7j0bGXdAAXAQGhYUfX2QAP06bJwRWLdxWJ0sJhnZSK360Q/JVmNF6uozlsNKrFvhB1BAwYhL0NW+9hbI7/fJ7DJ8dwPybyxmo27AfTsLQmhiNp+OGbCq0U0N1Yd/599ekKKPr5yPAcrT2uA+jFIajk2SQKd7nNELG6WBdePrJOajkY92LoVSABoL8/+iehnf0gmbusjotbwGWrClamyKsVM6kHqTo38jahO6aqb7TfXbwIE9nPRfLjjq3MZtrVMatpBrGWjKnij9b1Bgb32gtwdqVJUk2Zui9WdQczgQOhyZXW51YNN+7GNWnqbMKyI1DfP/azk4xRTAjcQp/YvGLDZ51CDoLC8CZRiPeDCbdBn FmwnlAzl Spu5yYN1gAdcQBf0odQzdGI2VvBmsMOdie5Y0YQmpHmk8qgkntKV/AcMR9ndLRZpRK8M8Qull7xAeOJE1YdYzW2JyQGIRTQcp1f505cRIaPJyorq59GOkE2kXpYNafGQEecUrothFR9JamsL/x0S3/920NBgRdd8Nd5rTariI6srTQBUtKI666zW034138ddikajdiQHjHhSz1ZJlFlMOjp4AHF/91WhWerCB/AWvzplGImxhyltz3puIIvKD+qxrp9gCUKPjHF5U5BsWJNIulT6X0wBkhDtT3mJAB6X0w2uDJWvt2r08gPRgt9ARReMZcPknAhnpgVzGY+dA3yCE+lE/6qu39gao+j7hMqk+L/dadM82WBJeJAUQVD/s5RCcJ2x1BDzgt5EYqIWfh1NkscHel6k5kf9mymgjSy9yuLpznlS0in50ts9hnBa9G7mBdXRl7xEBNG3a6WNAo54AB2yu1AwhmWBWOecFj1WBmxZB+XYTBtwpklJdfA== X-Bogosity: Unsure, tests=bogofilter, spamicity=0.491210, 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, 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 to > provide a way to access the current mm without incrementing its > refcount. > > With this change, you can write stuff such as > > let vma = 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 involves > 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 current > 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 want > the ability to have a safe `kthread_use_mm()` implementation in Rust. To > 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 = current!().mm(); > }); > drop(some_mm); > mm.do_something(); // UAF > > and: > > // Case 2: current!() called before the scope. > let mm; > let task = current!(); > kthread_use_mm(some_mm, || { > mm = 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 = unsafe { > - let current = 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 valid for that long. > - let mm = 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. Regards, Boqun > /// Returns a raw pointer to the inner `mm_struct`. > #[inline] > pub fn as_raw(&self) -> *mut bindings::mm_struct { > diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs > index 07bc22a7645c..8c1ee46c03eb 100644 > --- a/rust/kernel/task.rs > +++ b/rust/kernel/task.rs > @@ -7,6 +7,7 @@ > use crate::{ > bindings, > ffi::{c_int, c_long, c_uint}, > + mm::MmWithUser, > pid_namespace::PidNamespace, > types::{ARef, NotThreadSafe, Opaque}, > }; > @@ -31,22 +32,20 @@ [...]