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 B4040C4345F for ; Tue, 16 Apr 2024 05:05:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2C6BE6B0082; Tue, 16 Apr 2024 01:05:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 276226B0085; Tue, 16 Apr 2024 01:05:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 13D2C6B0087; Tue, 16 Apr 2024 01:05:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id E62A66B0082 for ; Tue, 16 Apr 2024 01:05:34 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 9C617A1439 for ; Tue, 16 Apr 2024 05:05:34 +0000 (UTC) X-FDA: 82014206988.23.1B7A8C7 Received: from mail-yw1-f173.google.com (mail-yw1-f173.google.com [209.85.128.173]) by imf19.hostedemail.com (Postfix) with ESMTP id 7122C1A0013 for ; Tue, 16 Apr 2024 05:05:32 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=umich.edu header.s=google-2016-06-03 header.b=l4otb+D1; spf=pass (imf19.hostedemail.com: domain of tmgross@umich.edu designates 209.85.128.173 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=1713243932; a=rsa-sha256; cv=none; b=tDo7NTDL3V2K0TZwNqgoHAHLgHiXicJcxn15lBZdXB4PMT+zlHUmhgdQBaZ7ts1UQXVsxx z2NEOt05g74Ya8ysiaV2zTXl8I+MN1EJwib433rS1oLSKxzdWtRGZr+hqWh5gj/XFXOBCN 8b43xXIQ8BmENl7Y2AUSTk+5UMMF4AU= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=umich.edu header.s=google-2016-06-03 header.b=l4otb+D1; spf=pass (imf19.hostedemail.com: domain of tmgross@umich.edu designates 209.85.128.173 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=1713243932; 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=qJmDU0qCtC6xbk/2Z6msCh6alcZpNMwqU1+ATj0oOrg=; b=q5R6nrseCYSYqV1pcLOZsL46p3Dc/UmwYKa6rnidqA1QrEPMwO7sDG5mHXzaoKCOg/qlrf +MMgkvyUbNfE6TeOkKVP1M5gTjpqIEAbWJL2b7LH9ppU2Vp+8MO7+XLvSlxb5EfTeTN4QA h8wd4C3wK7q27heiFMJx9lcxdRvGAi4= Received: by mail-yw1-f173.google.com with SMTP id 00721157ae682-617dfcf80aeso45355047b3.3 for ; Mon, 15 Apr 2024 22:05:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=umich.edu; s=google-2016-06-03; t=1713243931; x=1713848731; 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=qJmDU0qCtC6xbk/2Z6msCh6alcZpNMwqU1+ATj0oOrg=; b=l4otb+D1nh8tuPY1y24uzto7Tizh2JkTaosxUgE/tg73V2aESiTQF5MS7g1gy4H/bC CSJdijqf0yvDG5R6wglVDhb/n93IzwhkdHwbhgXC0d/bhy4ICKm0oye6X65cqo+6O9Hk P+o9s/zmojFGmelBF8oy81BR1qDDDRWmNi27IhZzGteY3ROzNZEOd3hDpPfkiNjTOCKu 0oNgAR8Hz7r3vuJULzKh8NJGZEckOckLzno8F2EchZdLIxKdAKOeUM2Pk/8uNNfT36Hh Zbu5GuHUUqqSzQHhzawWu5qs3IerPOEoptHXLMAqrPAzTcc4ryV07eh2iZlisHZINjjn Y1tQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713243931; x=1713848731; 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=qJmDU0qCtC6xbk/2Z6msCh6alcZpNMwqU1+ATj0oOrg=; b=RpRqIjdOtBoQDyMV9WqLr8C3Dqau5QqIP2M8Kc+tvOZFwA6qj19ytTQxlAtme+htLa rXHirvhbshBlOPT5POhRBZt6oHrPp8WzXj60/SpIxep8sLmigNLPi29GNR6U/4ji2eoq dJJ5U2xbTI3/0xSZObTI4dXThFHQqXRTy4SQ3iu0p6VnNT6kwzyiPmfqNImFig5Ng2g0 UXu5k/VByYQdb+VE0G7a9fpGUD3XkzAhXE9tbAHkXQDD/1ejUW43XkH/Z2LDcNR2KENd e2AITYG0owfrMDzXoGjbL8i1M03k2yYsCqH9iBp9Ft/y+vJUnIKQvphtv7OaX4SaXQl1 H/Yg== X-Forwarded-Encrypted: i=1; AJvYcCXmlBvp2RKDOrdxOSX68FCKjZLzPkUD8XYgjKQrRD4QNEX1JyhTtaWcWB8KUW+e9VnmTf3lJz3RhP3Mg6TP9dHMdAo= X-Gm-Message-State: AOJu0Yxj8RXNkY7pzZu0rSxTTLdotxthfdzP6z4e5CvIJkD0iJ9DV6T2 hHw+XjhWa+pULfv/CpKAN8oZwkPe3dB67lAnF5vMgEuejC7Qom5Krg6svwKpNvT1YW4aZP8JsRM /onzz76JQFyYeNlK4e+k+noLdjj2m5Kks0n1ScA== X-Google-Smtp-Source: AGHT+IGA2IjVKw8IkJCjL9FhLOVDIrOwEj1lKKmglsv29PQSjP7wQuUdMHMf3ycCLE4DIaLikqvNM7nDkFaVD2tnzjE= X-Received: by 2002:a25:ea05:0:b0:de1:27f4:38a5 with SMTP id p5-20020a25ea05000000b00de127f438a5mr10411763ybd.14.1713243931364; Mon, 15 Apr 2024 22:05:31 -0700 (PDT) MIME-Version: 1.0 References: <20240415-alice-mm-v5-0-6f55e4d8ef51@google.com> <20240415-alice-mm-v5-1-6f55e4d8ef51@google.com> In-Reply-To: <20240415-alice-mm-v5-1-6f55e4d8ef51@google.com> From: Trevor Gross Date: Tue, 16 Apr 2024 01:05:20 -0400 Message-ID: Subject: Re: [PATCH v5 1/4] rust: uaccess: add 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: 7122C1A0013 X-Stat-Signature: us6sy5oq6zbrrwai4wwr946kert5db9w X-Rspam-User: X-HE-Tag: 1713243932-497821 X-HE-Meta: U2FsdGVkX1+XHl1UfXJZqP0A+VRl4IP+bxl2duXDJRLtakQ0DTZqEU1pKgFVPRWDApUrQlqSTMYMK7amaUPiPil1O4RooJqSbhJZ8kdZ7/hj+W8gweBCdyr+ZZaO9W5eu87Iput/zsgKbjgl5gzPF5k6s1WKhP4FSFK+KDR0a6rBZH+qvqeO00Y1C9DBrZNYaGY/Ie9fjB/0FFXJGEwNt3y6ibPBo+idWJbEmO9lE3Q+LayWARwg1anYlKU11HReS3bRjqBODrKYtNeCa7kD4fLzhVeaRfZR+f9C5Tiq+2Q1XJ2tobojoRPDAwxUYmW356xmqr2l8zlXsHlykCsHPDmR3sobGdvtHhlhJMW2sOM/5CMzM+ix19lkAI1CGZc2GHw2HHVc4w5F9Ed8PsW63PtRMdebcpQGLoAJ/yQJJ7MMbQqojb8/+6pS3bcX/rTeGQHUm/wZ9Mpfe/le1JsMiVSAQ0gDnhq/4llmzWCibso/8iyk5BTFi/9ytziq+ent96VbcJ5p7JSPYxkfTFOwFsnqt6B5MaCe0L65JcKslD0N9HdoZGEpPZth1ycJfyg43eqHXU6iod/RmhPfayOlQ+BdTUFSqs+QNG3KTGF2aaKk16u7uETf1kmTV9Fp1h1dbcLFUqcUhiMFBFhRsBqtpBHI9IP2Ifd4wfPbhEZDINMNQWLpCA4R4gWcMPfCq5PF8pvA8BThufrrsPvLpJKeAV2rg/rpuEVm68luFg8tyWHUj9I9KJhUglIsMXvSMrVOL54sxA0xEn/Mym9tpbAZHh4gjEbyIT8sSsRBiII8pO9+yN5FgLehpIiPWfjiRdOS97x00OVjr3a7+ptpwem5wj3ikGd/cnp8NDM/nGCNI42mhEGMALMaAWZHmk8sSRnVbTG2yVCaNoYf2Ix3Tgu2sySdI3N7aVdUf/0OIsqAG2ScZFSnkHofU++E4AARb5ZqE2dxIod6FVSVU4pt97p gqaiChng I9ViCpZxo5Aas0XD1HEEyYbjcGJY0vx7nVOKFTedcBjVzgz/yYWYRgyrw4GlCV2mOTVuT+5886F+S/0Up0/vssIm42+VseYjVacIXoAnsg8b915Q+vVNAgq4aLbyjoAZO0iYi79CW5J4KmKkrw+SdI7jxtZ3+u0Wc8j+nTWAVJ+P8YyZWBrjyyKe+wrDcoRkzoZCOw6i19+IeZZlilDzs1wa47M3WJjpXCaIYQxaCBLp/SyBE87/pAFE4AIUozVYq2Qo69CqOD5Kchu2rlUjD5GF59sboYNb6rQOEP/F69yc25OvwEwDZzlLPhJ6iGpdE23mC7T8N/w2cPU2kT7pBw7kvIGXvJOBqsTylxSbUJhfZ26ij+GSn17cflc+RYd0HBMAN 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:14=E2=80=AFAM Alice Ryhl w= rote: > > From: Wedson Almeida Filho > > A pointer to an area in userspace memory, which can be either read-only > or read-write. > > All methods on this struct are safe: attempting to read or write on bad > addresses (either out of the bound of the slice or unmapped addresses) > will return `EFAULT`. Concurrent access, *including data races to/from > userspace memory*, is permitted, because fundamentally another userspace > thread/process could always be modifying memory at the same time (in the > same way that userspace Rust's `std::io` permits data races with the > contents of files on disk). In the presence of a race, the exact byte > values read/written are unspecified but the operation is well-defined. > Kernelspace code should validate its copy of data after completing a > read, and not expect that multiple reads of the same address will return > the same value. > > These APIs are designed to make it difficult to accidentally write > TOCTOU bugs. Every time you read from a memory location, the pointer is > advanced by the length so that you cannot use that reader to read the > same memory location twice. Preventing double-fetches avoids TOCTOU > bugs. This is accomplished by taking `self` by value to prevent > obtaining multiple readers on a given `UserSlicePtr`, and the readers > only permitting forward reads. If double-fetching a memory location is > necessary for some reason, then that is done by creating multiple > readers to the same memory location. > > Constructing a `UserSlicePtr` performs no checks on the provided > address and length, it can safely be constructed inside a kernel thread > with no current userspace process. Reads and writes wrap the kernel APIs > `copy_from_user` and `copy_to_user`, which check the memory map of the > current process and enforce that the address range is within the user > range (no additional calls to `access_ok` are needed). > > This code is based on something that was originally written by Wedson on > the old rust branch. It was modified by Alice by removing the > `IoBufferReader` and `IoBufferWriter` traits, and various other changes. > > Signed-off-by: Wedson Almeida Filho > Co-developed-by: Alice Ryhl > Signed-off-by: Alice Ryhl Reviewed-by: Trevor Gross I left some suggestions for documentation improvements and one question, but mostly LGTM. > --- > rust/helpers.c | 14 +++ > rust/kernel/lib.rs | 1 + > rust/kernel/uaccess.rs | 304 +++++++++++++++++++++++++++++++++++++++++++= ++++++ > 3 files changed, 319 insertions(+) > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > index be68d5e567b1..37f84223b83f 100644 > --- a/rust/kernel/lib.rs > +++ b/rust/kernel/lib.rs > @@ -49,6 +49,7 @@ > pub mod task; > pub mod time; > pub mod types; > +pub mod uaccess; > pub mod workqueue; > > #[doc(hidden)] > diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs > new file mode 100644 > index 000000000000..c97029cdeba1 > --- /dev/null > +++ b/rust/kernel/uaccess.rs > @@ -0,0 +1,304 @@ > [...] > +impl UserSlice { > + /// Constructs a user slice from a raw pointer and a length in bytes= . > + /// > + /// Constructing a [`UserSlice`] performs no checks on the provided = address and length, it can > + /// safely be constructed inside a kernel thread with no current use= rspace process. Reads and > + /// writes wrap the kernel APIs `copy_from_user` and `copy_to_user`,= which check the memory map > + /// of the current process and enforce that the address range is wit= hin the user range (no > + /// additional calls to `access_ok` are needed). I would just add a note that pointer should be a valid userspace pointer, but that gets checked at read/write time > + /// Callers must be careful to avoid time-of-check-time-of-use (TOCT= OU) issues. The simplest way > + /// is to create a single instance of [`UserSlice`] per user memory = block as it reads each byte > + /// at most once. > + pub fn new(ptr: *mut c_void, length: usize) -> Self { > + UserSlice { ptr, length } > + } > +impl UserSliceReader { > [...] > + /// Reads raw data from the user slice into a kernel buffer. > + /// > + /// After a successful call to this method, all bytes in `out` are i= nitialized. If this is guaranteed, could it return `Result<&mut [u8]>`? So the caller doesn't need to unsafely `assume_init` anything. > + /// Fails with `EFAULT` if the read happens on a bad address. This should also mention that the slice cannot be bigger than the reader's length. > + pub fn read_raw(&mut self, out: &mut [MaybeUninit]) -> Result { > + let len =3D out.len(); > + let out_ptr =3D out.as_mut_ptr().cast::(); > + if len > self.length { > + return Err(EFAULT); > + } > + let Ok(len_ulong) =3D c_ulong::try_from(len) else { > + return Err(EFAULT); > + }; > + // SAFETY: `out_ptr` points into a mutable slice of length `len_= ulong`, so we may write > + // that many bytes to it. > + let res =3D unsafe { bindings::copy_from_user(out_ptr, self.ptr,= len_ulong) }; > + if res !=3D 0 { > + return Err(EFAULT); > + } > + // Userspace pointers are not directly dereferencable by the ker= nel, so we cannot use `add`, > + // which has C-style rules for defined behavior. > + self.ptr =3D self.ptr.wrapping_byte_add(len); > + self.length -=3D len; > + Ok(()) > + } > + > + /// Reads raw data from the user slice into a kernel buffer. > + /// > + /// Fails with `EFAULT` if the read happens on a bad address. > + pub fn read_slice(&mut self, out: &mut [u8]) -> Result { > + // SAFETY: The types are compatible and `read_raw` doesn't write= uninitialized bytes to > + // `out`. > + let out =3D unsafe { &mut *(out as *mut [u8] as *mut [MaybeUnini= t]) }; > + self.read_raw(out) > + } If this is just a safe version of read_raw, could you crosslink the docs? > +impl UserSliceWriter { > + > + /// Writes raw data to this user pointer from a kernel buffer. > + /// > + /// Fails with `EFAULT` if the write happens on a bad address. > + pub fn write_slice(&mut self, data: &[u8]) -> Result { > [...] > + } Could use a note about length like `read_raw`.