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 156D4C54E71 for ; Wed, 20 Mar 2024 02:27:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 798F56B0085; Tue, 19 Mar 2024 22:27:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 748A36B0088; Tue, 19 Mar 2024 22:27:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5C2996B0089; Tue, 19 Mar 2024 22:27:58 -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 4D9016B0085 for ; Tue, 19 Mar 2024 22:27:58 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id E6BAA1211F1 for ; Wed, 20 Mar 2024 02:27:57 +0000 (UTC) X-FDA: 81915832194.18.62FDF9F Received: from mail-qv1-f52.google.com (mail-qv1-f52.google.com [209.85.219.52]) by imf01.hostedemail.com (Postfix) with ESMTP id C1BDD40010 for ; Wed, 20 Mar 2024 02:27:55 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=S2EaPVVR; spf=pass (imf01.hostedemail.com: domain of boqun.feng@gmail.com designates 209.85.219.52 as permitted sender) smtp.mailfrom=boqun.feng@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1710901675; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=BoYH80sJnhdliON/wfQAq4dWt7OAKJu6XRvbeyVwYo0=; b=xHxscvR5udJol9yOlA2nJ08TXMqEQJwFQaFRaNi7W82p74XPcvqfgcdyCksBilKCl8Kpus P5NKA167H0fosZZUQhOt0FS2kI9Ug/52JdXgWgZ9iLXSEdHUr2AFCNhx50ZGeJAUaJzty2 Sln1R6Dy935rquU/iV9i/apDK4jdFqE= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=S2EaPVVR; spf=pass (imf01.hostedemail.com: domain of boqun.feng@gmail.com designates 209.85.219.52 as permitted sender) smtp.mailfrom=boqun.feng@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1710901675; a=rsa-sha256; cv=none; b=WG5m4aPrqMkt1VqtFWOLitN42MajQ4++SCN3nrLfJxD4dBTarQ3s1ykJdnRrFGSeU7xGvG r6bLYrzoDiXZbK7PFA13lat5KL02e6k+nPslvWk29g5WxynT9Qz4zL1GxqUeQPp0qC3QkL wkd9Vx4BWQjsmOb24YHzqH9VLemH2do= Received: by mail-qv1-f52.google.com with SMTP id 6a1803df08f44-695df25699fso28893416d6.2 for ; Tue, 19 Mar 2024 19:27:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1710901675; x=1711506475; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:feedback-id:from:to:cc:subject:date :message-id:reply-to; bh=BoYH80sJnhdliON/wfQAq4dWt7OAKJu6XRvbeyVwYo0=; b=S2EaPVVRsssz1de5l+Cseed32RB85zGJMfWKoLuhNkZAluWYqsE8aGIrwhwJOs/tfY T/3s1NXKCgkUxFhA3oX0Swet7sxGXKCbdJ1lA7vdDohs6y8NtyoBzOQKap73Elr8Z6pl OSU5qMkzrlw5IHl3FgIN11JGxFV+ibdm6bL7OR/Xvct6Wx9MGBqkZAFsT5mnsyJa7oBs yUKCXwbAGM+CPAzMvwNHvWJdOgHOc2JxFtKDNXKlcPUUPN6UZtwDULvbFqHaChWXFYVa VOOI5i7AOWLGU5ujxCKW5yn0XnbDXKiwwp5kQBz/P9hit31dB8lyiE0KSLVOj5RmwH/h 0UKQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710901675; x=1711506475; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:feedback-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=BoYH80sJnhdliON/wfQAq4dWt7OAKJu6XRvbeyVwYo0=; b=sfVhWHYTZMSB9cAt7v87dow8D39cAVDC8fIaRhDq+8po93LFG3zPATqUSoW0tTlUMI 9DMsvm3HVn8IpXBT9zQVOlvLBxnxDXoSQN9ihQEbmpZzVIhLcv+d4HslL3Dm8RftVLRJ 0j2wj4f0X9PECn2bbQMeJa3LzSvTDdVffFGzQo5bT/lJgPt9zxwZbMeteeQNuBjtsPyg 3l1mH8CqLRthmm1ceuGemIDrX405AJEQRmBqx01LYlP4ZM1DukKwxZ20R6XRtAgQtkOg 6MkGVrbIjRk+FKq392fhUxgG0XWEq9EK9A7jxOLqt7Pt+CuFp+3Qp5VrlQAhYPiFFr+t NLVQ== X-Forwarded-Encrypted: i=1; AJvYcCU96oKede07tMVd3UIBCmomqwyo8MKLkfmLLDR2ljViJr4nAQOUz81I0XapYqZ0Ro1neUN4SK9B/E0NmGy4pTVST0M= X-Gm-Message-State: AOJu0Yw8M3xKrvJkENEtsF/uvUtwTUsuSXXeqjzJSoSTdemK5hn0oINd JyFbKDMjJncAwVLBRbg2WAmTTJVrnaHUEb7BCwRSp3G4qQW0nrJ5 X-Google-Smtp-Source: AGHT+IHkAo9eiqZEBwRiPx6Er0DVaobmgBZ9GU8BbZoe5uwTFIH5mrJndnxU84N5b4b+Ka9j6hodbw== X-Received: by 2002:a05:6214:5188:b0:690:bdd5:f25e with SMTP id kl8-20020a056214518800b00690bdd5f25emr618855qvb.59.1710901674843; Tue, 19 Mar 2024 19:27:54 -0700 (PDT) Received: from fauth1-smtp.messagingengine.com (fauth1-smtp.messagingengine.com. [103.168.172.200]) by smtp.gmail.com with ESMTPSA id j10-20020a0cf30a000000b006915cc2f655sm7167436qvl.30.2024.03.19.19.27.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 Mar 2024 19:27:54 -0700 (PDT) Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailfauth.nyi.internal (Postfix) with ESMTP id A9CAF1200032; Tue, 19 Mar 2024 22:27:53 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute7.internal (MEProxy); Tue, 19 Mar 2024 22:27:53 -0400 X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrledvgddujecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehttdertddttddvnecuhfhrohhmpeeuohhquhhn ucfhvghnghcuoegsohhquhhnrdhfvghnghesghhmrghilhdrtghomheqnecuggftrfgrth htvghrnhepvefhueduudehhedtffehffdutddugefgteeihfdvvddvhfdufefggefgfefg vdelnecuffhomhgrihhnpehrvggrshhonhdrthhopdduqddvtddtuddrshhonecuvehluh hsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepsghoqhhunhdomhgv shhmthhprghuthhhphgvrhhsohhnrghlihhthidqieelvdeghedtieegqddujeejkeehhe ehvddqsghoqhhunhdrfhgvnhhgpeepghhmrghilhdrtghomhesfhhigihmvgdrnhgrmhgv X-ME-Proxy: Feedback-ID: iad51458e:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 19 Mar 2024 22:27:52 -0400 (EDT) Date: Tue, 19 Mar 2024 19:27:38 -0700 From: Boqun Feng To: Alice Ryhl Cc: Miguel Ojeda , Matthew Wilcox , Al Viro , Andrew Morton , Kees Cook , Alex Gaynor , Wedson Almeida Filho , Gary Guo , =?iso-8859-1?Q?Bj=F6rn?= Roy Baron , Benno Lossin , Andreas Hindborg , Greg Kroah-Hartman , Arve =?iso-8859-1?B?SGr4bm5lduVn?= , 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 Subject: Re: [PATCH v3 1/4] rust: uaccess: add userspace pointers Message-ID: References: <20240311-alice-mm-v3-0-cdf7b3a2049c@google.com> <20240311-alice-mm-v3-1-cdf7b3a2049c@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240311-alice-mm-v3-1-cdf7b3a2049c@google.com> X-Rspamd-Queue-Id: C1BDD40010 X-Rspam-User: X-Stat-Signature: 6sup65hmwdxjabw1a9bq88a67sj5yhsp X-Rspamd-Server: rspam01 X-HE-Tag: 1710901675-366280 X-HE-Meta: U2FsdGVkX1+3gitobkFNvKumjXvnS40YPhhujteecVU8jVJI03YhV+hjpoI1n//REibaQ97GrBb5YyAg4yJ1vIdg7tZYA+0Th6PomoxzKU5+nkHa4JAtOK7mzvBNoKwJQ1hizoyoo3AbEli2ouogm+WwfPa4AS8seufwKSQzxJHXSAqnmtLQpONLI+nX00eDjUJR0e7Y0Dsz3t+w7WnZb8ElbJklS8sMV/4gRfnZFUj+1S5caDB2dCE1/rsFVQgaYpJt2EjOtjiACxPJPs1asxosUQvpw/NyOJbffwkNMLIgNLNikYVmlCUxRv0/zGUTnMhjCO1zUVGbe7Qy7hkOIXDH9AdUvEerVc/asRecf+b3O7ObFAU1wwKG90p+XV9cX0bImNkPg9Z9S7blQCaIo/U9PtzzPSruOL9QBiSSHAjNQPOyQD3K9FIoKoBuXXrWMLJH9u+BsVrZ8i1vSwG2AWVEoX0IIXSZZGlOzATcgkmKENHqq4SbRRjz/SEjYc/HYek3xnreMVs1WabvU0nEBv+J0v7Z5jw48JLzSlis7gjxjZIa1UFDp7Ub8kZsptgaKNmoK24o1q8jrZYr6E99EbxHscMfjJqrJzCkvC3obTT/bNV1h/rEdobDjs7RB1eOa/wBZKUZjXZPPERyQlt379z91o5+BJYebXIyT+lM/77xPmupuSzdj2mKB23lBGynnliWmZKkRJchRPtrJP5B5YJCYfDEFwuTtUUehxFrJuHwZd+rxDeU5nmYq3RjEasv1n3c3wMoENUjBa8idcvpP59wUe1CoCuj8EdVTrl8A7HQaWKsPg94Xh9himoSVIYd1Kv4krChdien3qU5VVtNxD/whExYZxMXW3JTg3mndZIuCbORDXsZt0YMR0O7UP1E61M/cw99RFMtvX8drHHFPsqO1z3ema472iUuAJDA0HLBe/8I3aLJIkYobaVPCwQ76ZsDAnxV7O3LjIOxF4W 6o8/2okU zPYcjaeMqmz5udTvvOd2ocj1NEfXFkXJR3ya4SVfonFE5pR+Gnk4oJ1oRSH7bUkdV/ZHCEIj1Zs9dMRCyZAdC+tDj7sGv9b5ZBMqyLl9GkD8PnX7eXvuoU492zlsliLrjuvsWD181jqDHPAIdHzGffH0To/epseQ9rWlLgP0hQXi+7XU= 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: Hi Alice, I was trying to work on a patch for UserSlice::read_slice(), and I found a few place that may need some documentation improvements. Please see below: On Mon, Mar 11, 2024 at 10:47:13AM +0000, Alice Ryhl wrote: [...] > diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs > new file mode 100644 > index 000000000000..020f3847683f > --- /dev/null > +++ b/rust/kernel/uaccess.rs > @@ -0,0 +1,315 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! User pointers. Since the type is renamed as UserSlice, maybe: //! Slices to user space memory regions. ? > +//! > +//! C header: [`include/linux/uaccess.h`](srctree/include/linux/uaccess.h) > + > +use crate::{bindings, error::code::*, error::Result}; > +use alloc::vec::Vec; > +use core::ffi::{c_ulong, c_void}; > + > +/// 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 invalid > +/// pointers will return `EFAULT`. Concurrent access, *including data races Probably reword this a little bit: "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`." , please see below for the reason. > +/// 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 > +/// (time-of-check to time-of-use) bugs. Every time a memory location is read, > +/// the reader's position is advanced by the read length and the next read will > +/// start from there. This helps prevent accidentally reading the same location > +/// twice and causing a TOCTOU bug. > +/// > +/// Creating a [`UserSliceReader`] and/or [`UserSliceWriter`] consumes the > +/// `UserSlice`, helping ensure that there aren't multiple readers or writers to > +/// the same location. > +/// > +/// If double-fetching a memory location is necessary for some reason, then that > +/// is done by creating multiple readers to the same memory location, e.g. using > +/// [`clone_reader`]. > +/// [...] > + /// Reads raw data from the user slice into a raw kernel buffer. > + /// > + /// Fails with `EFAULT` if the read encounters a page fault. Technically, this is not correct, since normal page faults can happen during copy_from_user() (for example, userspace memory gets swapped). So returning `EFAULT` really means the read happens on a bad address, which also matches `EFAULT`'s definition: EFAULT Bad address (POSIX.1-2001). so maybe reword this and the similar ones below into something like: /// Fails with `EFAULT` if the read happens on a bad address. Otherwise, people may think that this function just abort whenever there is a page fault. Thoughts? Regards, Boqun > + /// > + /// # Safety > + /// > + /// The `out` pointer must be valid for writing `len` bytes. > + pub unsafe fn read_raw(&mut self, out: *mut u8, len: usize) -> Result { > + if len > self.length { > + return Err(EFAULT); > + } > + let Ok(len_ulong) = c_ulong::try_from(len) else { > + return Err(EFAULT); > + }; > + // SAFETY: The caller promises that `out` is valid for writing `len` bytes. > + let res = unsafe { bindings::copy_from_user(out.cast::(), self.ptr, len_ulong) }; > + if res != 0 { > + return Err(EFAULT); > + } > + // Userspace pointers are not directly dereferencable by the kernel, so > + // we cannot use `add`, which has C-style rules for defined behavior. > + self.ptr = self.ptr.wrapping_byte_add(len); > + self.length -= len; > + Ok(()) > + } > + [...]