From: Valentin Obst <kernel@valentinobst.de>
To: aliceryhl@google.com
Cc: a.hindborg@samsung.com, akpm@linux-foundation.org,
alex.gaynor@gmail.com, arnd@arndb.de, arve@android.com,
benno.lossin@proton.me, bjorn3_gh@protonmail.com,
boqun.feng@gmail.com, brauner@kernel.org, cmllamas@google.com,
gary@garyguo.net, gregkh@linuxfoundation.org,
joel@joelfernandes.org, keescook@chromium.org,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
maco@android.com, ojeda@kernel.org,
rust-for-linux@vger.kernel.org, surenb@google.com,
tkjos@android.com, viro@zeniv.linux.org.uk, wedsonaf@gmail.com,
Valentin Obst <kernel@valentinobst.de>
Subject: Re: [PATCH 1/3] rust: add userspace pointers
Date: Thu, 25 Jan 2024 00:12:35 +0100 [thread overview]
Message-ID: <20240124231235.6183-1-kernel@valentinobst.de> (raw)
In-Reply-To: <20240124-alice-mm-v1-1-d1abcec83c44@google.com>
> +//! User pointers.
> +//!
> +//! C header: [`include/linux/uaccess.h`](../../../../include/linux/uaccess.h)
> +
nit: could this be using srctree-relative links?
> +/// The maximum length of a operation using `copy_[from|to]_user`.
nit: 'a' -> 'an'
> +///
> +/// If a usize is not greater than this constant, then casting it to `c_ulong`
> +/// is guaranteed to be lossless.
nit: could this be `usize` or [`usize`]. Maybe would also be clearer to
say "... a value of type [`usize`] is smaller than ..."
> +///
> +/// 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
Maybe makes sense to also introduce the abbreviation TOCTOU in the type
documentation when it is first used.
> + /// Reads the entirety of the user slice.
> + ///
> + /// Returns `EFAULT` if the address does not currently point to
> + /// mapped, readable memory.
> + pub fn read_all(self) -> Result<Vec<u8>> {
> + self.reader().read_all()
> + }
If I understand it correctly, the function will return `EFAULT` if _any_
address in the interval `[self.0, self.0 + self.1)` does not point to
mapped, readable memory. Maybe the docs could be more explicit.
> + // Since this is not a pointer to a valid object in our program,
> + // we cannot use `add`, which has C-style rules for defined
> + // behavior.
> + self.0 = self.0.wrapping_add(len);
If I understand it correctly, you are using 'valid object' to refer to
an 'allocated object' [1] as this is what the `add` method's docs
refer to [2]. In that case it might be better to use the latter term as
it has a defined meaning. Also see [3] and [4] which are about making it
more precise.
[1]: https://doc.rust-lang.org/core/ptr/index.html#allocated-object
[2]: https://doc.rust-lang.org/core/primitive.pointer.html#method.add
[3]: https://github.com/rust-lang/rust/pull/116675
[4]: https://github.com/rust-lang/unsafe-code-guidelines/issues/465
next prev parent reply other threads:[~2024-01-24 23:13 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-24 11:20 [PATCH 0/3] Memory management patches needed by Rust Binder Alice Ryhl
2024-01-24 11:20 ` [PATCH 1/3] rust: add userspace pointers Alice Ryhl
2024-01-24 23:12 ` Valentin Obst [this message]
2024-02-08 12:20 ` Alice Ryhl
2024-02-01 4:06 ` Trevor Gross
2024-02-08 12:53 ` Alice Ryhl
2024-02-08 15:35 ` Greg Kroah-Hartman
2024-02-08 15:41 ` Alice Ryhl
2024-02-08 15:59 ` Greg Kroah-Hartman
2024-02-10 6:20 ` Kees Cook
2024-02-10 7:06 ` Trevor Gross
2024-02-10 14:14 ` David Laight
2024-02-12 9:30 ` Alice Ryhl
2024-01-24 11:20 ` [PATCH 2/3] rust: add typed accessors for " Alice Ryhl
2024-01-24 23:46 ` Valentin Obst
2024-01-25 12:40 ` Alice Ryhl
2024-01-25 12:26 ` Arnd Bergmann
2024-01-25 12:37 ` Alice Ryhl
2024-01-25 15:59 ` Arnd Bergmann
2024-01-25 16:15 ` Alice Ryhl
2024-02-01 5:03 ` Trevor Gross
2024-02-08 13:14 ` Alice Ryhl
2024-01-24 11:20 ` [PATCH 3/3] rust: add abstraction for `struct page` Alice Ryhl
2024-01-26 0:46 ` Boqun Feng
2024-01-26 12:33 ` Alice Ryhl
2024-01-26 18:28 ` Boqun Feng
2024-02-01 6:50 ` Trevor Gross
2024-02-05 17:23 ` Boqun Feng
2024-02-08 13:36 ` Alice Ryhl
2024-01-30 9:15 ` Andreas Hindborg (Samsung)
2024-01-29 17:59 ` Matthew Wilcox
2024-01-29 18:56 ` Carlos Llamas
2024-01-29 20:19 ` Matthew Wilcox
2024-01-29 21:27 ` Alice Ryhl
2024-01-30 9:02 ` Andreas Hindborg
2024-01-30 9:06 ` Alice Ryhl
2024-02-01 6:02 ` Trevor Gross
2024-02-08 13:46 ` Alice Ryhl
2024-02-08 14:02 ` Andreas Hindborg
2024-02-08 14:12 ` Alice Ryhl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240124231235.6183-1-kernel@valentinobst.de \
--to=kernel@valentinobst.de \
--cc=a.hindborg@samsung.com \
--cc=akpm@linux-foundation.org \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=arnd@arndb.de \
--cc=arve@android.com \
--cc=benno.lossin@proton.me \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=brauner@kernel.org \
--cc=cmllamas@google.com \
--cc=gary@garyguo.net \
--cc=gregkh@linuxfoundation.org \
--cc=joel@joelfernandes.org \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=maco@android.com \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=surenb@google.com \
--cc=tkjos@android.com \
--cc=viro@zeniv.linux.org.uk \
--cc=wedsonaf@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.