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 A8C97E77188 for ; Tue, 14 Jan 2025 15:31:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1FE1B6B007B; Tue, 14 Jan 2025 10:31:52 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 1874B6B0083; Tue, 14 Jan 2025 10:31:52 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EF4936B0085; Tue, 14 Jan 2025 10:31:51 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id C85EA6B007B for ; Tue, 14 Jan 2025 10:31:51 -0500 (EST) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 8E7B7AEB85 for ; Tue, 14 Jan 2025 15:31:51 +0000 (UTC) X-FDA: 83006447622.27.324099B Received: from mail-qv1-f54.google.com (mail-qv1-f54.google.com [209.85.219.54]) by imf21.hostedemail.com (Postfix) with ESMTP id 4288A1C001A for ; Tue, 14 Jan 2025 15:31:49 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=TaaKgfM9; spf=pass (imf21.hostedemail.com: domain of boqun.feng@gmail.com designates 209.85.219.54 as permitted sender) smtp.mailfrom=boqun.feng@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=1736868709; 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=4YGqaBjrL5N1TiL6bnUVKqifu+Ce60WP5265OR5kAs8=; b=2rnf0ugByOgouKGkRovH6nQ0aHk/wxPBxcBow4cp2vt1gdTotHYiOnZfVpFGXH9HYVs6rv OVL/QFALTtopOv2kSsVz2nwBVt+Tg2G9eFp9hHgF+gmf2B5d4WEDwKOuVfFz3GieRc+X2U V32oFA+n1HLEeNE2dQkBV7QYJlzJhqk= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=TaaKgfM9; spf=pass (imf21.hostedemail.com: domain of boqun.feng@gmail.com designates 209.85.219.54 as permitted sender) smtp.mailfrom=boqun.feng@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736868709; a=rsa-sha256; cv=none; b=oCUHpYX9wmkr4JoAMijhr/vPEtaofvyoITGZRK9a2/By++LvHS5V11F19daA51BIpbGuq3 mVePLw7PCJBqYW444VLSrYR8VXvCu6R7aby6pITKa0rtz66zE2sSV2qaRCX+kj/JgHMqfN smicI8c1hJLjJdU9W05+qZUFvnxbAdQ= Received: by mail-qv1-f54.google.com with SMTP id 6a1803df08f44-6dfbc45355bso34448756d6.2 for ; Tue, 14 Jan 2025 07:31:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736868708; x=1737473508; darn=kvack.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :feedback-id:from:to:cc:subject:date:message-id:reply-to; bh=4YGqaBjrL5N1TiL6bnUVKqifu+Ce60WP5265OR5kAs8=; b=TaaKgfM9RWP6mZgHitKgnph+XXat3nWsf8N60HchE9ZdUqluXN1zUOV3JyCxElP3CF qzJiZWAVU+ucxRILBjfNLQw3dNrq7IONNsCDn2nFswtC+Xo460PIoSIR/TH90YLXFPbw tLUVhh140ukA8OYfS8gl6gWNUax935zFNcs/kd4jPEcZ6pTIQN27g3nQxcgrJvMIcdlh tvGSTDH4cFGTod+B3R6FDlqUhJ5TnUhVKeu9UG9Fb14GBwmxnjha5pyG+vt/IOedIaF0 H5ZU8mz4Iw4vKCkMqdld5KrL8r84sMcsxXkmVPodQzF3WdNLdzs4+V7S9BVflS1PGqCI p3tw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736868708; x=1737473508; h=in-reply-to:content-transfer-encoding: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=4YGqaBjrL5N1TiL6bnUVKqifu+Ce60WP5265OR5kAs8=; b=bWxvhjPGDMt41G4SNL6G/SuRyYgBxvZzyHAWHpG88H+b7lfgbO6bsUYMnKs1nMT+0p uPt93up8BhO6YZhqAc4wyrOXl8u4J7x3ZyGR9x9psKxWODk1H0/8pXm7ow3LPrAIujr7 wGFNi7+w323uGHPpOEzhrIUiP4+1H8BZRu57EhwejZWrHdqjmVrPpJO0+oBTYepNJbm4 cz6TAywChPqGi2ZrdlqSs7ZzGPu7OenCmxxlwJwz6gBUcM11t6TDjvkA4WxS/ymjqAZf RfzpiBdKJNt45cCubKVXdGT06rmLL0TzRW4Wp+bQ6ot0qWOmOYu0QutMMGpE7+u6gS/9 Bnaw== X-Forwarded-Encrypted: i=1; AJvYcCXhzrJWcItDZMc7bJNz8QKDRI1pcpMQTHij8MqwaNs39q9ddJ8mx4Gza/bEV1xkPTubbDIloB7K2w==@kvack.org X-Gm-Message-State: AOJu0Yy7ARTrf5z5liQn+OyZlb59I6/3nXGah4LtY10ryDJDPZON0sYI sPJ6QrCbnV3RjUeYWYzmdURfvqP8h08XmCs+xPN0abxpz7YiaVSX X-Gm-Gg: ASbGncutA+CFkjU6PYWbg5rPjiIBVyOeCQjB5zKZq/jxORAl+HBfxk3R7yPD9ifw+H9 /XMbMcsXJk7GIoIqf32KXdyjZq3aZDKtW4fEddsH8z6zOACbks4pKkZq38zuKQjejf8RfAdZBFK rBppnuzu+L+HUAZjpigLw/o0QalEZF64y/Na1hZf07Ink8bCrFtkPBf6fU2uIXZGhQi8pzTH4S8 KGX4dpdOuAQQnAwPG4EoIGNl6vj9JPylFTXdrwruIYRY3WB5evi+vCA9Jbr/M+mm9j6foLHf6/D 2pc5zM7MArnv00VO1MIxRx77AJcsGAVL6npFUpHbHorheeE= X-Google-Smtp-Source: AGHT+IHzBG/FLEMK050VdhMv0b8TqzaFK952QqudUpznb/pNNsKQV4bAqSp3RRURwZglCPvFSrYVkQ== X-Received: by 2002:ad4:560f:0:b0:6e1:5076:c3ff with SMTP id 6a1803df08f44-6e15076c864mr160477066d6.36.1736868708145; Tue, 14 Jan 2025 07:31:48 -0800 (PST) Received: from fauth-a1-smtp.messagingengine.com (fauth-a1-smtp.messagingengine.com. [103.168.172.200]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6e19084d07dsm374156d6.35.2025.01.14.07.31.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Jan 2025 07:31:47 -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 DE0C2120006A; Tue, 14 Jan 2025 10:31:46 -0500 (EST) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-04.internal (MEProxy); Tue, 14 Jan 2025 10:31:46 -0500 X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefuddrudehiedgjeejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggugfgjsehtkeertddttdej necuhfhrohhmpeeuohhquhhnucfhvghnghcuoegsohhquhhnrdhfvghnghesghhmrghilh drtghomheqnecuggftrfgrthhtvghrnhepvefghfeuveekudetgfevudeuudejfeeltdfh gfehgeekkeeigfdukefhgfegleefnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrg hmpehmrghilhhfrhhomhepsghoqhhunhdomhgvshhmthhprghuthhhphgvrhhsohhnrghl ihhthidqieelvdeghedtieegqddujeejkeehheehvddqsghoqhhunhdrfhgvnhhgpeepgh hmrghilhdrtghomhesfhhigihmvgdrnhgrmhgvpdhnsggprhgtphhtthhopedvfedpmhho uggvpehsmhhtphhouhhtpdhrtghpthhtoheprghlihgtvghrhihhlhesghhoohhglhgvrd gtohhmpdhrtghpthhtohepohhjvggurgeskhgvrhhnvghlrdhorhhgpdhrtghpthhtohep fihilhhlhiesihhnfhhrrgguvggrugdrohhrghdprhgtphhtthhopehlohhrvghniihord hsthhorghkvghssehorhgrtghlvgdrtghomhdprhgtphhtthhopehvsggrsghkrgesshhu shgvrdgtiidprhgtphhtthhopehjhhhusggsrghrugesnhhvihguihgrrdgtohhmpdhrtg hpthhtoheplhhirghmrdhhohiflhgvthhtsehorhgrtghlvgdrtghomhdprhgtphhtthho pegrkhhpmheslhhinhhugidqfhhouhhnuggrthhiohhnrdhorhhgpdhrtghpthhtohepgh hrvghgkhhhsehlihhnuhigfhhouhhnuggrthhiohhnrdhorhhg X-ME-Proxy: Feedback-ID: iad51458e:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 14 Jan 2025 10:31:46 -0500 (EST) Date: Tue, 14 Jan 2025 07:30:30 -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=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 4288A1C001A X-Rspam-User: X-Stat-Signature: enw5k33bsuk53agpcyseyx7rdjp9afkt X-HE-Tag: 1736868709-115639 X-HE-Meta: U2FsdGVkX19quxvT0YpLe8R3GxEx9GTKYuqR9LGzxgDwdbgQz62ikd3+hP8o7EV+l393Hfdo+kYOyVsne9c9ZmMEzmBQzmDR8EpEuKT+6+IJ46/cLqKjBq8VRTHqPho2Q8CVY/KcQfmkbd/sLwDxFK4K7FXN4uQUjQEq4NjFp+emEJbsiDMHb+ZiyxeyUFF0xdpTpHAVyZkI0TBw+YVsKWcp5GYQzElILSsVcHJ4AATDTa9WyVkJpjGmJCa85eP2fnyWg0XteBF6tQ9ne95GmidFVsfBzXeiccg/44Isu5QBvb0WPw/7VBr3R5tqDf2wt6jFTUrGk5oN/MF5D5/7r0fqYXFSOgm8O97RQPUHGt1D89cjOpshSvnh4TLBWbMMqHKMwQCNkFsj/nsNgBdNwM+lx4y2g5VkFdRS/EkszsmP7tNQjvYxLnfZqqkXQa7vCDlwQjqGG+Qcbf3FEimCIhJdI8xQK946Jjzczb9ND2RE76+ARQPkBbkMmXqw2wBQt+GJs48d/HXqduiacyuw8HNZeSEa8jzdWLZU9CHYyTZdYgHAi/Gtef/dfA1UEdKn7NpO+Tf/8ssFfxIM5C8gVfJ0v37fHLeW0F/gns0O5LAdpBzY/ukUpTK7jyztpBGp9QQf4ytuOczMPWdHMLL4cqI1q9QUG5XZtpXck9MZMSqLIw4aD0IpddXdQ6gdsI82sYjrxQ9mqttm47jbzmgMfOw2zXF05z/fpiIR//i3+MACAXtRZ2Iu7Mo+ijGE1nj1xuCqqN4Hu9QOtRmk5PTX0X/eFetj+f/EPVmxOFwe6X0aThn38ys1ojydGuSvLeKhTFf8LTvgMxR8nvTUR8cgy655S3W8vP+DqYf5OD30Y8vSU2gCfGn8Ko2fh1Ru8FXTIh3WnRXr3VvMhbdNtguaWm0WhaxoGiq7+B0tuXrYul7dl7h0vvJ2n1DCntVj2BrlwgGAK3Uxu0pB9DfIcw0 Fv1cjF7U apgZ4ZFv2JmDx3eUYU3yCUWfxFTOMj/vvhqiNdsHLzzdEXezjfrgL63P4/zWHpxzG1uL5SLOLArNk0N1IYy1lUvvs1kKHDyJnXN2prsffwB2kwgEl+sn2OEHeEcSXKzhRdnkEJjR35tiXBExASbxSK75RGfKaHbAy8qSvNN4nUZupO2ZEQKaefqMmXYbxMCq/2uTPsacJvRawr20wlJCqaxufvtbcjZ6L/QP2hPoGqq5kRQq474yZsGHnWaS2vFfHqzVprPB79a2zFtZX1xW7iNx/TTE2UHkE13o5xIb4mULVBLojh/aoJOsjDFkLmVyDhMXt39zCMWUUBCLVcEK9IOgw3dgV+y4s66vo08dR65/9YkFV9VCVAD1SOyuxNEAW7ZPCdjw7kSh/UdCI2whD+NdUWzf35etEoS0i5O9Cy4AP0Ngp9gKGaGC0R34TMoO8ih5akwxAZuektF1LryNlBBr38SHq8zSmx3KD+evxJ1DWF4cr9cnsAObV7TWmBqF3xfdWf7m7euwn1asK0XRpa76F8ECJowcx2AnbKRTTNVf45g0jEDLLMu4pFIay3Mvui0tIRF3LSo32VhQ= X-Bogosity: Ham, tests=bogofilter, spamicity=0.036000, 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 Mon, Jan 13, 2025 at 11:30:05AM +0100, Alice Ryhl wrote: > On Tue, Dec 17, 2024 at 12:40 AM 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 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. > > 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 As someone who usually dig a lot of history in git, I would like to see the drop of the temporary introduction ;-) If you're going to rebase, please see whether the drop can be done easily, thanks! Not a block issue though. With or without the change, feel free to add: Reviewed-by: Boqun Feng Regards, Boqun > 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