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 5C78FD62073 for ; Tue, 19 Nov 2024 17:08:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E97EE6B0085; Tue, 19 Nov 2024 12:08:23 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E474E6B0088; Tue, 19 Nov 2024 12:08:23 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D35C66B0089; Tue, 19 Nov 2024 12:08:23 -0500 (EST) 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 B5BDE6B0085 for ; Tue, 19 Nov 2024 12:08:23 -0500 (EST) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 617031A05EA for ; Tue, 19 Nov 2024 17:08:23 +0000 (UTC) X-FDA: 82803475272.09.455745C Received: from mail-ed1-f41.google.com (mail-ed1-f41.google.com [209.85.208.41]) by imf19.hostedemail.com (Postfix) with ESMTP id 2ED491A0012 for ; Tue, 19 Nov 2024 17:07:17 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="G7o/hMsE"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of jannh@google.com designates 209.85.208.41 as permitted sender) smtp.mailfrom=jannh@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1732035917; a=rsa-sha256; cv=none; b=tBj3aHZzfiLfC/+TkCVS7ESKJLyt83NP5pzDDP77Ji7I0O2fkh7dKzbEeBNsD3fbm8VwbO HCvJNYsH5WsgVh3+eSQue3Frbm7sJE+2Pfz1mtUNzvIAXHjtJdFpyneCjwuGiK95FtVgPU inFHZ92EQ85sR4NVTR7pXfrs0TL4GXk= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="G7o/hMsE"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of jannh@google.com designates 209.85.208.41 as permitted sender) smtp.mailfrom=jannh@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1732035917; 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=uCV9WHZTFYTHla+38liafRITO+eDswnLk5ctpG/DKf4=; b=SSwf+cWinhqtOfgnvYxrZp0upyomYnDEbvU9Wo7hpgSgs41xowgZ9xXFil78UTIM++V3rJ bDy4ZbmSXWPGc0fFLkLBIMbkmA6LyOGGErCz9i6EiSe5vT0F4WFHdn5wAVtvmUkoe69vIf 93Wt7+JRMJXcV1sjVYMh7DPPkWsLtGU= Received: by mail-ed1-f41.google.com with SMTP id 4fb4d7f45d1cf-5cf6f5d4939so9084a12.1 for ; Tue, 19 Nov 2024 09:08:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1732036100; x=1732640900; 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=uCV9WHZTFYTHla+38liafRITO+eDswnLk5ctpG/DKf4=; b=G7o/hMsEueanmqTP1iTamKuM/O6uxjqeGZhv5sk2hLvnNOgrfoOcgdJxbxnqb7vjH9 /vxoVB394SC0VJkDAptEjrSsMaepYn/Ml6U/GPN3eGlMGq3pYNM0xbePfXJ+OZ8zsTP5 a45al0nJb2eE81Y/dX2O5j2i+7bkLie2G3g2F0jrONEoNTm16tpDp8N0LmMo1XRw/EWL XyhRdmjIOCHBrnIl6W4hoC9fusdFtz0AAWpHWKPAIYtoxvgk9tA+UF5oS4Hy0wpz2DjZ K/N7wQd4IgBC9AwRO4PgTra4HGmtSLmy05lhzGGnSjq9jM7MfE9TPth4TY/aB6Pw5Sox QjJg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732036100; x=1732640900; 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=uCV9WHZTFYTHla+38liafRITO+eDswnLk5ctpG/DKf4=; b=w4sVkhZ4TskspaKsF1IERdhb4d8BYgHdJB11BQLu2WoipA+ZHnLuX6UunS/RGRRSf8 1kH3Fp+69f5FKzKC6zPbqgI4CoevTWxkVnSQBQNrkD5JcACUOT2TkaQ5QgPDz+dOH/Sh p9shKf6vGbCoZTVI8412thvROncby6N3Sd2tVFrVrOebtUqafN3YUCnP+kRrGMjv/XUW XJ3w70c0esJJr2t7sw724guQLEEiqrVahNPEoWMeOCo6gcLH3w/d9CiHm8YdJ3VpHOqe 4YssawEWFgawP41A3GHfo0HQLTVjEIfkdkSc9cn/i6KaxkwTftlzcK5DHQPmsNyihwPl kWJQ== X-Forwarded-Encrypted: i=1; AJvYcCU6ZTdSvufF3sQ3mYO5tun1ygN3Z8mGDSeK4UpwtUjAcAVo0IR2TFsH7uL///2idl5qmFXsl3IQJw==@kvack.org X-Gm-Message-State: AOJu0YwULYjlYnKKiZXQ/rd/XEeoIkY91q+zbaFoowktBtnRm7dvyD9j BkkloJTV4EBhsQ8oPLpWD/wSZC+XGCik4UxR9AP0FmumxP2VFu0q96kk68ontGCA8XdC363Ro/H /G0KnBidfw+75GsQDcghUwbgCiPezddv9CvW2 X-Gm-Gg: ASbGncvhJ0weP9cgqs2iTxOd2Usri+MtRI9e6oA5DVTqDgvXH7Uk2lL+155Ei11a1vn 9XWOIoqfWZTvnnLMPzDteaLsf+FAvDq6Y9HYHNnOeYFADl38Fk5OJTfpQxJo= X-Google-Smtp-Source: AGHT+IF2NA4+N5BCrudBX2E2EdWGDzoY5N2xiN8KTBuW6FT8jxbNTi1fhsn3CWPCChG9Plc207DSpKkVbPrO1sPa7NU= X-Received: by 2002:a05:6402:50c6:b0:5cf:deaf:ac2 with SMTP id 4fb4d7f45d1cf-5cfdec1bb2cmr115336a12.2.1732036099141; Tue, 19 Nov 2024 09:08:19 -0800 (PST) MIME-Version: 1.0 References: <20241119112408.779243-1-abdiel.janulgue@gmail.com> <20241119112408.779243-3-abdiel.janulgue@gmail.com> In-Reply-To: <20241119112408.779243-3-abdiel.janulgue@gmail.com> From: Jann Horn Date: Tue, 19 Nov 2024 18:07:43 +0100 Message-ID: Subject: Re: [PATCH v3 2/2] rust: page: Extend support to existing struct page mappings To: Abdiel Janulgue Cc: rust-for-linux@vger.kernel.org, Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= , Benno Lossin , Andreas Hindborg , Alice Ryhl , Trevor Gross , Danilo Krummrich , Wedson Almeida Filho , Valentin Obst , open list , Andrew Morton , "open list:MEMORY MANAGEMENT" , airlied@redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: jpi4mnp9c8okzs8srt4azms4e9hynsho X-Rspamd-Queue-Id: 2ED491A0012 X-Rspamd-Server: rspam08 X-Rspam-User: X-HE-Tag: 1732036037-285477 X-HE-Meta: U2FsdGVkX1+VHbXGi71yz4AOhKHxExSGD0I1d+K5x83YWuPtaE1Fs1NPfjLC0OCqHWhxUPS6g8tCgCqMmIxfiLJ9Eb35pW+cmKso2102BorxS3VcZikO2tNRyilQIhgOdcuGaNA5sYdOoxudLtOYHD77QmLKtpllhOYH4tJw6eUyWNAMVSl+b5shymXSxyC1JWnEkCg/Wrqb4wCOaH+Tp52wJbjxzVJFE/F80Efiv3AmqSaDz40raPO0wtuTNvIL1No1ndK1rqrIM2EW8HL6nHDiRBVktxDhkDtG8TatxHBQYuvRra/4d74ccWdwpDHATlJlCguYfo+yzHNpw/goX8dIsWmK2jBPysjhbMbV8uF0akIHNQqPEgVwOeyp1n9O4wMLS7wxqNGDDf2WIY/SzGaaweRvhisYYbHMN9QL4tFWJdC6mE9JMuMSg70NZl5a+nIlVRoq/+ZGzKH4QhzyLMPlpuMqK/da9tuBs7yNantJh/mvvr9PufT/bYyTg9wY8w/5ZHFhzjtHrdOEIpZlAyLRd5gVsytwsszX/Yv1m9+bC3+KbTOdsjT1U8Z9U5Qr5W+QmWAlZNsGmie5wCRNn9vYhmzHUd96x2GoaD759zVYfowhvVIUqpzVYdK9vl4IAqOVRTEY4Re9rkKvyrbZsTi7xypoKt8HdBn+SRjjXLPEZ1nK+X1QSMyn0u1Whkiz0q0LXgfQwKMvkJcaU+fc1Jx3v5evaCKgOTjoGZOSQFXlt2jS9L4uMMxyPgUkH14VHHr0WkPwJ6nLkgZptOkzBSpvOMWJ1Nfp/SbbPlBWhpZYOoHlMz08lGIlLgS4cORUQXk1g3hjLSnv1Fy9HbNA0RlMsEslXTcv3XjY+VzzP70Y60OEBkm69bckyYEDP9DJZWkOCiPGEpsh+4VH516LaBpevn1Wb83lGprQAE5ih5KD3pB5gBFyZ/u+QMqprSIMVctiEpe0J9g1JjzF2Qw w7Jk0Ezp QV9fR2IqAyhh14C9snkmoZEpKa3H/RauEVW4XUQ/PpmMkpsEaCwNszXYSXRFv6Tj8C2+qaontnKb2mZGrkJCKUNT10txb2+eegOKznOmCDYPdYEmP6bwRXIwblFomTgi5sxheTaaUQqsZVmWeYoXUosY0+2Q1IZyAv/URVhH9mNZAgGxfcb3KSblcQFgR1xQh5EUnvP0srmMYGfuYaqvXAphcc4Vy+94+FWKfS1PEP+l2jT/92R1G3cbFnYI55eGLEpN2Ume7yUjYM1Xqz06kw9p8PyxMDcdub+fA 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: Commenting as someone who understands kernel MM decently but only knows a tiny bit about Rust: On Tue, Nov 19, 2024 at 12:24=E2=80=AFPM Abdiel Janulgue wrote: > Extend Page to support pages that are not allocated by the constructor, > for example, those returned by vmalloc_to_page() or virt_to_page(). > Since we don't own those pages we shouldn't Drop them either. Hence we > take advantage of the switch to Opaque so we can cast to a Page pointer > from a struct page pointer and be able to retrieve the reference on an > existing struct page mapping. In this case no destructor will be called > since we are not instantiating a new Page instance. > > The new page_slice_to_page wrapper ensures that it explicity accepts > only page-sized chunks. > > Signed-off-by: Abdiel Janulgue [...] > diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs > index fdf7ee203597..d0a896f53afb 100644 > --- a/rust/kernel/page.rs > +++ b/rust/kernel/page.rs > @@ -3,7 +3,7 @@ > //! Kernel page allocation and management. > > use crate::{ > - alloc::{AllocError, Flags}, > + alloc::{AllocError, Allocator, Flags, VVec, KVec, KVVec, Vec, flags:= :*}, > bindings, > error::code::*, > error::Result, > @@ -87,6 +87,49 @@ pub fn alloc_page(flags: Flags) -> Result, = AllocError> { > Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(ptr)) }) > } > > + /// Create a page object from a buffer which is associated with an e= xisting C `struct page`. > + /// > + /// This function ensures it takes a page-sized buffer as represente= d by `PageSlice`. > + /// > + /// # Examples > + /// > + /// ``` > + /// use kernel::page::*; > + /// > + /// let somedata: [u8; PAGE_SIZE * 2] =3D [0; PAGE_SIZE * 2]; > + /// let buf: &[u8] =3D &somedata; > + /// let pages: VVec =3D buf.try_into()?; > + /// let page =3D Page::page_slice_to_page(&pages[0])?; > + /// # Ok::<(), Error>(()) > + /// ``` > + pub fn page_slice_to_page<'a>(page: &PageSlice) -> Result<&'a Self> Sorry, can you explain to me what the semantics of this are? Does this create a Page reference that is not lifetime-bound to the PageSlice? > + { > + let ptr: *const core::ffi::c_void =3D page.0.as_ptr() as _; > + if ptr.is_null() { > + return Err(EINVAL) > + } > + // SAFETY: We've checked that `ptr` is non-null, hence it's safe= to call this method. > + let page =3D if unsafe { bindings::is_vmalloc_addr(ptr) } { > + // SAFETY: We've checked that `ptr` is non-null and within t= he vmalloc range, hence > + // it's safe to call this method. > + unsafe { bindings::vmalloc_to_page(ptr) } > + // SAFETY: We've checked that `ptr` is non-null, hence it's safe= to call this method. > + } else if unsafe { bindings::virt_addr_valid(ptr) } { > + // SAFETY: We've checked that `ptr` is non-null and a valid = virtual address, hence > + // it's safe to call this method. > + unsafe { bindings::virt_to_page(ptr) } > + } else { > + ptr::null_mut() > + }; > + if page.is_null() { > + return Err(EINVAL); > + } > + // CAST: `Self` is a `repr(transparent)` wrapper around `binding= s::page`. > + // SAFETY: We just successfully retrieved an existing `bindings:= :page`, therefore > + // dereferencing the page pointer is valid. > + Ok(unsafe { &*page.cast() }) > + } > + > /// Returns a raw pointer to the page. > pub fn as_ptr(&self) -> *mut bindings::page { > self.page.get() > @@ -270,3 +313,55 @@ unsafe fn dec_ref(obj: ptr::NonNull) { > unsafe { bindings::put_page(obj.cast().as_ptr()) } > } > } > + > +/// A page-aligned, page-sized object. > +/// > +/// This is used for convenience to convert a large buffer into an array= of page-sized chunks > +/// allocated with the kernel's allocators which can then be used in the > +/// `Page::page_slice_to_page` wrapper. > +/// > +// FIXME: This should be `PAGE_SIZE`, but the compiler rejects everythin= g except a literal > +// integer argument for the `repr(align)` attribute. > +#[repr(align(4096))] > +pub struct PageSlice([u8; PAGE_SIZE]); > + > +fn to_vec_with_allocator(val: &[u8]) -> Result, AllocError> { > + let mut k =3D Vec::::new(); > + let pages =3D page_align(val.len()) >> PAGE_SHIFT; > + match k.reserve(pages, GFP_KERNEL) { Do I understand correctly that this can be used to create a kmalloc allocation whose pages can then basically be passed to page_slice_to_page()? FYI, the page refcount does not protect against UAF of slab allocations through new slab allocations of the same size. In other words: The slab allocator can internally recycle memory without going through the page allocator, and the slab allocator itself does not care about page refcounts. If the Page returned from calling page_slice_to_page() on the slab memory pages returned from to_vec_with_allocator() is purely usable as a borrow and there is no way to later grab a refcounted reference to it or pass it into a C function that assumes it can grab a reference to the page, I guess that works. But if I understand correctly what's going on here, and you can grab references to slab pages returned from page_slice_to_page(to_vec_with_allocator()), that'd be bad. > + Ok(()) =3D> { > + // SAFETY: from above, the length should be equal to the vec= tor's capacity > + unsafe { k.set_len(pages); } > + // SAFETY: src buffer sized val.len() does not overlap with = dst buffer since > + // the dst buffer's size is val.len() padded up to a multipl= e of PAGE_SIZE. > + unsafe { ptr::copy_nonoverlapping(val.as_ptr(), k.as_mut_ptr= () as *mut u8, > + val.len()) }; > + Ok(k) > + }, > + Err(_) =3D> Err(AllocError), > + } > +}