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 7C3C8C4345F for ; Tue, 16 Apr 2024 05:53:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F25B06B0085; Tue, 16 Apr 2024 01:53:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id ED7146B0088; Tue, 16 Apr 2024 01:53:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D9DED6B0089; Tue, 16 Apr 2024 01:53:35 -0400 (EDT) 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 B902A6B0085 for ; Tue, 16 Apr 2024 01:53:35 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 652BD40906 for ; Tue, 16 Apr 2024 05:53:35 +0000 (UTC) X-FDA: 82014327990.19.3E409CE Received: from mail-yb1-f169.google.com (mail-yb1-f169.google.com [209.85.219.169]) by imf02.hostedemail.com (Postfix) with ESMTP id AAFF980008 for ; Tue, 16 Apr 2024 05:53:33 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=umich.edu header.s=google-2016-06-03 header.b="fo9vdu/D"; spf=pass (imf02.hostedemail.com: domain of tmgross@umich.edu designates 209.85.219.169 as permitted sender) smtp.mailfrom=tmgross@umich.edu; dmarc=pass (policy=none) header.from=umich.edu ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713246813; a=rsa-sha256; cv=none; b=WxyHjlfpjp+fiWeC0ls3XvlW0ObJB4+9FyBgnqom1Z661L0ra6EpXbVd3Tw9GGxjSEB7M6 PC8lT1UelHRTsBBz5JOZ6AUdVWIftqnukdxYdFvXyPdoY/YKUZGd12YOG8JECSGdmdhzlU DayJxiemc43HSgNuPxNpR1RkjPG4Ar8= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=umich.edu header.s=google-2016-06-03 header.b="fo9vdu/D"; spf=pass (imf02.hostedemail.com: domain of tmgross@umich.edu designates 209.85.219.169 as permitted sender) smtp.mailfrom=tmgross@umich.edu; dmarc=pass (policy=none) header.from=umich.edu ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1713246813; 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=fOqlvnKucywUEZPZVlRG+mSpGLeAjqCUUNbMfAp62nk=; b=ao4+4kSw6GvdNtyb5VDSZQlyiCbPt8MCVIPIoedpHPJkXrExbb/GodEFwom/rWUHgt4w3C dLxXHLBx68sSLUpekzXvbTc6LsE1A+Ua1shY7i4HdwuuJgAMbExRV5B24zjgsuS50kaTaf VmUncU2QQ4TBXYNtTYCWPc7ks/LPbdc= Received: by mail-yb1-f169.google.com with SMTP id 3f1490d57ef6-dbed179f0faso3031113276.1 for ; Mon, 15 Apr 2024 22:53:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=umich.edu; s=google-2016-06-03; t=1713246813; x=1713851613; 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=fOqlvnKucywUEZPZVlRG+mSpGLeAjqCUUNbMfAp62nk=; b=fo9vdu/D9JU8/MQI/hfhq1JByw9k+DOXHdhkPLnwdE//2rrINVzJDyLZ+pLIaQd3bs USh4Li9OE0ByjKxmPDK485pA6g/fS24CcKqvJkI4HGogSsdA5JosgM6VFZwUwAYOp7wh pOWnAwEDWLwSROCCWX6gakwx5JpcSV3dwfIz/EA3ml0wsRGUc7y7tRxDz++SCWiPDQzz 3vKqFGUcyp3DIIcnPrn5bHia9qztC2uXXuFmsI1CUK+orNC5VOkdHo71EjUbXz3fzGUo 0mTnSmKo6KC6voR8OH/y+KQzBvSeXoN03wBRZMPkYW6ed7A52WkLGOxkhdz3Hw5q++I7 ojEQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713246813; x=1713851613; 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=fOqlvnKucywUEZPZVlRG+mSpGLeAjqCUUNbMfAp62nk=; b=Z6qw0+VxHlnKFxgODnFRRw/VVDVp2woOYk9hT+cMl7fPCgJQqFCuPaKUAgqdIXZQUI lGWcnrcxJMFGlmIR7f+mwTFXOHGQlzb2UhtH8QKHZo97ovb6UnYLIm4c9o7ze0LNj1oJ 47lSG4Mf6K4TNs0HQqKYPw0MeBdbxiKlR0cofxOh075W5es6IIQ8QmJr1PtV9726O/di QTVOIqE8RMz4YQsFqRHJzRo3mPhBeo06mO9ZdjWNRhoG0AoSNggIoSqwxinkMiXdWmQc zEQ4Ybncag+FaRSaeefVC9dI7ndVYuZSt7AzxkIn7TbmjG2gBoT/QsZL2zSrsOGo2rAO qGhw== X-Forwarded-Encrypted: i=1; AJvYcCXa5SvOAoGTgheYMlvnmXgDiQDQc6SlTHwyOLxxIfa1/kLxpL6mUsy4fDRx3G2f1lXP95blhUriUqqC74gozF/F0Co= X-Gm-Message-State: AOJu0YycteBjlZXwg1E26iTvhzBvjE88jcYso6RTbiFJb5lFalWJ92Fu PDya58Yh1NZVShKWeWgGGmUTC/N3PkT6rIYepGMWvENbHjwxqQg3F+CSN+EEG3WXWSnEe2wKrI7 hiWqlteC3Zwgxw2fwz1G9JDQcabLt09BJug+ipw== X-Google-Smtp-Source: AGHT+IEk4cS/dQD+EYvxY5dx55Tkwtvot6iZduodfKV8e4eY/Ded+reARURG0wd/8TQWyTEpiJDKGNwvHHvzNoyWrEc= X-Received: by 2002:a25:1fd6:0:b0:dc6:de93:7929 with SMTP id f205-20020a251fd6000000b00dc6de937929mr1415768ybf.26.1713246812640; Mon, 15 Apr 2024 22:53:32 -0700 (PDT) MIME-Version: 1.0 References: <20240415-alice-mm-v5-0-6f55e4d8ef51@google.com> <20240415-alice-mm-v5-3-6f55e4d8ef51@google.com> In-Reply-To: <20240415-alice-mm-v5-3-6f55e4d8ef51@google.com> From: Trevor Gross Date: Tue, 16 Apr 2024 01:53:21 -0400 Message-ID: Subject: Re: [PATCH v5 3/4] rust: uaccess: add typed accessors for userspace pointers To: Alice Ryhl Cc: Miguel Ojeda , Matthew Wilcox , Al Viro , Andrew Morton , Kees Cook , Alex Gaynor , Wedson Almeida Filho , Boqun Feng , Gary Guo , =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= , Benno Lossin , Andreas Hindborg , Greg Kroah-Hartman , =?UTF-8?B?QXJ2ZSBIasO4bm5ldsOlZw==?= , Todd Kjos , Martijn Coenen , Joel Fernandes , Carlos Llamas , Suren Baghdasaryan , Arnd Bergmann , linux-mm@kvack.org, linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org, Christian Brauner Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: AAFF980008 X-Stat-Signature: 3ngo33n7am4ggn3uj4wcd9awzdskmkud X-Rspam-User: X-HE-Tag: 1713246813-249650 X-HE-Meta: U2FsdGVkX193tAURjtjdlCkwQVILPD7pBYiIlN6sLc+wRaJyJpq680W0033rYuGft9GHj3OcN8x+rdxCJtQ/1qEuLyRGr2fseh0rT0GEfC4vysAEWVgiY0XxYjz2iohVMC4BblQQ5Wm54uO0BIKr0DkVZ2qpSRLxT0XjMq+w4uRVLOcvI2BOZ8qi7nXT1qcOInZRgWo/7T/oq0SoytnilbU8+uicnyMbosJadDB4HFBEFOkiS8T1NcnVW8LO1wv0fADqc07BSf1AYbz59F7JZyPyRL57AqJqnVZmwI1kHlp83E++Nrv08UtwSxhr03PlKO5qoxWO6hoOvF5MyYDkdNYrPwO5/84Oz5gEwU7xtZcxgweurrUjGmQBGJjNDy2q26YX3oyf8wmOq2DrkSDhzkkmTx2c1d8atlY+Qc63wcXMo+zi7FRhnkw1yZbl4334QMggcCK+6hsaNAx9iiWtBcGK8UIg88TBAN3DqdzhtZ1KfbX9Vl96rhLsn79V//WJJe7/m95Oxo1RifGMYZTIIJVhuZVw56B1KaiaC9UgobC0aaG8bHtbEi7Mqq1KtydzarsZesQO5YjhBkHYGodol9eHZ5v7hYBl+r3w8wZ/15RuywA4G7vMabz42vlNCkt3oJZwRv4MfJg7hMq2oq4Q7HYT2xnbMIiYQiMAryIVhs/2Cy/HTrVbfn4eSTyb8n1/WxB6t38npJwhndRSmQoWmzT44206YpGPUsS6Rs1bBRkDBhHhZbTx+IPlNIJGusNQazHen+DYk2n6sKeY9JvSI/Z75t0pjCznUEnoeAY5A731e/+JKDiygEYlG3SFb5+1PM6pq2HzC3iofNVX/1s3nZma/4sswJzMzH0X8/ZYOV8r1QUl8sUSsRyIoAcRxHXpr+kkXNwcEnWAbljaAcc9ASnmcayMjTzs3StBroKlVZYMvYNr6Mo8uKWd93RXt7yQgBtGWoOm5hk= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Mon, Apr 15, 2024 at 3:15=E2=80=AFAM Alice Ryhl w= rote: > > Add safe methods for reading and writing Rust values to and from > userspace pointers. > > The C methods for copying to/from userspace use a function called > `check_object_size` to verify that the kernel pointer is not dangling. > However, this check is skipped when the length is a compile-time > constant, with the assumption that such cases trivially have a correct > kernel pointer. > > In this patch, we apply the same optimization to the typed accessors. > For both methods, the size of the operation is known at compile time to > be size_of of the type being read or written. Since the C side doesn't > provide a variant that skips only this check, we create custom helpers > for this purpose. > > The majority of reads and writes to userspace pointers in the Rust > Binder driver uses these accessor methods. Benchmarking has found that > skipping the `check_object_size` check makes a big difference for the > cases being skipped here. (And that the check doesn't make a difference > for the cases that use the raw read/write methods.) > > This code is based on something that was originally written by Wedson on > the old rust branch. It was modified by Alice to skip the > `check_object_size` check, and to update various comments, including the > notes about kernel pointers in `WritableToBytes`. > > Co-developed-by: Wedson Almeida Filho > Signed-off-by: Wedson Almeida Filho > Reviewed-by: Benno Lossin > Reviewed-by: Boqun Feng > Signed-off-by: Alice Ryhl Couple of docs nits but this looks good to me. Reviewed-by: Trevor Gross > +/// Types for which any bit pattern is valid. > +/// > +/// Not all types are valid for all values. For example, a `bool` must b= e either zero or one, so > +/// reading arbitrary bytes into something that contains a `bool` is not= okay. > +/// > +/// It's okay for the type to have padding, as initializing those bytes = has no effect. > +/// > +/// # Safety > +/// > +/// All bit-patterns must be valid for this type. > +pub unsafe trait FromBytes {} No `UnsafeCell` is also a requirement in zerocopy/bytemuck > +/// Types that can be viewed as an immutable slice of initialized bytes. > +/// > +/// If a struct implements this trait, then it is okay to copy it byte-f= or-byte to userspace. This > +/// means that it should not have any padding, as padding bytes are unin= itialized. Reading > +/// uninitialized memory is not just undefined behavior, it may even lea= d to leaking sensitive > +/// information on the stack to userspace. > +/// > +/// The struct should also not hold kernel pointers, as kernel pointer a= ddresses are also considered > +/// sensitive. However, leaking kernel pointers is not considered undefi= ned behavior by Rust, so > +/// this is a correctness requirement, but not a safety requirement. I don't think mentions of userspace are relevant here since the trait is more general. Maybe a `# Interfacing with userspace` section if there is enough relevant information. > +/// # Safety > +/// > +/// Values of this type may not contain any uninitialized bytes. No UnsafeCell > +pub unsafe trait AsBytes {} > diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs > index c97029cdeba1..e3953eec61a3 100644 > --- a/rust/kernel/uaccess.rs > +++ b/rust/kernel/uaccess.rs > @@ -4,10 +4,15 @@ > //! > //! C header: [`include/linux/uaccess.h`](srctree/include/linux/uaccess.= h) > > -use crate::{bindings, error::code::*, error::Result}; > +use crate::{ > + bindings, > + error::code::*, > + error::Result, > + types::{AsBytes, FromBytes}, > +}; > use alloc::vec::Vec; > use core::ffi::{c_ulong, c_void}; > -use core::mem::MaybeUninit; > +use core::mem::{size_of, MaybeUninit}; > > /// A pointer to an area in userspace memory, which can be either read-o= nly or read-write. > /// > @@ -238,6 +243,38 @@ pub fn read_slice(&mut self, out: &mut [u8]) -> Resu= lt { > self.read_raw(out) > } > > + /// Reads a value of the specified type. > + /// > + /// Fails with `EFAULT` if the read encounters a page fault. > + pub fn read(&mut self) -> Result { > [...] > + /// Writes the provided Rust value to this userspace pointer. > + /// > + /// Fails with `EFAULT` if the write encounters a page fault. > + pub fn write(&mut self, value: &T) -> Result { Read & write could use an example if you are up for it