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 54EE3C4345F for ; Fri, 26 Apr 2024 07:13:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BC4276B0085; Fri, 26 Apr 2024 03:13:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B99E16B0088; Fri, 26 Apr 2024 03:13:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A3AC16B0089; Fri, 26 Apr 2024 03:13:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 8378C6B0085 for ; Fri, 26 Apr 2024 03:13:32 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 39B40806CB for ; Fri, 26 Apr 2024 07:13:32 +0000 (UTC) X-FDA: 82050817464.15.BC1BF9C Received: from mail-vs1-f46.google.com (mail-vs1-f46.google.com [209.85.217.46]) by imf29.hostedemail.com (Postfix) with ESMTP id 7EA8712000A for ; Fri, 26 Apr 2024 07:13:30 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=NuRXz6og; spf=pass (imf29.hostedemail.com: domain of aliceryhl@google.com designates 209.85.217.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=1714115610; 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=PteTKUkLdI4YjMSnQZKKT6B1AyC9xLcHreBvLXZqBtc=; b=KPCbni1INh7hDJEq0JCVNbYkFazPkMFSzYbUT/JT3blf819pcxfEniA1sUQqgqx4O1Fz1V kAEOP4BQUAFJmhrKFdlynRrTKGzxsU2P9jcYAUi/ZTCbTA7FkOqTdQV8kX6RfqXIH8sfKy GiVniYQos34zGob2onN7Kgr2byg+PpA= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=NuRXz6og; spf=pass (imf29.hostedemail.com: domain of aliceryhl@google.com designates 209.85.217.46 as permitted sender) smtp.mailfrom=aliceryhl@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1714115610; a=rsa-sha256; cv=none; b=ijT+lAHdwl38APUpemTq4azAxtIPbco3giP1C21fznwWnZARPic64Tz1iZZBe5N8N+oJUe om0JIWjh54+eR+X+jWaOYTIbagNSkT0GAZALySPY892gNE/6BZrratXC6WuBgtZ6VcwhEW 79W1Bq0f+Xa+MwKr4w1sGZSMVq31a3s= Received: by mail-vs1-f46.google.com with SMTP id ada2fe7eead31-479ef9db155so563111137.1 for ; Fri, 26 Apr 2024 00:13:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1714115609; x=1714720409; 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=PteTKUkLdI4YjMSnQZKKT6B1AyC9xLcHreBvLXZqBtc=; b=NuRXz6ogjch4dLgHHJS6eSBxJbpplZ1ePPpGLeIMb3f1lKiphjTLRlxdeQQsZkX1lT 6NHK4Z+glOYDsed443Tzif/A2+Qp8ZLocsgPpC4o7l12H+RTpoqrClluc4SZsDwUPQgz WThcFzS5eqOP2Wr2GOfV1AUnVQpG62urctE5VKDxYrGbZA1WEXJpEBAQrcTs9P6T+NEg Nmpjadrzh1hV91cvPgQ6lLej34wYkZ/60fo4s0xXr6E2Qatz3yumlcqkYcmwZpjhcDdM 2NtLs9VRSun9k4F3yoOLfPqBJTy+ja2fDmTrDwyklF1Gx2pVPcB+pSdyCRuPDfCUIX2O FRsA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714115609; x=1714720409; 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=PteTKUkLdI4YjMSnQZKKT6B1AyC9xLcHreBvLXZqBtc=; b=BvcaRie2PD1/VxY1Zp+xyI5YhWgzEnT+oHCP7WCsvXisYISL0PRBJUU6xRhJmZp2xp 7iRn3r0Onw415f+rhcT+a5zoL3l3VCqF8Wgtcn/VUV6SKmM84HxniKka6NWpDRiSDIhN U2NQT2rl5nQtP47aahcNKYrsV0IeM2fQg+3xo9mV8V4u9UFBTdXOTMTFl/HFcTBuMJcu BCFyI3jUjrMpHLd+39oLl3luUmmeH5sdM4xgsG2kHcT4BuaL3jl3qFgE2gxcMPrZAyVM /j5I7UwmByy3kPL3Kop2tlgLNAxwEhANS9pT/MxhS+5ZZOy8RFniC3DF1FG8xzZOhT3F Xynw== X-Forwarded-Encrypted: i=1; AJvYcCVmo6UnK0sWADwSNJrE57bMnrPy0F3GI1jzhlqRuzsBe7p1ZiOPccRXTEy0fCryqMttBMJo+y0ykg7upIXKFGCF+d8= X-Gm-Message-State: AOJu0YzILqY4nneNVdnUda97mvqxQ7MVih42ihNZkM4pgg2MgdMnx3ug 6tNVjYDm6xQFFjaoqRn4KxZHZJIXPVDKTAMlbINY+1R26GuNbOhEnhDfI1HQyIVbuuEaqRjJ4NN sxYLCPfd4UHGGrYtFrRiBCzFsQwJlcizem05r X-Google-Smtp-Source: AGHT+IEY6tPxGSAoyXAMZ9xl7MkAtpE3MXWty6FU1VWiNNbdMvog3QBTWAS/HBT1lxBzTi/ClX+7Jxe7L8pspE2mwbU= X-Received: by 2002:a05:6102:457:b0:47b:614e:cbd with SMTP id e23-20020a056102045700b0047b614e0cbdmr1763298vsq.31.1714115609277; Fri, 26 Apr 2024 00:13:29 -0700 (PDT) MIME-Version: 1.0 References: <20240418-alice-mm-v6-0-cb8f3e5d688f@google.com> <20240418-alice-mm-v6-3-cb8f3e5d688f@google.com> <20240425171358.54dc96e4@eugeo> In-Reply-To: <20240425171358.54dc96e4@eugeo> From: Alice Ryhl Date: Fri, 26 Apr 2024 09:13:18 +0200 Message-ID: Subject: Re: [PATCH v6 3/4] rust: uaccess: add typed accessors for userspace pointers To: Gary Guo Cc: Miguel Ojeda , Matthew Wilcox , Al Viro , Andrew Morton , Kees Cook , Alex Gaynor , Wedson Almeida Filho , Boqun Feng , =?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 , Trevor Gross , 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-Stat-Signature: inruebaaxomkn7qdbmk6h9oz3d4hdk1b X-Rspamd-Queue-Id: 7EA8712000A X-Rspamd-Server: rspam02 X-Rspam-User: X-HE-Tag: 1714115610-596119 X-HE-Meta: U2FsdGVkX180cy9h3Kqs6Uo7Fw8ekemwMwL50B7uK07YGxOFK+2BV/S4HiDFxXBykFzWf/VlWRv2/m2msW4v3g4kJ+81uwcxlrYenxSjqE/4GhtbHKQrZAhLX3zHz39/t7RSAjiZNAS51lAM7WybEhS3tjKtCt5u8+Smos6FVvYl4l+xpy6J6SIuE7izGd4HmPQYwdUzgK7mMJcqO0gmnNn7b2CmWdgwSS6kC56Smf4ifPb/fmSb2w3zWVy6juv3tcj8vLQ2Brn18CnFCNnB3brs/rm9EV0yzuv7zlOjZENr8x/QKwmP39HksWsqAgTRx3qgotG3Hlx1PSazNQ6KjHz71pz4sKEqKLBg/dgIKcp76SiJpvNMSfVMyKxDPBujSydiqeoSR4sO4K8tkeQStNZskT+5edPIwrGCJek6cX1UKaqL06R0wFZYbs2EdE4aRrd8K+RxwSAvSYaMxqcYDx/6Vx4bE1RQ0BON5OPdPa01xRkKlDyYd96hEhQk807Q+KBJ49b4ke0WGUckfVozyq78RMJOaq4RBw/GxgbjmaVg6XX+Jtw2MVcxmmUUQpgg/J8M/6hBxwwsbUQJ1lFc2wJUucEKJe9YnvBz9d7o8jF02atLjQj6yWgpU1ZvHQeRWzHzDmXode2v4icHGUZsMiiFqqhk2G+eDbnmYUU75bpGaFwwwbZFyTqmNR8s2uUgcrZvmucXn8SvqXmqhQYg0EmteQofWaFvsPa97I5Wd5MVQG+oEXZ4J5nWnUP6ycs/u97HrDJDgLbAE5hFU9G2YzNQOmyy9VNYzN10VnkAFAonGM8Y7tjYJjvCQ7qU7EO+BF2fgTm39dYH75hTyLpYx/ptgl6ST/rN3pHeeSI4IWlFJSLFxWx11LaoxQYhrZXVu2khSob6fMG8ByTeqyVxX5oVVzPEHYjdal6NpJ/GCNpBc9LFLZXuaCdDAghHZzN0ba1mQDcHNeYB7kvGcjm 5RwRQjqK aX6pYl9J07MeoK2Q2QBwoYdLYyEYym03+BP14p6rlbAJiUCzILuxZyja0iLt1gbKrwLYUGjIi1o9I6YzroztCwJr7fpblyy5c+5fuoLD6889aLVyl3ynqoEiVNMmLhh5o9aG3bBoWdMU0RUJ2eeA2DFtRq37TR4cqAObm2gVmqiuJ8v9Wb+qeDDu5MwA6JFY8QSnL1uMMsdJEZgHPAjOCkSOBnTInuqeW7YTS1QCxJKQq0BGuJ4IRZg0WGEryW/Wbqx+ADrwyrKPbQCt334814CrnPlBLHscoktkmLVSI/7m3AEpUo2Ourp94Uibq1fWqjvHCSfU5KaPQhsWXm8QlGey8nCUARy1nKYcuD2etJGp2Hl7zizD4CqUViUvorcDy2S250Myl2fOjep00JpuRkMhSDg== 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 Thu, Apr 25, 2024 at 6:14=E2=80=AFPM Gary Guo wrote: > > On Thu, 18 Apr 2024 08:59:19 +0000 > Alice Ryhl wrote: > > > 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 o= n > > the old rust branch. It was modified by Alice to skip the > > `check_object_size` check, and to update various comments, including th= e > > 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 > > Reviewed-by: Trevor Gross > > Signed-off-by: Alice Ryhl > > Reviewed-by: Gary Guo > > > --- > > rust/kernel/types.rs | 64 ++++++++++++++++++++++++++++++++++++++++ > > rust/kernel/uaccess.rs | 79 ++++++++++++++++++++++++++++++++++++++++++= ++++++-- > > 2 files changed, 141 insertions(+), 2 deletions(-) > > > > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs > > index 8fad61268465..9c57c6c75553 100644 > > --- a/rust/kernel/types.rs > > +++ b/rust/kernel/types.rs > > > > +/// Types that can be viewed as an immutable slice of initialized byte= s. > > +/// > > +/// If a struct implements this trait, then it is okay to copy it byte= -for-byte to userspace. This > > +/// means that it should not have any padding, as padding bytes are un= initialized. Reading > > +/// uninitialized memory is not just undefined behavior, it may even l= ead to leaking sensitive > > +/// information on the stack to userspace. > > +/// > > +/// The struct should also not hold kernel pointers, as kernel pointer= addresses are also considered > > +/// sensitive. However, leaking kernel pointers is not considered unde= fined behavior by Rust, so > > +/// this is a correctness requirement, but not a safety requirement. > > +/// > > +/// # Safety > > +/// > > +/// Values of this type may not contain any uninitialized bytes. This = type must not have interior > > +/// mutability. > > +pub unsafe trait AsBytes {} > > + > > +// SAFETY: Instances of the following types have no uninitialized port= ions. > > +unsafe impl AsBytes for u8 {} > > +unsafe impl AsBytes for u16 {} > > +unsafe impl AsBytes for u32 {} > > +unsafe impl AsBytes for u64 {} > > +unsafe impl AsBytes for usize {} > > +unsafe impl AsBytes for i8 {} > > +unsafe impl AsBytes for i16 {} > > +unsafe impl AsBytes for i32 {} > > +unsafe impl AsBytes for i64 {} > > +unsafe impl AsBytes for isize {} > > +unsafe impl AsBytes for bool {} > > +unsafe impl AsBytes for char {} > > +unsafe impl AsBytes for str {} > > +// SAFETY: If individual values in an array have no uninitialized port= ions, then the array itself > > +// does not have any uninitialized portions either. > > +unsafe impl AsBytes for [T] {} > > nit: I would move `str` to here, since `str` is essentially `[u8]` with > UTF-8 guarantee. > > > +unsafe impl AsBytes for [T; N] {} Yes ... but the safety comment here talks about arrays and their individual values. I don't think it transfers cleanly to str, and that the other safety comment fits str better. Alice