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 0A9F1C04FF8 for ; Tue, 16 Apr 2024 09:53:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 942DA6B0087; Tue, 16 Apr 2024 05:53:33 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8F1C76B0088; Tue, 16 Apr 2024 05:53:33 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 791EE6B0089; Tue, 16 Apr 2024 05:53:33 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 54EB96B0087 for ; Tue, 16 Apr 2024 05:53:33 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id BD6EA1A0A9F for ; Tue, 16 Apr 2024 09:53:32 +0000 (UTC) X-FDA: 82014932664.26.BB68646 Received: from mail-yw1-f201.google.com (mail-yw1-f201.google.com [209.85.128.201]) by imf08.hostedemail.com (Postfix) with ESMTP id E519616000E for ; Tue, 16 Apr 2024 09:53:30 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=P0KCkbsX; spf=pass (imf08.hostedemail.com: domain of 3mUoeZgkKCN4ALICERYHLGOOGLE.COMLINUX-MMKVACK.ORG@flex--aliceryhl.bounces.google.com designates 209.85.128.201 as permitted sender) smtp.mailfrom=3mUoeZgkKCN4ALICERYHLGOOGLE.COMLINUX-MMKVACK.ORG@flex--aliceryhl.bounces.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=1713261210; 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=T+zM/toovgZChd6lSt17rRhWXRLFcTgeeFTkR1bkDyE=; b=h4W/JUmyKv9GkOy2gKKED8BWO7GVsx4dh9YrN1eHLZRmd2zAYU5llXuFCGL6QkAOA6CnBH 6eDZDFiEmEu26rvEQ1KWZ7vGYCVJky0FDCLupFkg2XbusDID1cnpfoSaUeswxOMxPHN8P6 8ULzg8HT/LUyNtvf52mxlM4gBcv7wNA= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=P0KCkbsX; spf=pass (imf08.hostedemail.com: domain of 3mUoeZgkKCN4ALICERYHLGOOGLE.COMLINUX-MMKVACK.ORG@flex--aliceryhl.bounces.google.com designates 209.85.128.201 as permitted sender) smtp.mailfrom=3mUoeZgkKCN4ALICERYHLGOOGLE.COMLINUX-MMKVACK.ORG@flex--aliceryhl.bounces.google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713261210; a=rsa-sha256; cv=none; b=DUIVAoCaiy0jbnkOnv2QL5iPX8ZHxS0g0UR64/lSO/9dFDtfM38eUYm+q5dKsweoCdJsmi HvAnLtB/BsuaS5U92Fygn9W7eMt1DaftMFlkENxSTkyJP3RvLv+/qd0uiOuJ2xoPXZv/dO Erq578fNVhH8k907gJzDV8S5yaHAFvw= Received: by mail-yw1-f201.google.com with SMTP id 00721157ae682-61afb46bad8so821027b3.0 for ; Tue, 16 Apr 2024 02:53:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1713261210; x=1713866010; darn=kvack.org; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:from:to:cc:subject:date:message-id :reply-to; bh=T+zM/toovgZChd6lSt17rRhWXRLFcTgeeFTkR1bkDyE=; b=P0KCkbsXNEyZswVXE9/7c89UIc+I7kS2Dd0Grp7roZ3EVeXEwHnKxX79fA43bXonvn LrihlINVu0UvmJekzIsJFvxh2DNFU2CwbnoGaL/aizQFXVlZwqmEhRxQ0bggW232E8sQ DqrFDmcd46psat/kX6YFnXDQzIkyT5QcdOYhajkylizICVMH+D/xH/sGLknK5iWVvKyP MN0mul4SVFBHg3cogCmSMo2ONp+gdoEyRawBvPS7dFLVvHWDs6lOtz8SS8cI1LUPFCgC wmohp4FR7elWcsJBdTOcZLK3hnJ6wgKKoNjodAkni/q17nrHcRZW4TDhWYPrtl65phbH Y9Iw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713261210; x=1713866010; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=T+zM/toovgZChd6lSt17rRhWXRLFcTgeeFTkR1bkDyE=; b=fZ/XXi+MMYmv5m+Af7KuZ4FpGx0nbCYNm+KnnIKW0dOCs/JhQ2cI5OCRH3DXTmUrPV k8gLaZ3LZHlIzf/ljsGTZ09dyB1t2PC4GNzb0tO2gNGCp/wV5Wr359khOstCTPNPConU wH+NPG0QR9y5jZ+flmC/UF9RBc9Qrfhjqg3c0cOXQYUwv7B1/3EomoTTVe8QqRYf4YMC glha8AQmtOQbB9pjTHJVLkmS1FjoolpCELWncK1kfoT37erM1OSo4NY2oNi6z2KMZpKC slYoG+06d5QTLwaTChSjBLCD5LD5zpXMkmxxIIiP8UiA/QzETUc4DyDOlrP4UHx/G+tB GFrQ== X-Forwarded-Encrypted: i=1; AJvYcCVsrNeYwD6GSyBQnRNTKuQSgspKcRoVEEOM5T3QhgeFhWWaTeGy90iG3axhe1HHiSFLH4wHZ4ZZYTcZxoVWLvNwdos= X-Gm-Message-State: AOJu0YxxJK3f7LUY3mfCKU8HnSo9VQyj6nOdNtfo7t1Pa5GV31Z7v3OS bGW1AqlT6HdrdQblcSYsQAzcDMa0pWiQysHS+uw13xjmbt+kHpxEIDTQ0yhXXdCuEsVvASXfYoq XmkScA6D74CRKWQ== X-Google-Smtp-Source: AGHT+IHqpiuhJ/B/pAW+yzXL6D9AWrgYwUHawkxp0orSfwtiF7ApZ+7li+Co9gqRhQKL/XN+D4nrEDtdSip48zs= X-Received: from aliceryhl2.c.googlers.com ([fda3:e722:ac3:cc00:68:949d:c0a8:572]) (user=aliceryhl job=sendgmr) by 2002:a0d:d850:0:b0:618:62d9:5dbb with SMTP id a77-20020a0dd850000000b0061862d95dbbmr578543ywe.0.1713261209871; Tue, 16 Apr 2024 02:53:29 -0700 (PDT) Date: Tue, 16 Apr 2024 09:53:27 +0000 In-Reply-To: Mime-Version: 1.0 References: X-Mailer: git-send-email 2.44.0.683.g7961c838ac-goog Message-ID: <20240416095327.1108227-1-aliceryhl@google.com> Subject: Re: [PATCH v5 1/4] rust: uaccess: add userspace pointers From: Alice Ryhl To: tmgross@umich.edu Cc: a.hindborg@samsung.com, akpm@linux-foundation.org, alex.gaynor@gmail.com, aliceryhl@google.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, willy@infradead.org Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: E519616000E X-Rspam-User: X-Stat-Signature: 9nq1r8wbeguis84fhdnwkkqrbmddiryk X-Rspamd-Server: rspam01 X-HE-Tag: 1713261210-990762 X-HE-Meta: U2FsdGVkX187452KlWBre3lZEJ2Yu2OSndldJRHnNCqhsfdowRDrjkW2LiI1p0c7RvuRAnKzhGb+xqSWWp/fjcH5Y47y6gx2H5UFOqjQ/6t+PpVFiHl/9V0vvw4qUr6RLVhS9g2QKAiBqT3foIsCM6Zju1JimmnmL5UIsBCkYlGT+PJM068y5TWDSc1+MbBhRdYsbKzVHckga5336SfQCQ+SIxLZisZVo4RfebdDGvFao0Co8nBG1iMxEsfT37L888i7sz2CkuW6PYY1A6pLP8SigEEG6jXPMRAPuuZLKY5tSbQHSJRvabDQnAn4MIPyGS8qBx/E+N6s8HZeVZqgOlXYoXouhg6yNjAU2dpXrkk/VsT72B/UWiLwI7cZDYIfgvQVd/fnXx29SOK/jGCp8jZrF5NQ/WEYN8q8ZjFa1AgEBMvzJcNCf3/cPuZXZx4rK7InyY+SrObxl7tOqXqed3bbhnIkGcPrEEOS1fF2ntVpMw03PfFr0FXHMYmFfuWVAeb7N7U2el87TYyCV/+Ql3VzTmTsjttbHqzmQlF6K0fE6VivD6LQ0mFhfjJmKXfyDM6KDRrZbh2/XA/u4I+kso/jDydNXSXf4W1uEtiPBt3r4INpeuz0+7iu+FQWbEKqMP6/+qY0XCD8GxoaElVVv/ZoZL4JeHZJHgBebupoLG7JsONaRYH7g9/SPsyAsxhHGPgh62embWfui/HMDlMDiocTB9DNq8hgKXhzlpH72qT0H9Yz10TvMP8PEXmPN6hybqq7aZaRHuRNvDQc8ZFrw0TGEDLpcPOMzZ3bQMj7CYDBnkYxpwtRDIEd5T+fXoj8qNeIf730tMrJCeKzVN96TXJb5dT14ByrFtttar1XRTmqov5ZghiJGbgNYToHMA/FTVEM07g17W/IGzCKqCpI2RjF9GvV60lpGyPwdVavxwBd3/NeI5TEWPrFpHJ3Fg1ohoPWL2YM4l7ysMzFQ7J Igq0LS1h l+SWJkeNXywvI6YxQNQRKbXDG4oIYXbxltQeJwaBzJVDmTp26HmPj5MBi8hsa/q9sSt6yE6WtlKyFwzbQavbMcdfAtSpdMmioKDRkqC+Iz2aHW1pLazuQMz39X1bvCmokNd0Sa5kDltfxx8rdoi8PQ9XpaPf0cp8EXQg6qbJ2NgZ2Y2E95reOGZq/HPqbyq8JDzxKLIjjUN7l0Pj9IxCrPeM7WSrkOFtBmadSyYfiO/Cckz9nIUkiRYGNXonBULHE1pBwlxAAglBwjeeMQWXVyw4xPHOUWGvMsmWHHdM0WLpjv3Vc/mAL1TQA3EOKKteSMt+tTsklzvneBZFysKCU2H0dw/bO2m93av+Eb+XfaWXWFcC3IL4aYyBOwyRKTOPYIwDhuqR2NbSBJNVa2JKZSjg5WBD0uO80P549GSogoFbJkF2WZZXGa+J+TYcbS57cr6ujIdF4jl/q+kycoJY1ND0A3zre2Dbb4SE0DOZq35FCDqHXKVI0ccPHhEOtmnUqLQFTOL9HgTr8D96JD3yZKLRV6fsHCfGt072Vt+Z/+SlLexcKvUHqvGBlCzSpE0iTg+CEBNdaaSfk2cGOQL9YHOmcyHvAcmY94fA2WsOwyGGo9Bf6ZXGe4gHQCK/r0t3yS1tKoSzQCkn0cJdhwrUxCkAynCNNioYEOwCe6uzPDUmd02ePA+DYqi5Akeveig+PFSwmyNGWrSSHW3SdR9vsm0s/BECjdE7ltBBsXEp4H6C/uvuppvOYxmzF0A== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000004, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Trevor Gross writes: > On Mon, Apr 15, 2024 at 3:14=E2=80=AFAM Alice Ryhl = wrote: >> >> 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 >=20 > Reviewed-by: Trevor Gross >=20 > I left some suggestions for documentation improvements and one > question, but mostly LGTM. Thanks for taking a look! >> +impl UserSlice { >> + /// Constructs a user slice from a raw pointer and a length in byte= s. >> + /// >> + /// Constructing a [`UserSlice`] performs no checks on the provided= address and length, it can >> + /// safely be constructed inside a kernel thread with no current us= erspace 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 wi= thin the user range (no >> + /// additional calls to `access_ok` are needed). >=20 > I would just add a note that pointer should be a valid userspace > pointer, but that gets checked at read/write time Will do. >> + /// Callers must be careful to avoid time-of-check-time-of-use (TOC= TOU) 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 } >> + } >=20 >> +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 = initialized. >=20 > If this is guaranteed, could it return `Result<&mut [u8]>`? So the > caller doesn't need to unsafely `assume_init` anything. It could, but I don't think it's that useful. All existing callers will want to record it somewhere with something like `Vec::set_len`, which this doesn't help with. There are ways to do something like that, but it complicates the API further which I am not interested in. >> + /// Fails with `EFAULT` if the read happens on a bad address. >=20 > This should also mention that the slice cannot be bigger than the > reader's length. I can add a note. =20 >> + 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 ke= rnel, 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 writ= e uninitialized bytes to >> + // `out`. >> + let out =3D unsafe { &mut *(out as *mut [u8] as *mut [MaybeUnin= it]) }; >> + self.read_raw(out) >> + } >=20 > If this is just a safe version of read_raw, could you crosslink the docs? Okay. >> +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 { >> [...] >> + } >=20 > Could use a note about length like `read_raw`. Okay. Alice