Message ID | 20231129-alice-file-v1-5-f81afe8c7261@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:a5a7:0:b0:403:3b70:6f57 with SMTP id d7csp325809vqn; Wed, 29 Nov 2023 05:12:37 -0800 (PST) X-Google-Smtp-Source: AGHT+IHFqQjOUi4klG5tcy7n5nR2+C10mi95dOYS3iLiw3qNikugGU7Xk5K2x7Fx/f3dWFYroxix X-Received: by 2002:a05:6a00:21d2:b0:68e:3772:4e40 with SMTP id t18-20020a056a0021d200b0068e37724e40mr18515658pfj.3.1701263557476; Wed, 29 Nov 2023 05:12:37 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701263557; cv=none; d=google.com; s=arc-20160816; b=Gfo39XhNEC5s6vIeAIWgy9bKsHOMlxeYCoIsTnqQJO0aAohqx0A5vi26rqpwPftlyh FEPQjjcxaGn0Zok9u0tyHt45MnAZcotEvWZBo+i90WbfWUulXzZ2XNvOzwAOTyZVjRaw DwHPR7qHhXn7ETXER+5ZjWJ4lSaQbXvbf+PmLHWz3p5K+PCtPQcoFeksK9aM2mW9yQNR k+MJQwCnj6hp+aCSXdII7jWFjpyLOorxljyP7vXG2etC631OMLiGsdQLJBMjRFJY4jV5 FqDWnzFT56MO9MY//gsze1kR8n5hxS9ZwfEX/y0flJAPCkfKyIIOxuyE+dXVXmt5a75c InWA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:dkim-signature; bh=Jdn+NMvalWu2DX8Rm0t7kAAGEBw7/t9Uqy6Z/eCT3dQ=; fh=6EEtf8+s6yyABtSDZ6Fe4igLduHvwiZU5fhW8T0ggOw=; b=J2hM3VMad7mBLqcWmncp7hNqVlheEETsyd8QDO/JeDrq0HlUtEB+Sa68AwV7m7dYex XJIXS3CG3fYvRl6fb/E4gIkBSuqHVE34SZeWX5rBhQFEos2Tj78aszN3TeL7Rsl7d6c4 uibpcjmhmYXbAyBi0Zr4zt1Lv0aDI2oVViH2mfamJhwPLSKmqc7tYYKRcKMMZvXgRuNw RLb87gh0+IAYuQFoAqd2uaphjJ7cvXNeXLUT0BVzNF2f+FkgeaRvs9bGunnXdVTPPYc8 F+8bxj6fz9zJVnTLmp5oxcfsZH9mr9X8Gl+SYl652FHqf7indOCukefPLsGtbWNfSKNv D77A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=seezu6Ah; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from agentk.vger.email (agentk.vger.email. [23.128.96.32]) by mx.google.com with ESMTPS id p4-20020a634204000000b005b34179728esi14007263pga.237.2023.11.29.05.12.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Nov 2023 05:12:37 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=seezu6Ah; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by agentk.vger.email (Postfix) with ESMTP id 11A2E8041EB6; Wed, 29 Nov 2023 05:12:29 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at agentk.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233832AbjK2NMU (ORCPT <rfc822;toshivichauhan@gmail.com> + 99 others); Wed, 29 Nov 2023 08:12:20 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58066 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232775AbjK2NMT (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 29 Nov 2023 08:12:19 -0500 Received: from mail-lf1-x149.google.com (mail-lf1-x149.google.com [IPv6:2a00:1450:4864:20::149]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F192F194 for <linux-kernel@vger.kernel.org>; Wed, 29 Nov 2023 05:12:24 -0800 (PST) Received: by mail-lf1-x149.google.com with SMTP id 2adb3069b0e04-50aa6b26836so7645898e87.3 for <linux-kernel@vger.kernel.org>; Wed, 29 Nov 2023 05:12:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1701263543; x=1701868343; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=Jdn+NMvalWu2DX8Rm0t7kAAGEBw7/t9Uqy6Z/eCT3dQ=; b=seezu6AhKOabbKhc6zu7EmPoVOghl6g8w6Jk2qMxHeSf7Ilowwzr0Iw+HNI2y7RwJD 8jvazds4UAVYd9mx7CTpb8EVlcCVEIBiP1wOPTffO12uhUJ8fTvuFi9X/AYT0i79EdwX 0UM2br3x645sI/mwtACih4p0BlgWtzY78zo2KVrPYgfQTv/nGyTM7cm6EUTxuMiaAF+Y fIAeFIVcaq5dQrixm6pNtMgqxGRDE7kRomc43h23cxf41o9PkdMJq+QmP2sT5L3DmAjA J8FmdZHQPsa3KPkzLrYs1pWTU/BTUVDt+JGtFugRGYqitGYAdhyIVdejxYRoLtCYXsav pGaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701263543; x=1701868343; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Jdn+NMvalWu2DX8Rm0t7kAAGEBw7/t9Uqy6Z/eCT3dQ=; b=fj4JpA4xAtoffvwCbQaVaTXd8JhGY+8VoXG8dy44i4s/3bB3UZMkmtXDe0bfHx68+J 5f/kzCmZyUoMB9aImdQS0qa3wLB6354GFSH2G9PeKwTUz2aYj2PEquXktbT+4snVkKZ/ rMkyy/zH8+4o1Yd/x+U3VCtLJumFhsashq6KxadVNgzLjM4ENfUMvTjCzAaWYWgcZs91 v9uX2Fo2ZszuVBnzEnl8prf+6OPS6otUGQlkKv8ei47n2Zzl9o2+uYxCz2XKxDzAFwhe /bRzeRS3wGi4VAM/uIYpY360DPS2EsO2wnXxWGuI468AT2RUWrpYbN924whW0V3LaLEE Abtw== X-Gm-Message-State: AOJu0YxpMKb2YgAbFLCZ0iZae5yeFAVDzKNgRzRTWBXNBtKkKvwcGHGq 8eAU4AUJNE6Q8oAcotxQFFk1v4fEbVET/ec= X-Received: from aliceryhl2.c.googlers.com ([fda3:e722:ac3:cc00:68:949d:c0a8:572]) (user=aliceryhl job=sendgmr) by 2002:ac2:5224:0:b0:50b:bca7:968c with SMTP id i4-20020ac25224000000b0050bbca7968cmr58963lfl.11.1701263543178; Wed, 29 Nov 2023 05:12:23 -0800 (PST) Date: Wed, 29 Nov 2023 13:12:17 +0000 In-Reply-To: <20231129-alice-file-v1-0-f81afe8c7261@google.com> Mime-Version: 1.0 References: <20231129-alice-file-v1-0-f81afe8c7261@google.com> X-Developer-Key: i=aliceryhl@google.com; a=openpgp; fpr=49F6C1FAA74960F43A5B86A1EE7A392FDE96209F X-Developer-Signature: v=1; a=openpgp-sha256; l=6837; i=aliceryhl@google.com; h=from:subject:message-id; bh=fzhPWG3FyEb6bwd3huD/CCWPlg1T+DI/I6PnRrR6U60=; b=owEBbQKS/ZANAwAKAQRYvu5YxjlGAcsmYgBlZzMyxp/PiE2vc51gW7AeguEkf+zG+hlvclKDg z15KrtnzgeJAjMEAAEKAB0WIQSDkqKUTWQHCvFIvbIEWL7uWMY5RgUCZWczMgAKCRAEWL7uWMY5 Rr0qD/sEfvqOb/IvSsLWcLMiUMGY1ZzjqsXSsbGWHIw5ZhJfuDcIjm4XM1OkrOwdyDRnUixIFHO rHrqNwf6vIUvWLx+d+0IKNUYFPRrx4Jo8T/9ITcfwCkYCj8WV1ouAXWe3EK154x+3SW6BkUj1W/ zsxlPzIy5zVju+h0FNheyCpN2Z8+XrvLR/q5ussUBYroyuT0AjGC0AeR4TA/pgcnQ60ek1YVniA 2Tqq3xzYdX7HlT9ivqOgzUCPWdfSaksZB6Zsj8VYS2QT1qiH19c9fu7razhL87/xeQbHBEwphHx aGghuDnMfE/oU/KlnMB2xHru4JP8E1NteTa4ZLMnLS/36cwT6sEELQq0mS1sJVqCpw0Fmrjx0qk AuKzgL5xhYqsroAD1/NX4GxAgaXrcIkRbdM84FY1ALLWZoWkYUydRGQRBxFybL+G/vr4IFK1bfP QXljESusHDDgQMCj4UPE0c+u9F8V3Wyhzy9iVbc+denXT/xR5nwg8EWMkugWZy7lPdGTyag1zFY MQguoJGaHrdpT9eAAf4fCtnjoTorm6MAOc7NOI2lfrHVIaK/l4aqmP8PX66//ke8l6eGZnuORaz zx5rCKJ5w8KFsKXp9EG9SkYJ+w9O4JDg9sn5ZtcOpKglyp/pR2fu2Iuw54AR4nClN4TAU5sm2D5 ouAI5yUjfk31JMQ== X-Mailer: git-send-email 2.43.0.rc1.413.gea7ed67945-goog Message-ID: <20231129-alice-file-v1-5-f81afe8c7261@google.com> Subject: [PATCH 5/7] rust: file: add `Kuid` wrapper From: Alice Ryhl <aliceryhl@google.com> To: Miguel Ojeda <ojeda@kernel.org>, Alex Gaynor <alex.gaynor@gmail.com>, Wedson Almeida Filho <wedsonaf@gmail.com>, Boqun Feng <boqun.feng@gmail.com>, Gary Guo <gary@garyguo.net>, " =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= " <bjorn3_gh@protonmail.com>, Benno Lossin <benno.lossin@proton.me>, Andreas Hindborg <a.hindborg@samsung.com>, Peter Zijlstra <peterz@infradead.org>, Alexander Viro <viro@zeniv.linux.org.uk>, Christian Brauner <brauner@kernel.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, " =?utf-8?q?Arve_Hj=C3=B8n?= =?utf-8?q?nev=C3=A5g?= " <arve@android.com>, Todd Kjos <tkjos@android.com>, Martijn Coenen <maco@android.com>, Joel Fernandes <joel@joelfernandes.org>, Carlos Llamas <cmllamas@google.com>, Suren Baghdasaryan <surenb@google.com> Cc: Alice Ryhl <aliceryhl@google.com>, Dan Williams <dan.j.williams@intel.com>, Kees Cook <keescook@chromium.org>, Matthew Wilcox <willy@infradead.org>, Thomas Gleixner <tglx@linutronix.de>, Daniel Xu <dxu@dxuuu.xyz>, linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org, linux-fsdevel@vger.kernel.org Content-Type: text/plain; charset="utf-8" X-Spam-Status: No, score=-8.4 required=5.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Wed, 29 Nov 2023 05:12:29 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783904135972008731 X-GMAIL-MSGID: 1783904135972008731 |
Series |
File abstractions needed by Rust Binder
|
|
Commit Message
Alice Ryhl
Nov. 29, 2023, 1:12 p.m. UTC
Adds a wrapper around `kuid_t` called `Kuid`. This allows us to define
various operations on kuids such as equality and current_euid. It also
lets us provide conversions from kuid into userspace values.
Rust Binder needs these operations because it needs to compare kuids for
equality, and it needs to tell userspace about the pid and uid of
incoming transactions.
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
rust/bindings/bindings_helper.h | 1 +
rust/helpers.c | 45 ++++++++++++++++++++++++++
rust/kernel/cred.rs | 5 +--
rust/kernel/task.rs | 71 ++++++++++++++++++++++++++++++++++++++++-
4 files changed, 119 insertions(+), 3 deletions(-)
Comments
On Wed, Nov 29, 2023 at 01:12:17PM +0000, Alice Ryhl wrote: > Adds a wrapper around `kuid_t` called `Kuid`. This allows us to define > various operations on kuids such as equality and current_euid. It also > lets us provide conversions from kuid into userspace values. > > Rust Binder needs these operations because it needs to compare kuids for > equality, and it needs to tell userspace about the pid and uid of > incoming transactions. > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> > --- > rust/bindings/bindings_helper.h | 1 + > rust/helpers.c | 45 ++++++++++++++++++++++++++ > rust/kernel/cred.rs | 5 +-- > rust/kernel/task.rs | 71 ++++++++++++++++++++++++++++++++++++++++- > 4 files changed, 119 insertions(+), 3 deletions(-) > > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h > index 81b13a953eae..700f01840188 100644 > --- a/rust/bindings/bindings_helper.h > +++ b/rust/bindings/bindings_helper.h > @@ -11,6 +11,7 @@ > #include <linux/errname.h> > #include <linux/file.h> > #include <linux/fs.h> > +#include <linux/pid_namespace.h> > #include <linux/security.h> > #include <linux/slab.h> > #include <linux/refcount.h> > diff --git a/rust/helpers.c b/rust/helpers.c > index fd633d9db79a..58e3a9dff349 100644 > --- a/rust/helpers.c > +++ b/rust/helpers.c > @@ -142,6 +142,51 @@ void rust_helper_put_task_struct(struct task_struct *t) > } > EXPORT_SYMBOL_GPL(rust_helper_put_task_struct); > > +kuid_t rust_helper_task_uid(struct task_struct *task) > +{ > + return task_uid(task); > +} > +EXPORT_SYMBOL_GPL(rust_helper_task_uid); > + > +kuid_t rust_helper_task_euid(struct task_struct *task) > +{ > + return task_euid(task); > +} > +EXPORT_SYMBOL_GPL(rust_helper_task_euid); > + > +#ifndef CONFIG_USER_NS > +uid_t rust_helper_from_kuid(struct user_namespace *to, kuid_t uid) > +{ > + return from_kuid(to, uid); > +} > +EXPORT_SYMBOL_GPL(rust_helper_from_kuid); > +#endif /* CONFIG_USER_NS */ > + > +bool rust_helper_uid_eq(kuid_t left, kuid_t right) > +{ > + return uid_eq(left, right); > +} > +EXPORT_SYMBOL_GPL(rust_helper_uid_eq); > + > +kuid_t rust_helper_current_euid(void) > +{ > + return current_euid(); > +} > +EXPORT_SYMBOL_GPL(rust_helper_current_euid); > + > +struct user_namespace *rust_helper_current_user_ns(void) > +{ > + return current_user_ns(); > +} > +EXPORT_SYMBOL_GPL(rust_helper_current_user_ns); > + > +pid_t rust_helper_task_tgid_nr_ns(struct task_struct *tsk, > + struct pid_namespace *ns) > +{ > + return task_tgid_nr_ns(tsk, ns); > +} > +EXPORT_SYMBOL_GPL(rust_helper_task_tgid_nr_ns); I'm a bit puzzled by all these rust_helper_*() calls. Can you explain why they are needed? Because they are/can be static inlines and that somehow doesn't work? > + > struct kunit *rust_helper_kunit_get_current_test(void) > { > return kunit_get_current_test(); > diff --git a/rust/kernel/cred.rs b/rust/kernel/cred.rs > index 3794937b5294..fbc749788bfa 100644 > --- a/rust/kernel/cred.rs > +++ b/rust/kernel/cred.rs > @@ -8,6 +8,7 @@ > > use crate::{ > bindings, > + task::Kuid, > types::{AlwaysRefCounted, Opaque}, > }; > > @@ -52,9 +53,9 @@ pub fn get_secid(&self) -> u32 { > } > > /// Returns the effective UID of the given credential. > - pub fn euid(&self) -> bindings::kuid_t { > + pub fn euid(&self) -> Kuid { > // SAFETY: By the type invariant, we know that `self.0` is valid. > - unsafe { (*self.0.get()).euid } > + Kuid::from_raw(unsafe { (*self.0.get()).euid }) > } > } > > diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs > index b2299bc7ac1f..1a27b968a907 100644 > --- a/rust/kernel/task.rs > +++ b/rust/kernel/task.rs > @@ -5,7 +5,12 @@ > //! C header: [`include/linux/sched.h`](../../../../include/linux/sched.h). > > use crate::{bindings, types::Opaque}; > -use core::{marker::PhantomData, ops::Deref, ptr}; > +use core::{ > + cmp::{Eq, PartialEq}, > + marker::PhantomData, > + ops::Deref, > + ptr, > +}; > > /// Returns the currently running task. > #[macro_export] > @@ -78,6 +83,12 @@ unsafe impl Sync for Task {} > /// The type of process identifiers (PIDs). > type Pid = bindings::pid_t; > > +/// The type of user identifiers (UIDs). > +#[derive(Copy, Clone)] > +pub struct Kuid { > + kuid: bindings::kuid_t, > +} > + > impl Task { > /// Returns a task reference for the currently executing task/thread. > /// > @@ -132,12 +143,34 @@ pub fn pid(&self) -> Pid { > unsafe { *ptr::addr_of!((*self.0.get()).pid) } > } > > + /// Returns the UID of the given task. > + pub fn uid(&self) -> Kuid { > + // SAFETY: By the type invariant, we know that `self.0` is valid. > + Kuid::from_raw(unsafe { bindings::task_uid(self.0.get()) }) > + } > + > + /// Returns the effective UID of the given task. > + pub fn euid(&self) -> Kuid { > + // SAFETY: By the type invariant, we know that `self.0` is valid. > + Kuid::from_raw(unsafe { bindings::task_euid(self.0.get()) }) > + } > + > /// Determines whether the given task has pending signals. > pub fn signal_pending(&self) -> bool { > // SAFETY: By the type invariant, we know that `self.0` is valid. > unsafe { bindings::signal_pending(self.0.get()) != 0 } > } > > + /// Returns the given task's pid in the current pid namespace. > + pub fn pid_in_current_ns(&self) -> Pid { > + // SAFETY: We know that `self.0.get()` is valid by the type invariant. The rest is just FFI > + // calls. > + unsafe { > + let namespace = bindings::task_active_pid_ns(bindings::get_current()); > + bindings::task_tgid_nr_ns(self.0.get(), namespace) > + } > + } > + > /// Wakes up the task. > pub fn wake_up(&self) { > // SAFETY: By the type invariant, we know that `self.0.get()` is non-null and valid. > @@ -147,6 +180,42 @@ pub fn wake_up(&self) { > } > } > > +impl Kuid { > + /// Get the current euid. > + pub fn current_euid() -> Kuid { > + // SAFETY: Just an FFI call. > + Self { > + kuid: unsafe { bindings::current_euid() }, > + } > + } > + > + /// Create a `Kuid` given the raw C type. > + pub fn from_raw(kuid: bindings::kuid_t) -> Self { > + Self { kuid } > + } > + > + /// Turn this kuid into the raw C type. > + pub fn into_raw(self) -> bindings::kuid_t { > + self.kuid > + } > + > + /// Converts this kernel UID into a UID that userspace understands. Uses the namespace of the > + /// current task. > + pub fn into_uid_in_current_ns(self) -> bindings::uid_t { Hm, I wouldn't special-case this. Just expose from_kuid() and let it take a namespace argument, no? You don't need to provide bindings for namespaces ofc. > + // SAFETY: Just an FFI call. > + unsafe { bindings::from_kuid(bindings::current_user_ns(), self.kuid) } > + } > +} > + > +impl PartialEq for Kuid { > + fn eq(&self, other: &Kuid) -> bool { > + // SAFETY: Just an FFI call. > + unsafe { bindings::uid_eq(self.kuid, other.kuid) } > + } > +} > + > +impl Eq for Kuid {} Do you need that? > + > // SAFETY: The type invariants guarantee that `Task` is always ref-counted. > unsafe impl crate::types::AlwaysRefCounted for Task { > fn inc_ref(&self) { > > -- > 2.43.0.rc1.413.gea7ed67945-goog >
On Wed, Nov 29, 2023 at 05:28:27PM +0100, Christian Brauner wrote: > > +pid_t rust_helper_task_tgid_nr_ns(struct task_struct *tsk, > > + struct pid_namespace *ns) > > +{ > > + return task_tgid_nr_ns(tsk, ns); > > +} > > +EXPORT_SYMBOL_GPL(rust_helper_task_tgid_nr_ns); > > I'm a bit puzzled by all these rust_helper_*() calls. Can you explain > why they are needed? Because they are/can be static inlines and that > somehow doesn't work? Correct, because Rust can only talk to C ABI, it cannot use C headers. Bindgen would need to translate the full C headers into valid Rust for that to work. I really think the Rust peoples should spend more effort on that, because you are quite right, all this wrappery is tedious at best.
Christian Brauner <brauner@kernel.org> writes: > I'm a bit puzzled by all these rust_helper_*() calls. Can you explain > why they are needed? Because they are/can be static inlines and that > somehow doesn't work? Yes, it's because the methods are inline. Rust can only call C methods that are actually exported by the C code. >> + /// Converts this kernel UID into a UID that userspace understands. Uses the namespace of the >> + /// current task. >> + pub fn into_uid_in_current_ns(self) -> bindings::uid_t { > > Hm, I wouldn't special-case this. Just expose from_kuid() and let it > take a namespace argument, no? You don't need to provide bindings for > namespaces ofc. To make `from_kuid` safe, I would need to wrap the namespace type too. I could do that, but it would be more code than this method because I need another wrapper struct and so on. Personally I would prefer to special-case it until someone needs the non-special-case. Then, they can delete this method when they introduce the non-special-case. But I'll do it if you think I should. >> +impl PartialEq for Kuid { >> + fn eq(&self, other: &Kuid) -> bool { >> + // SAFETY: Just an FFI call. >> + unsafe { bindings::uid_eq(self.kuid, other.kuid) } >> + } >> +} >> + >> +impl Eq for Kuid {} > > Do you need that? Yes. This is the code that tells the compiler what `==` means for the `Kuid` type. Binder uses it here: https://github.com/Darksonn/linux/blob/dca45e6c7848e024709b165a306cdbe88e5b086a/drivers/android/context.rs#L174 Alice
On Wed, Nov 29, 2023 at 01:12:17PM +0000, Alice Ryhl wrote: > diff --git a/rust/helpers.c b/rust/helpers.c > index fd633d9db79a..58e3a9dff349 100644 > --- a/rust/helpers.c > +++ b/rust/helpers.c > @@ -142,6 +142,51 @@ void rust_helper_put_task_struct(struct task_struct *t) > } > EXPORT_SYMBOL_GPL(rust_helper_put_task_struct); > > +kuid_t rust_helper_task_uid(struct task_struct *task) > +{ > + return task_uid(task); > +} > +EXPORT_SYMBOL_GPL(rust_helper_task_uid); > + > +kuid_t rust_helper_task_euid(struct task_struct *task) > +{ > + return task_euid(task); > +} > +EXPORT_SYMBOL_GPL(rust_helper_task_euid); Aren't these like ideal speculation gadgets? And shouldn't we avoid functions like this for exactly that reason?
On Thu, Nov 30, 2023 at 09:36:03AM +0000, Alice Ryhl wrote: > Christian Brauner <brauner@kernel.org> writes: > > I'm a bit puzzled by all these rust_helper_*() calls. Can you explain > > why they are needed? Because they are/can be static inlines and that > > somehow doesn't work? > > Yes, it's because the methods are inline. Rust can only call C methods > that are actually exported by the C code. > > >> + /// Converts this kernel UID into a UID that userspace understands. Uses the namespace of the > >> + /// current task. > >> + pub fn into_uid_in_current_ns(self) -> bindings::uid_t { > > > > Hm, I wouldn't special-case this. Just expose from_kuid() and let it > > take a namespace argument, no? You don't need to provide bindings for > > namespaces ofc. > > To make `from_kuid` safe, I would need to wrap the namespace type too. I > could do that, but it would be more code than this method because I need > another wrapper struct and so on. > > Personally I would prefer to special-case it until someone needs the > non-special-case. Then, they can delete this method when they introduce > the non-special-case. > > But I'll do it if you think I should. No, don't start wrapping namespaces as well. You already do parts of LSM as well. > > >> +impl PartialEq for Kuid { > >> + fn eq(&self, other: &Kuid) -> bool { > >> + // SAFETY: Just an FFI call. > >> + unsafe { bindings::uid_eq(self.kuid, other.kuid) } > >> + } > >> +} > >> + > >> +impl Eq for Kuid {} > > > > Do you need that? > > Yes. This is the code that tells the compiler what `==` means for the > `Kuid` type. Binder uses it here: Ok, thanks.
On 11/29/23 14:12, Alice Ryhl wrote: > + /// Returns the given task's pid in the current pid namespace. > + pub fn pid_in_current_ns(&self) -> Pid { > + // SAFETY: We know that `self.0.get()` is valid by the type invariant. The rest is just FFI > + // calls. > + unsafe { > + let namespace = bindings::task_active_pid_ns(bindings::get_current()); > + bindings::task_tgid_nr_ns(self.0.get(), namespace) > + } I would split this into two `unsafe` blocks. > + } > + > /// Wakes up the task. > pub fn wake_up(&self) { > // SAFETY: By the type invariant, we know that `self.0.get()` is non-null and valid. > @@ -147,6 +180,42 @@ pub fn wake_up(&self) { > } > } > > +impl Kuid { > + /// Get the current euid. > + pub fn current_euid() -> Kuid { > + // SAFETY: Just an FFI call. > + Self { > + kuid: unsafe { bindings::current_euid() }, > + } Would expect a call to `from_raw` here instead of `Self {}`. > + } > + > + /// Create a `Kuid` given the raw C type. > + pub fn from_raw(kuid: bindings::kuid_t) -> Self { > + Self { kuid } > + } Is there a reason that this is named `from_raw` and not just a normal `From` impl? AFAICT any `bindings::kuid_t` is a valid `Kuid`. > + > + /// Turn this kuid into the raw C type. > + pub fn into_raw(self) -> bindings::kuid_t { > + self.kuid > + } > + > + /// Converts this kernel UID into a UID that userspace understands. Uses the namespace of the > + /// current task. Why not: /// Converts this kernel UID into a userspace UID. /// /// Uses the namespace of the current task.
On Thu, Nov 30, 2023 at 01:46:36PM +0100, Christian Brauner wrote: > On Wed, Nov 29, 2023 at 05:48:15PM +0100, Peter Zijlstra wrote: > > On Wed, Nov 29, 2023 at 05:28:27PM +0100, Christian Brauner wrote: > > > > > > +pid_t rust_helper_task_tgid_nr_ns(struct task_struct *tsk, > > > > + struct pid_namespace *ns) > > > > +{ > > > > + return task_tgid_nr_ns(tsk, ns); > > > > +} > > > > +EXPORT_SYMBOL_GPL(rust_helper_task_tgid_nr_ns); > > > > > > I'm a bit puzzled by all these rust_helper_*() calls. Can you explain > > > why they are needed? Because they are/can be static inlines and that > > > somehow doesn't work? > > > > Correct, because Rust can only talk to C ABI, it cannot use C headers. > > Bindgen would need to translate the full C headers into valid Rust for > > that to work. > > > > I really think the Rust peoples should spend more effort on that, > > because you are quite right, all this wrappery is tedious at best. I suspect even if the manpower existed to go that route we'd end up regretting it, because then the Rust compiler would need to be able to handle _all_ the craziness a modern C compiler knows how to do - preprocessor magic/devilry isn't even the worst of it, it gets even worse when you start to consider things like bitfields and all the crazy __attributes__(()) people have invented. Swift went that route, but they have Apple funding them, and I doubt even they would want anything to do with Linux kernel C. IOW: yes, the extra friction from not being able to do full C -> Rust translation is annoying now, but probably a good thing in the long run. > The problem is that we end up with a long list of explicit exports that > also are all really weirdly named like rust_helper_*(). I wouldn't even > complain if it they were somehow auto-generated but as you say that > might be out of scope. I think we'd need help from the C side to auto generate them - what we really want is for them to be inline, not static inline, but of course that has never really worked for functions used across a single C file. But maybe C compiler people are smarter these days? Just a keyword to to tell the C compiler "take this static inline and generate a compiled version in this .c file" would be all we need. I could see it being handy for other things, too: as Linus has been saying, we tend to inline too much code these days, and part of the reason for that is we make a function inline because of the _one_ fastpath that needs it, but there's 3 more slowpaths that don't. And right now we don't have any sane way of having a function be available with both inlined and outlined versions, besides the same kind of manual wrappers the Rust people are doing here... so we should probably just fix that.
On Thu, Nov 30, 2023 at 11:36:35AM +0100, Peter Zijlstra wrote: > On Wed, Nov 29, 2023 at 01:12:17PM +0000, Alice Ryhl wrote: > > > diff --git a/rust/helpers.c b/rust/helpers.c > > index fd633d9db79a..58e3a9dff349 100644 > > --- a/rust/helpers.c > > +++ b/rust/helpers.c > > @@ -142,6 +142,51 @@ void rust_helper_put_task_struct(struct task_struct *t) > > } > > EXPORT_SYMBOL_GPL(rust_helper_put_task_struct); > > > > +kuid_t rust_helper_task_uid(struct task_struct *task) > > +{ > > + return task_uid(task); > > +} > > +EXPORT_SYMBOL_GPL(rust_helper_task_uid); > > + > > +kuid_t rust_helper_task_euid(struct task_struct *task) > > +{ > > + return task_euid(task); > > +} > > +EXPORT_SYMBOL_GPL(rust_helper_task_euid); > > Aren't these like ideal speculation gadgets? And shouldn't we avoid > functions like this for exactly that reason? I think asking the Rust people to care about that is probably putting too many constraints on them, unless you actually have an idea for something better to do... (loudly giving the CPU manufacturers the middle finger for making _all_ of us deal with this bullshit)
On Wed, Dec 06, 2023 at 03:02:24PM -0500, Kent Overstreet wrote: > On Thu, Nov 30, 2023 at 11:36:35AM +0100, Peter Zijlstra wrote: > > On Wed, Nov 29, 2023 at 01:12:17PM +0000, Alice Ryhl wrote: > > > > > diff --git a/rust/helpers.c b/rust/helpers.c > > > index fd633d9db79a..58e3a9dff349 100644 > > > --- a/rust/helpers.c > > > +++ b/rust/helpers.c > > > @@ -142,6 +142,51 @@ void rust_helper_put_task_struct(struct task_struct *t) > > > } > > > EXPORT_SYMBOL_GPL(rust_helper_put_task_struct); > > > > > > +kuid_t rust_helper_task_uid(struct task_struct *task) > > > +{ > > > + return task_uid(task); > > > +} > > > +EXPORT_SYMBOL_GPL(rust_helper_task_uid); > > > + > > > +kuid_t rust_helper_task_euid(struct task_struct *task) > > > +{ > > > + return task_euid(task); > > > +} > > > +EXPORT_SYMBOL_GPL(rust_helper_task_euid); > > > > Aren't these like ideal speculation gadgets? And shouldn't we avoid > > functions like this for exactly that reason? > > I think asking the Rust people to care about that is probably putting > too many constraints on them, unless you actually have an idea for > something better to do... It's not a constraint, it is a "we can not do this as it is buggy because cpus are broken and we need to protect users from those bugs." If we were to accept this type of code, then the people who are going "it's safer to write kernel code in Rust" would be "pleasantly surprised" when it turns out that their systems are actually more insecure. Hint, when "known broken" code is found in code review, it can not just be ignored. thanks, greg k-h
On Thu, Dec 07, 2023 at 08:18:37AM +0100, Greg Kroah-Hartman wrote: > On Wed, Dec 06, 2023 at 03:02:24PM -0500, Kent Overstreet wrote: > > On Thu, Nov 30, 2023 at 11:36:35AM +0100, Peter Zijlstra wrote: > > > On Wed, Nov 29, 2023 at 01:12:17PM +0000, Alice Ryhl wrote: > > > > > > > diff --git a/rust/helpers.c b/rust/helpers.c > > > > index fd633d9db79a..58e3a9dff349 100644 > > > > --- a/rust/helpers.c > > > > +++ b/rust/helpers.c > > > > @@ -142,6 +142,51 @@ void rust_helper_put_task_struct(struct task_struct *t) > > > > } > > > > EXPORT_SYMBOL_GPL(rust_helper_put_task_struct); > > > > > > > > +kuid_t rust_helper_task_uid(struct task_struct *task) > > > > +{ > > > > + return task_uid(task); > > > > +} > > > > +EXPORT_SYMBOL_GPL(rust_helper_task_uid); > > > > + > > > > +kuid_t rust_helper_task_euid(struct task_struct *task) > > > > +{ > > > > + return task_euid(task); > > > > +} > > > > +EXPORT_SYMBOL_GPL(rust_helper_task_euid); > > > > > > Aren't these like ideal speculation gadgets? And shouldn't we avoid > > > functions like this for exactly that reason? > > > > I think asking the Rust people to care about that is probably putting > > too many constraints on them, unless you actually have an idea for > > something better to do... > > It's not a constraint, it is a "we can not do this as it is buggy > because cpus are broken and we need to protect users from those bugs." > > If we were to accept this type of code, then the people who are going > "it's safer to write kernel code in Rust" would be "pleasantly > surprised" when it turns out that their systems are actually more > insecure. > > Hint, when "known broken" code is found in code review, it can not just > be ignored. We're talking about a CPU bug, not a Rust bug, and maybe try a nm --size-sort and see what you find before throwing stones at them...
> On Nov 30, 2023, at 4:46 AM, Christian Brauner <brauner@kernel.org> wrote: > > I wouldn't even > complain if it they were somehow auto-generated but as you say that > might be out of scope. FYI, rust-bindgen got an experimental feature of this nature earlier this year: https://github.com/rust-lang/rust-bindgen/pull/2335 Though apparently it has significant limitations meriting it the “experimental” title. Regarding the issue of wrappers not being inlined, it's possible to get LLVM to optimize C and Rust code together into an object file, with the help of a compatible Clang and LLD: @ rustc -O --emit llvm-bc a.rs @ clang --target=x86_64-unknown-linux-gnu -O2 -c -emit-llvm -o b.bc b.c @ ld.lld -r -o c.o a.bc b.bc Basically LTO but within the scope of a single object file. This would be redundant in cases where kernel-wide LTO is enabled. Using this approach might slow down compilation a bit due to needing to pass the LLVM bitcode between multiple commands, but probably not very much. Just chiming in as someone not involved in Rust for Linux but familiar with these tools. Perhaps this has been considered before and rejected for some reason; I wouldn’t know.
On Fri, Dec 8, 2023 at 6:28 AM comex <comexk@gmail.com> wrote: > > Regarding the issue of wrappers not being inlined, it's possible to get LLVM to optimize C and Rust code together into an object file, with the help of a compatible Clang and LLD: > > @ rustc -O --emit llvm-bc a.rs > @ clang --target=x86_64-unknown-linux-gnu -O2 -c -emit-llvm -o b.bc b.c > @ ld.lld -r -o c.o a.bc b.bc > > Basically LTO but within the scope of a single object file. This would be redundant in cases where kernel-wide LTO is enabled. > > Using this approach might slow down compilation a bit due to needing to pass the LLVM bitcode between multiple commands, but probably not very much. > > Just chiming in as someone not involved in Rust for Linux but familiar with these tools. Perhaps this has been considered before and rejected for some reason; I wouldn’t know. Thanks comex for chiming in, much appreciated. Yeah, this is what we have been calling the "local-LTO hack" and it was one of the possibilities we were considering for non-LTO kernel builds for performance reasons originally. I don't recall who originally suggested it in one of our meetings (Gary or Björn perhaps). If LLVM folks think LLVM-wise nothing will break, then we are happy to go ahead with that (since it also solves the performance side), but it would be nice to know if it will always be OK to build like that, i.e. I think Andreas actually tried it and it seemed to work and boot, but the worry is whether there is something subtle that could have bad codegen in the future. (We will also need to worry about GCC.) Cheers, Miguel
On Wed, Dec 06, 2023 at 02:59:11PM -0500, Kent Overstreet wrote: > I suspect even if the manpower existed to go that route we'd end up > regretting it, because then the Rust compiler would need to be able to > handle _all_ the craziness a modern C compiler knows how to do - > preprocessor magic/devilry isn't even the worst of it, it gets even > worse when you start to consider things like bitfields and all the crazy > __attributes__(()) people have invented. Dude, clang can already handle all of that. Both rust and clang are build on top of llvm, they generate the same IR, you can simply feed a string into libclang and get IR out of it, which you can splice into your rust generated IR.
On Fri, Dec 8, 2023 at 8:19 AM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Fri, Dec 8, 2023 at 6:28 AM comex <comexk@gmail.com> wrote: > > > > Regarding the issue of wrappers not being inlined, it's possible to get LLVM to optimize C and Rust code together into an object file, with the help of a compatible Clang and LLD: > > > > @ rustc -O --emit llvm-bc a.rs > > @ clang --target=x86_64-unknown-linux-gnu -O2 -c -emit-llvm -o b.bc b.c > > @ ld.lld -r -o c.o a.bc b.bc > > > > Basically LTO but within the scope of a single object file. This would be redundant in cases where kernel-wide LTO is enabled. > > > > Using this approach might slow down compilation a bit due to needing to pass the LLVM bitcode between multiple commands, but probably not very much. > > > > Just chiming in as someone not involved in Rust for Linux but familiar with these tools. Perhaps this has been considered before and rejected for some reason; I wouldn’t know. > > Thanks comex for chiming in, much appreciated. > > Yeah, this is what we have been calling the "local-LTO hack" and it > was one of the possibilities we were considering for non-LTO kernel > builds for performance reasons originally. I don't recall who > originally suggested it in one of our meetings (Gary or Björn > perhaps). > > If LLVM folks think LLVM-wise nothing will break, then we are happy to On paper, nothing comes to mind. No promises though. From a build system perspective, I'd rather just point users towards LTO if they have this concern. We support full and thin lto. This proposal would add a third variant for just rust drivers. Each variation on LTO has a maintenance cost and each have had their own distinct fun bugs in the past. Not sure an additional variant is worth the maintenance cost, even if it's technically feasible. > go ahead with that (since it also solves the performance side), but it > would be nice to know if it will always be OK to build like that, i.e. > I think Andreas actually tried it and it seemed to work and boot, but > the worry is whether there is something subtle that could have bad > codegen in the future. > > (We will also need to worry about GCC.) > > Cheers, > Miguel
On Fri, Dec 8, 2023 at 6:09 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On paper, nothing comes to mind. No promises though. Thanks Nick -- that is useful nevertheless. > From a build system perspective, I'd rather just point users towards > LTO if they have this concern. We support full and thin lto. This > proposal would add a third variant for just rust drivers. Each > variation on LTO has a maintenance cost and each have had their own > distinct fun bugs in the past. Not sure an additional variant is > worth the maintenance cost, even if it's technically feasible. I was thinking it would be something always done for Rust object files: under a normal "no LTO" build, the Rust object files would always get the cross-language inlining done and therefore no extra dimension in the matrix. Would that help? I think it is worth at least considering, given there is also a non-trivial amount of performance to gain if we always do it, e.g. Andreas wanted it for non-LTO kernel for this reason. Cheers, Miguel
On Fri, Dec 08, 2023 at 09:08:47AM -0800, Nick Desaulniers wrote: > On Fri, Dec 8, 2023 at 8:19 AM Miguel Ojeda > <miguel.ojeda.sandonis@gmail.com> wrote: > > > > On Fri, Dec 8, 2023 at 6:28 AM comex <comexk@gmail.com> wrote: > > > > > > Regarding the issue of wrappers not being inlined, it's possible to get LLVM to optimize C and Rust code together into an object file, with the help of a compatible Clang and LLD: > > > > > > @ rustc -O --emit llvm-bc a.rs > > > @ clang --target=x86_64-unknown-linux-gnu -O2 -c -emit-llvm -o b.bc b.c > > > @ ld.lld -r -o c.o a.bc b.bc > > > > > > Basically LTO but within the scope of a single object file. This would be redundant in cases where kernel-wide LTO is enabled. > > > > > > Using this approach might slow down compilation a bit due to needing to pass the LLVM bitcode between multiple commands, but probably not very much. > > > > > > Just chiming in as someone not involved in Rust for Linux but familiar with these tools. Perhaps this has been considered before and rejected for some reason; I wouldn’t know. > > > > Thanks comex for chiming in, much appreciated. > > > > Yeah, this is what we have been calling the "local-LTO hack" and it > > was one of the possibilities we were considering for non-LTO kernel > > builds for performance reasons originally. I don't recall who > > originally suggested it in one of our meetings (Gary or Björn > > perhaps). > > > > If LLVM folks think LLVM-wise nothing will break, then we are happy to > > On paper, nothing comes to mind. No promises though. > > From a build system perspective, I'd rather just point users towards > LTO if they have this concern. We support full and thin lto. This > proposal would add a third variant for just rust drivers. Each > variation on LTO has a maintenance cost and each have had their own > distinct fun bugs in the past. Not sure an additional variant is > worth the maintenance cost, even if it's technically feasible. > Actually, the "LTO" in "local-LTO" may be misleading ;-) The problem we want to resolve here is letting Rust code call small C functions (or macros) without exporting the symbols. To me, it's really just "static linking" a library (right now it's rust/helpers.o) contains small C functions and macros used by Rust into a Rust driver kmodule, the "LTO" part can be optional: let the linker make the call. Regards, Boqun > > go ahead with that (since it also solves the performance side), but it > > would be nice to know if it will always be OK to build like that, i.e. > > I think Andreas actually tried it and it seemed to work and boot, but > > the worry is whether there is something subtle that could have bad > > codegen in the future. > > > > (We will also need to worry about GCC.) > > > > Cheers, > > Miguel > > > > -- > Thanks, > ~Nick Desaulniers
On Fri, Dec 08, 2023 at 05:26:16PM +0100, Peter Zijlstra wrote: > On Wed, Dec 06, 2023 at 02:59:11PM -0500, Kent Overstreet wrote: > > > I suspect even if the manpower existed to go that route we'd end up > > regretting it, because then the Rust compiler would need to be able to > > handle _all_ the craziness a modern C compiler knows how to do - > > preprocessor magic/devilry isn't even the worst of it, it gets even > > worse when you start to consider things like bitfields and all the crazy > > __attributes__(()) people have invented. > > Dude, clang can already handle all of that. Both rust and clang are > build on top of llvm, they generate the same IR, you can simply feed a > string into libclang and get IR out of it, which you can splice into > your rust generated IR. If only it were that simple :) This is struct definitions we're talking about, not code, so what you want isn't even IR, what you're generating is a memory layout for a type, linked in with all your other type information. And people critize Linux for being a giant monorepo that makes no considerations for making our code reusable in other contexts; clang and LLVM are no different. But that's not really the issue because you're going to need a huge chunk of clang to even parse this stuff, what you really want is a way to invoke clang and dump _type information_ in a standardized, easy to consume way. What you want is actually more akin to the debug info that's generated today. So... yeah, sure, lovely if it existed, but not the world we live in :) (As an aside, I've actually got an outstanding bug filed with rustc because it needs to be able to handle types that are marked both packed and aligned... if anyone in this thread _does_ know some rust compiler folks, we need that for bcachefs on disk format types).
On Fri, Dec 08, 2023 at 09:08:47AM -0800, Nick Desaulniers wrote: > From a build system perspective, I'd rather just point users towards > LTO if they have this concern. We support full and thin lto. This > proposal would add a third variant for just rust drivers. Each > variation on LTO has a maintenance cost and each have had their own > distinct fun bugs in the past. Not sure an additional variant is > worth the maintenance cost, even if it's technically feasible. If we're allowed to talk about ideal solutions ... I hate putting code in header files. I'd rather be able to put, eg: __force_inline int put_page_testzero(struct page *page) { VM_BUG_ON_PAGE(page_ref_count(page) == 0, page); return page_ref_dec_and_test(page); } __force_inline int folio_put_testzero(struct folio *folio) { return put_page_testzero(&folio->page); } __force_inline void folio_put(struct folio *folio) { if (folio_put_testzero(folio)) __folio_put(folio); } into a .c file and have both C and Rust inline folio_put(), folio_put_testzero(), put_page_testzero(), VM_BUG_ON_PAGE() and page_ref_dec_and_test(), but not even attempt to inline __folio_put() (because We Know Better, and have determined that is the point at which to stop).
On Dec 8, 2023, at 8:19 AM, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > If LLVM folks think LLVM-wise nothing will break, then we are happy to > go ahead with that (since it also solves the performance side), but it > would be nice to know if it will always be OK to build like that, i.e. > I think Andreas actually tried it and it seemed to work and boot, but > the worry is whether there is something subtle that could have bad > codegen in the future. One potential issue is incompatibility between the LLVM versions used by rustc, Clang, and LLD. At minimum, whichever tool is reading bitcode (LLD in my example) should have an LLVM version >= that of the tools producing bitcode, since newer LLVM versions can read older bitcode but not vice versa. But ideally the tools would all just be linked against the same copy of LLVM. If you’re getting your tools from a distro, then that may already be true for you. But if you’re using upstream rustc binaries, those are built against a custom branch of LLVM, which is based on upstream release versions but adds a handful of patches [1]; by policy, those patches can include cherry-picks of miscompilation fixes that are upstream but haven’t made it into a release yet [2]. Upstream rustc binaries are accompanied by a copy of LLD linked against the same LLVM library, named rust-lld, but there’s no corresponding copy of Clang [3]. I’d say that agreement between rustc and LLD is the most important thing, but it would be nice if they'd make a matching Clang available through rustup. [1] https://github.com/llvm/llvm-project/compare/release/17.x...rust-lang:llvm-project:rustc/17.0-2023-09-19 [2] https://rustc-dev-guide.rust-lang.org/backend/updating-llvm.html#bugfix-updates [3] https://github.com/rust-lang/rust/issues/56371
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index 81b13a953eae..700f01840188 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -11,6 +11,7 @@ #include <linux/errname.h> #include <linux/file.h> #include <linux/fs.h> +#include <linux/pid_namespace.h> #include <linux/security.h> #include <linux/slab.h> #include <linux/refcount.h> diff --git a/rust/helpers.c b/rust/helpers.c index fd633d9db79a..58e3a9dff349 100644 --- a/rust/helpers.c +++ b/rust/helpers.c @@ -142,6 +142,51 @@ void rust_helper_put_task_struct(struct task_struct *t) } EXPORT_SYMBOL_GPL(rust_helper_put_task_struct); +kuid_t rust_helper_task_uid(struct task_struct *task) +{ + return task_uid(task); +} +EXPORT_SYMBOL_GPL(rust_helper_task_uid); + +kuid_t rust_helper_task_euid(struct task_struct *task) +{ + return task_euid(task); +} +EXPORT_SYMBOL_GPL(rust_helper_task_euid); + +#ifndef CONFIG_USER_NS +uid_t rust_helper_from_kuid(struct user_namespace *to, kuid_t uid) +{ + return from_kuid(to, uid); +} +EXPORT_SYMBOL_GPL(rust_helper_from_kuid); +#endif /* CONFIG_USER_NS */ + +bool rust_helper_uid_eq(kuid_t left, kuid_t right) +{ + return uid_eq(left, right); +} +EXPORT_SYMBOL_GPL(rust_helper_uid_eq); + +kuid_t rust_helper_current_euid(void) +{ + return current_euid(); +} +EXPORT_SYMBOL_GPL(rust_helper_current_euid); + +struct user_namespace *rust_helper_current_user_ns(void) +{ + return current_user_ns(); +} +EXPORT_SYMBOL_GPL(rust_helper_current_user_ns); + +pid_t rust_helper_task_tgid_nr_ns(struct task_struct *tsk, + struct pid_namespace *ns) +{ + return task_tgid_nr_ns(tsk, ns); +} +EXPORT_SYMBOL_GPL(rust_helper_task_tgid_nr_ns); + struct kunit *rust_helper_kunit_get_current_test(void) { return kunit_get_current_test(); diff --git a/rust/kernel/cred.rs b/rust/kernel/cred.rs index 3794937b5294..fbc749788bfa 100644 --- a/rust/kernel/cred.rs +++ b/rust/kernel/cred.rs @@ -8,6 +8,7 @@ use crate::{ bindings, + task::Kuid, types::{AlwaysRefCounted, Opaque}, }; @@ -52,9 +53,9 @@ pub fn get_secid(&self) -> u32 { } /// Returns the effective UID of the given credential. - pub fn euid(&self) -> bindings::kuid_t { + pub fn euid(&self) -> Kuid { // SAFETY: By the type invariant, we know that `self.0` is valid. - unsafe { (*self.0.get()).euid } + Kuid::from_raw(unsafe { (*self.0.get()).euid }) } } diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs index b2299bc7ac1f..1a27b968a907 100644 --- a/rust/kernel/task.rs +++ b/rust/kernel/task.rs @@ -5,7 +5,12 @@ //! C header: [`include/linux/sched.h`](../../../../include/linux/sched.h). use crate::{bindings, types::Opaque}; -use core::{marker::PhantomData, ops::Deref, ptr}; +use core::{ + cmp::{Eq, PartialEq}, + marker::PhantomData, + ops::Deref, + ptr, +}; /// Returns the currently running task. #[macro_export] @@ -78,6 +83,12 @@ unsafe impl Sync for Task {} /// The type of process identifiers (PIDs). type Pid = bindings::pid_t; +/// The type of user identifiers (UIDs). +#[derive(Copy, Clone)] +pub struct Kuid { + kuid: bindings::kuid_t, +} + impl Task { /// Returns a task reference for the currently executing task/thread. /// @@ -132,12 +143,34 @@ pub fn pid(&self) -> Pid { unsafe { *ptr::addr_of!((*self.0.get()).pid) } } + /// Returns the UID of the given task. + pub fn uid(&self) -> Kuid { + // SAFETY: By the type invariant, we know that `self.0` is valid. + Kuid::from_raw(unsafe { bindings::task_uid(self.0.get()) }) + } + + /// Returns the effective UID of the given task. + pub fn euid(&self) -> Kuid { + // SAFETY: By the type invariant, we know that `self.0` is valid. + Kuid::from_raw(unsafe { bindings::task_euid(self.0.get()) }) + } + /// Determines whether the given task has pending signals. pub fn signal_pending(&self) -> bool { // SAFETY: By the type invariant, we know that `self.0` is valid. unsafe { bindings::signal_pending(self.0.get()) != 0 } } + /// Returns the given task's pid in the current pid namespace. + pub fn pid_in_current_ns(&self) -> Pid { + // SAFETY: We know that `self.0.get()` is valid by the type invariant. The rest is just FFI + // calls. + unsafe { + let namespace = bindings::task_active_pid_ns(bindings::get_current()); + bindings::task_tgid_nr_ns(self.0.get(), namespace) + } + } + /// Wakes up the task. pub fn wake_up(&self) { // SAFETY: By the type invariant, we know that `self.0.get()` is non-null and valid. @@ -147,6 +180,42 @@ pub fn wake_up(&self) { } } +impl Kuid { + /// Get the current euid. + pub fn current_euid() -> Kuid { + // SAFETY: Just an FFI call. + Self { + kuid: unsafe { bindings::current_euid() }, + } + } + + /// Create a `Kuid` given the raw C type. + pub fn from_raw(kuid: bindings::kuid_t) -> Self { + Self { kuid } + } + + /// Turn this kuid into the raw C type. + pub fn into_raw(self) -> bindings::kuid_t { + self.kuid + } + + /// Converts this kernel UID into a UID that userspace understands. Uses the namespace of the + /// current task. + pub fn into_uid_in_current_ns(self) -> bindings::uid_t { + // SAFETY: Just an FFI call. + unsafe { bindings::from_kuid(bindings::current_user_ns(), self.kuid) } + } +} + +impl PartialEq for Kuid { + fn eq(&self, other: &Kuid) -> bool { + // SAFETY: Just an FFI call. + unsafe { bindings::uid_eq(self.kuid, other.kuid) } + } +} + +impl Eq for Kuid {} + // SAFETY: The type invariants guarantee that `Task` is always ref-counted. unsafe impl crate::types::AlwaysRefCounted for Task { fn inc_ref(&self) {