From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5097440E8DB for ; Fri, 5 Jun 2026 13:49:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780667377; cv=none; b=sX1d8aRLsNFuJv0e5sL8LYlzkV73ppPTCPw2j0FPf4ScZrerGKxEbiL9u1jEv8hAd2T0lklqULktqCwTXV7Iachby/NOlCXi9oW9LyNv43pEb/WFJ4olL1BbtfjC4v5JC455h0pDlabtkmIDEYU+BBVXejNQYjQfKH+Pdwqqhnk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780667377; c=relaxed/simple; bh=KTXS4bSniXv1T9Lpl7LThf2fu0ztGfgBSEx3Oe+rZRk=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=NaWlmh2fKWLjaplrqejUQ2lf1tnad+t0u+6p4Tg4+6qLE1PlXlbU1s+gsHclg+vqxDggRNClk+xdcuDrHehHgivOa/bk6L7EuJZafF3rLnkUkSf2eiYkyu1cvxBWHe0lLiY9Wffg6ZbCZFvIDQGJyEsYhekAwnpu0OHCObs6eAc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=RfJBG7+A; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="RfJBG7+A" Received: by smtp.kernel.org (Postfix) with ESMTPSA id B6C0E1F00893; Fri, 5 Jun 2026 13:49:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1780667375; bh=NOPCHrzvq5CVQ0Mcn5Bk28yhps2kKyAdDW4GC+Kd1wE=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=RfJBG7+AKc4vFm0yolWSn6ut3MzqJ/i+zK+QagG1WD0OEeDNux64PFP5Fjr352I/9 UehLP4Ft8uwgK9MlwAJ9OgwBflybZZb0Ze/yVgJ+7pL6wyGPWQhDjmF6DCcvFAB9i6 9Ckn00rSWP8DeR+gT43utpRsc15S1bzR3mKGH/PgqThZtME/ifD0O3/1PvFkP5Jwg4 GwZxrIyac14aj8wmhjJ5TRPK8/8BVhOGYtfCLYOqAbi9hp7iDsW7Tqf8dcQ3NxI10a 5+P3w/0fQzjbBslaXhd6dpXvI4ZpebxeplcOrOjQuKihM27qkIjP87/yZDukz46deT X7KIXbdh15W+g== From: sashiko-bot@kernel.org Subject: Re: [PATCH 1/3] rust: rcu: add RcuBox type Reply-To: sashiko-reviews@lists.linux.dev To: "Boqun Feng" Cc: ojeda@kernel.org, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org In-Reply-To: <20260605133541.22569-2-boqun@kernel.org> References: <20260605133541.22569-2-boqun@kernel.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Fri, 05 Jun 2026 13:49:35 +0000 Message-Id: <20260605134935.B6C0E1F00893@smtp.kernel.org> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 2 potential issue(= s) to consider: - [Critical] RcuBox::into_foreign drops `self` instead of forgetting it, ca= using a guaranteed Use-After-Free. - [High] RcuBox::drop bypasses the custom Allocator A and hardcodes kvfree_= call_rcu() for types that do not require dropping, causing undefined behavi= or for non-kvfree compatible allocators. -- commit f69c06884595bb68a76f4f1dd4014ee9e2c9e31c Author: Alice Ryhl rust: rcu: add RcuBox type =20 This adds an RcuBox container, which is like Box except that the value is freed after waiting for one grace period (via {kvfree_,}call_rcu()). To allow containers to rely on the RCU properties of RcuBox, an extension of ForeignOwnable is added. > diff --git a/rust/kernel/sync/rcu/rcu_box.rs b/rust/kernel/sync/rcu/rcu_b= ox.rs > new file mode 100644 > index 0000000000000..943fe3e8974e0 > --- /dev/null > +++ b/rust/kernel/sync/rcu/rcu_box.rs [ ... ] > +// SAFETY: > +// * The `RcuBoxInner` was allocated with `A`. > +// * `NonNull::as_ptr` returns a non-null pointer. > +unsafe impl ForeignOwnable for RcuBox { > + const FOREIGN_ALIGN: usize =3D , A> as ForeignOwn= able>::FOREIGN_ALIGN; > + > + type Borrowed<'a> =3D &'a T; > + type BorrowedMut<'a> =3D &'a T; > + > + fn into_foreign(self) -> *mut c_void { > + self.0.as_ptr().cast() > + } [Severity: Critical] Since `self` is consumed by value here without being wrapped in `ManuallyDr= op` or passed to `core::mem::forget()`, won't it be dropped at the end of this method? This seems like it would immediately schedule the memory for RCU free, caus= ing a use-after-free for the foreign code when it later accesses the pointer. > + > + unsafe fn from_foreign(ptr: *mut c_void) -> Self { [ ... ] > +impl Drop for RcuBox { > + fn drop(&mut self) { > + // SAFETY: The `rcu_head` field is in-bounds of a valid allocati= on. > + let rcu_head =3D unsafe { &raw mut (*self.0.as_ptr()).rcu_head }; > + if core::mem::needs_drop::() { > + // SAFETY: `rcu_head` is the `rcu_head` field of `RcuBoxInne= r`. All users will be > + // gone in an rcu grace period. This is the destructor, so w= e may pass ownership of the > + // allocation. > + unsafe { bindings::call_rcu(rcu_head, Some(drop_rcu_box::)) }; > + } else { > + // SAFETY: All users will be gone in an rcu grace period. > + // TODO: We are luckily since `kvfree_call_rcu()` works on b= oth kmalloc and vmalloc, > + // maybe a new `Allocator` method is needed. > + unsafe { bindings::kvfree_call_rcu(rcu_head, self.0.as_ptr()= .cast()) }; [Severity: High] Since `RcuBox` is generic over any custom `A: Allocator`, wouldn't calling `kvfree_call_rcu()` here break for allocators other than kmalloc or vmalloc? Does this need to always route through `A::free()` via `drop_rcu_box`, or should `A` be explicitly bounded to an allocator trait that supports `kvfree_rcu`? > + } > + } > +} --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260605133541.2256= 9-1-boqun@kernel.org?part=3D1