| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20251015194936.121586-1-delcastillodelarosadaniel@gmail.com>
Date: Wed, 15 Oct 2025 21:49:35 +0200
From: Daniel del Castillo <delcastillodelarosadaniel@...il.com>
To: Danilo Krummrich <dakr@...nel.org>,
Alexandre Courbot <acourbot@...dia.com>,
David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>,
Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>
Cc: nouveau@...ts.freedesktop.org,
dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org,
Boqun Feng <boqun.feng@...il.com>,
Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <lossin@...nel.org>,
Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>,
Trevor Gross <tmgross@...ch.edu>,
rust-for-linux@...r.kernel.org,
Daniel del Castillo <delcastillodelarosadaniel@...il.com>
Subject: [PATCH 1/2] nova-core: Solve mentions of `CoherentAllocation` improvements [COHA]
This patch solves the existing mentions of COHA, a task
in the Nova task list about improving the `CoherentAllocation` API.
I confirmed by talking to Alexandre Courbot, that the reading/writing
methods in `CoherentAllocation` can never be safe, so
this patch doesn't actually change `CoherentAllocation`, but rather
tries to solve the existing references to [COHA].
Signed-off-by: Daniel del Castillo <delcastillodelarosadaniel@...il.com>
---
drivers/gpu/nova-core/dma.rs | 20 ++---
drivers/gpu/nova-core/firmware/fwsec.rs | 104 ++++++++++--------------
2 files changed, 50 insertions(+), 74 deletions(-)
diff --git a/drivers/gpu/nova-core/dma.rs b/drivers/gpu/nova-core/dma.rs
index 94f44bcfd748..639a99cf72c4 100644
--- a/drivers/gpu/nova-core/dma.rs
+++ b/drivers/gpu/nova-core/dma.rs
@@ -25,21 +25,11 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, len: usize) -> Result<Sel
}
pub(crate) fn from_data(dev: &device::Device<device::Bound>, data: &[u8]) -> Result<Self> {
- Self::new(dev, data.len()).map(|mut dma_obj| {
- // TODO[COHA]: replace with `CoherentAllocation::write()` once available.
- // SAFETY:
- // - `dma_obj`'s size is at least `data.len()`.
- // - We have just created this object and there is no other user at this stage.
- unsafe {
- core::ptr::copy_nonoverlapping(
- data.as_ptr(),
- dma_obj.dma.start_ptr_mut(),
- data.len(),
- );
- }
-
- dma_obj
- })
+ let mut dma_obj = Self::new(dev, data.len())?;
+ // SAFETY: We have just created this object and there is no other user at this stage.
+ unsafe { dma_obj.write(data, 0)? };
+
+ Ok(dma_obj)
}
}
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index 8edbb5c0572c..cc5a6200d0de 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -11,12 +11,12 @@
//! - The ucode signature, so the GSP falcon can run FWSEC in HS mode.
use core::marker::PhantomData;
-use core::mem::{align_of, size_of};
+use core::mem::size_of;
use core::ops::Deref;
use kernel::device::{self, Device};
use kernel::prelude::*;
-use kernel::transmute::FromBytes;
+use kernel::transmute::{AsBytes, FromBytes};
use crate::dma::DmaObject;
use crate::driver::Bar0;
@@ -70,6 +70,8 @@ struct FalconAppifDmemmapperV3 {
}
// SAFETY: any byte sequence is valid for this struct.
unsafe impl FromBytes for FalconAppifDmemmapperV3 {}
+// SAFETY: any byte sequence is valid and it contains no padding nor kernel pointers.
+unsafe impl AsBytes for FalconAppifDmemmapperV3 {}
#[derive(Debug)]
#[repr(C, packed)]
@@ -82,6 +84,8 @@ struct ReadVbios {
}
// SAFETY: any byte sequence is valid for this struct.
unsafe impl FromBytes for ReadVbios {}
+// SAFETY: any byte sequence is valid and it contains no padding nor kernel pointers.
+unsafe impl AsBytes for ReadVbios {}
#[derive(Debug)]
#[repr(C, packed)]
@@ -94,6 +98,8 @@ struct FrtsRegion {
}
// SAFETY: any byte sequence is valid for this struct.
unsafe impl FromBytes for FrtsRegion {}
+// SAFETY: any byte sequence is valid and it contains no padding nor kernel pointers.
+unsafe impl AsBytes for FrtsRegion {}
const NVFW_FRTS_CMD_REGION_TYPE_FB: u32 = 2;
@@ -104,6 +110,8 @@ struct FrtsCmd {
}
// SAFETY: any byte sequence is valid for this struct.
unsafe impl FromBytes for FrtsCmd {}
+// SAFETY: any byte sequence is valid and it contains no padding nor kernel pointers.
+unsafe impl AsBytes for FrtsCmd {}
const NVFW_FALCON_APPIF_DMEMMAPPER_CMD_FRTS: u32 = 0x15;
const NVFW_FALCON_APPIF_DMEMMAPPER_CMD_SB: u32 = 0x19;
@@ -149,24 +157,9 @@ impl FirmwareSignature<FwsecFirmware> for Bcrt30Rsa3kSignature {}
///
/// Callers must ensure that the region of memory returned is not written for as long as the
/// returned reference is alive.
-///
-/// TODO[TRSM][COHA]: Remove this and `transmute_mut` once `CoherentAllocation::as_slice` is
-/// available and we have a way to transmute objects implementing FromBytes, e.g.:
-/// https://lore.kernel.org/lkml/20250330234039.29814-1-christiansantoslima21@gmail.com/
-unsafe fn transmute<'a, 'b, T: Sized + FromBytes>(
- fw: &'a DmaObject,
- offset: usize,
-) -> Result<&'b T> {
- if offset + size_of::<T>() > fw.size() {
- return Err(EINVAL);
- }
- if (fw.start_ptr() as usize + offset) % align_of::<T>() != 0 {
- return Err(EINVAL);
- }
-
- // SAFETY: we have checked that the pointer is properly aligned that its pointed memory is
- // large enough the contains an instance of `T`, which implements `FromBytes`.
- Ok(unsafe { &*(fw.start_ptr().add(offset).cast::<T>()) })
+unsafe fn transmute<T: Sized + FromBytes>(fw: &DmaObject, offset: usize) -> Result<&T> {
+ // SAFETY: Guaranteed by the safety requirements of the function.
+ T::from_bytes(unsafe { fw.as_slice(offset, size_of::<T>())? }).ok_or(EINVAL)
}
/// Reinterpret the area starting from `offset` in `fw` as a mutable instance of `T` (which must
@@ -176,20 +169,12 @@ unsafe fn transmute<'a, 'b, T: Sized + FromBytes>(
///
/// Callers must ensure that the region of memory returned is not read or written for as long as
/// the returned reference is alive.
-unsafe fn transmute_mut<'a, 'b, T: Sized + FromBytes>(
- fw: &'a mut DmaObject,
+unsafe fn transmute_mut<T: Sized + FromBytes + AsBytes>(
+ fw: &mut DmaObject,
offset: usize,
-) -> Result<&'b mut T> {
- if offset + size_of::<T>() > fw.size() {
- return Err(EINVAL);
- }
- if (fw.start_ptr_mut() as usize + offset) % align_of::<T>() != 0 {
- return Err(EINVAL);
- }
-
- // SAFETY: we have checked that the pointer is properly aligned that its pointed memory is
- // large enough the contains an instance of `T`, which implements `FromBytes`.
- Ok(unsafe { &mut *(fw.start_ptr_mut().add(offset).cast::<T>()) })
+) -> Result<&mut T> {
+ // SAFETY: Guaranteed by the safety requirements of the function.
+ T::from_bytes_mut(unsafe { fw.as_slice_mut(offset, size_of::<T>())? }).ok_or(EINVAL)
}
/// The FWSEC microcode, extracted from the BIOS and to be run on the GSP falcon.
@@ -260,32 +245,38 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re
// Find the DMEM mapper section in the firmware.
for i in 0..hdr.entry_count as usize {
- let app: &FalconAppifV1 =
// SAFETY: we have exclusive access to `dma_object`.
- unsafe {
+ let app: &FalconAppifV1 = unsafe {
transmute(
&dma_object,
- hdr_offset + hdr.header_size as usize + i * hdr.entry_size as usize
+ hdr_offset + hdr.header_size as usize + i * hdr.entry_size as usize,
)
}?;
if app.id != NVFW_FALCON_APPIF_ID_DMEMMAPPER {
continue;
}
+ let dmem_base = app.dmem_base;
// SAFETY: we have exclusive access to `dma_object`.
let dmem_mapper: &mut FalconAppifDmemmapperV3 = unsafe {
- transmute_mut(
- &mut dma_object,
- (desc.imem_load_size + app.dmem_base) as usize,
- )
+ transmute_mut(&mut dma_object, (desc.imem_load_size + dmem_base) as usize)
}?;
+ dmem_mapper.init_cmd = match cmd {
+ FwsecCommand::Frts {
+ frts_addr: _,
+ frts_size: _,
+ } => NVFW_FALCON_APPIF_DMEMMAPPER_CMD_FRTS,
+ FwsecCommand::Sb => NVFW_FALCON_APPIF_DMEMMAPPER_CMD_SB,
+ };
+ let cmd_in_buffer_offset = dmem_mapper.cmd_in_buffer_offset;
+
// SAFETY: we have exclusive access to `dma_object`.
let frts_cmd: &mut FrtsCmd = unsafe {
transmute_mut(
&mut dma_object,
- (desc.imem_load_size + dmem_mapper.cmd_in_buffer_offset) as usize,
+ (desc.imem_load_size + cmd_in_buffer_offset) as usize,
)
}?;
@@ -296,24 +287,19 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re
size: 0,
flags: 2,
};
-
- dmem_mapper.init_cmd = match cmd {
- FwsecCommand::Frts {
- frts_addr,
- frts_size,
- } => {
- frts_cmd.frts_region = FrtsRegion {
- ver: 1,
- hdr: size_of::<FrtsRegion>() as u32,
- addr: (frts_addr >> 12) as u32,
- size: (frts_size >> 12) as u32,
- ftype: NVFW_FRTS_CMD_REGION_TYPE_FB,
- };
-
- NVFW_FALCON_APPIF_DMEMMAPPER_CMD_FRTS
- }
- FwsecCommand::Sb => NVFW_FALCON_APPIF_DMEMMAPPER_CMD_SB,
- };
+ if let FwsecCommand::Frts {
+ frts_addr,
+ frts_size,
+ } = cmd
+ {
+ frts_cmd.frts_region = FrtsRegion {
+ ver: 1,
+ hdr: size_of::<FrtsRegion>() as u32,
+ addr: (frts_addr >> 12) as u32,
+ size: (frts_size >> 12) as u32,
+ ftype: NVFW_FRTS_CMD_REGION_TYPE_FB,
+ };
+ }
// Return early as we found and patched the DMEMMAPPER region.
return Ok(Self(dma_object, PhantomData));
--
2.51.0
Powered by blists - more mailing lists