Patch "rust: dma: use pointer projection infra for `dma_{read,write}` macro" has been added to the 6.19-stable tree
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 10 lines · Sasha Levin
This is a note to let you know that I've just added the patch titled
rust: dma: use pointer projection infra for
dma_{read,write}macroto the 6.19-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summaryThe filename of the patch is:
rust-dma-use-pointer-projection-infra-for-dma_-read-.patch
and it can be found in the queue-6.19 subdirectory.
Hi Sasha,
commit 08da98f18f4f ("rust: ptr: add KnownSize trait to support DST size info extraction") and commit f41941aab3ac ("rust: ptr: add projection infrastructure") are dependencies of this fix.
It doesn't look like these commits are currently being picked. They're needed for building.
They're part of the same series: https://lore.kernel.org/rust-for-linux/[email protected]/
Best,
Gary
Quoted 345 lines
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
commit
26dd613c37a79884da7fd824263d4c93123688c3Author: Gary Guo <[email protected]> Date: Mon Mar 2 16:42:36 2026 +0000rust: dma: use pointer projection infra for
dma_{read,write}macro[ Upstream commit
4da879a0d3fd170a70994b73baa554c6913918b5]Current
dma_read!,dma_write!macros also use a customaddr_of!()-based implementation for projecting pointers, which has soundness issue as it relies on absence ofDerefimplementation on types. It also has a soundness issue where it does not protect against unaligned fields (when#[repr(packed)]is used) so it can generate misaligned accesses.This commit migrates them to use the general pointer projection infrastructure, which handles these cases correctly.
As part of migration, the macro is updated to have an improved surface syntax. The current macro have
dma_read!(a.b.c[d].e.f)
to mean
a.b.cis a DMA coherent allocation and it should project into it with[d].e.fand do a read, which is confusing as it makes the indexing operator integral to the macro (so it will break if you have an array ofCoherentAllocation, for example).This also is problematic as we would like to generalize
CoherentAllocationfrom just slices to arbitrary types.Make the macro expects
dma_read!(path.to.dma, .path.inside.dma)as the canonical syntax. The index operator is no longer special and is just one type of projection (in additional to field projection). Similarly, makedma_write!(path.to.dma, .path.inside.dma, value)become the canonical syntax for writing.Another issue of the current macro is that it is always fallible. This makes sense with existing design of
CoherentAllocation, but once we support fixed size arrays withCoherentAllocation, it is desirable to have the ability to perform infallible indexing as well, e.g. doing a[0]index of[Foo; 2]is okay and can be checked at build-time, so forcing falliblity is non-ideal. To capture this, the macro is changed to use[idx]as infallible projection and[idx]?as fallible index projection (those syntax are part of the general projection infra). A benefit of this is that while individual indexing operation may fail, the overall read/write operation is not fallible.Fixes ad2907b4e308 ("rust: add dma coherent allocator abstraction")Reviewed-by Benno Lossin <[email protected]>Signed-off-by Gary Guo <[email protected]>[ Capitalize safety comments; slightly improve wording in doc-comments.
- Danilo ]Signed-off-by Danilo Krummrich <[email protected]>Signed-off-by Sasha Levin <[email protected]>drivers/gpu/nova-core/gsp.rs +7
-7@@ -143,14 +143,14 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error// _kgspInitLibosLoggingStructures (allocates memory for buffers) // kgspSetupLibosInitArgs_IMPL (creates pLibosInitArgs[] array) dma_write! ( - libos[0 ] =LibosMemoryRegionInitArgument ::new ("LOGINIT" , &loginit.0 ) - )?; + libos, [0 ]?,LibosMemoryRegionInitArgument ::new ("LOGINIT" , &loginit.0 ) + );dma_write! ( - libos[1 ] =LibosMemoryRegionInitArgument ::new ("LOGINTR" , &logintr.0 ) - )?; -dma_write! (libos[2 ] =LibosMemoryRegionInitArgument ::new ("LOGRM" , &logrm.0 ))?; -dma_write! (rmargs[0 ].inner = fw::GspArgumentsCached ::new (cmdq))?; -dma_write! (libos[3 ] =LibosMemoryRegionInitArgument ::new ("RMARGS" , rmargs))?; + libos, [1 ]?,LibosMemoryRegionInitArgument ::new ("LOGINTR" , &logintr.0 ) + ); +dma_write! (libos, [2 ]?,LibosMemoryRegionInitArgument ::new ("LOGRM" , &logrm.0 )); +dma_write! (rmargs, [0 ]?.inner, fw::GspArgumentsCached ::new (cmdq)); +dma_write! (libos, [3 ]?,LibosMemoryRegionInitArgument ::new ("RMARGS" , rmargs)); }, })) })drivers/gpu/nova-core/gsp/boot.rs +1
-1@@ -160,7 +160,7 @@ pub(crate) fn boot(let wpr_meta =CoherentAllocation ::<GspFwWprMeta >::alloc_coherent (dev,1 ,GFP_KERNEL | __GFP_ZERO)?; -dma_write! (wpr_meta[0 ] =GspFwWprMeta ::new (&gsp_fw, &fb_layout))?; +dma_write! (wpr_meta, [0 ]?,GspFwWprMeta ::new (&gsp_fw, &fb_layout)); self.cmdq .send_command (bar, commands::SetSystemInfo ::new (pdev))?;drivers/gpu/nova-core/gsp/cmdq.rs +7
-3@@ -201,9 +201,13 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {let gsp_mem =CoherentAllocation ::<GspMem >::alloc_coherent (dev,1 ,GFP_KERNEL | __GFP_ZERO)?; -dma_write! (gsp_mem[0 ].ptes =PteArray ::new (gsp_mem.dma_handle ())?)?; -dma_write! (gsp_mem[0 ].cpuq.tx =MsgqTxHeader ::new (MSGQ_SIZE ,RX_HDR_OFF ,MSGQ_NUM_PAGES ))?; -dma_write! (gsp_mem[0 ].cpuq.rx =MsgqRxHeader ::new ())?; +dma_write! (gsp_mem, [0 ]?.ptes,PteArray ::new (gsp_mem.dma_handle ())?); +dma_write! ( + gsp_mem, + [0 ]?.cpuq.tx, +MsgqTxHeader ::new (MSGQ_SIZE ,RX_HDR_OFF ,MSGQ_NUM_PAGES ) + ); +dma_write! (gsp_mem, [0 ]?.cpuq.rx,MsgqRxHeader ::new ());Ok (Self (gsp_mem)) }rust/kernel/dma.rs +50
-64@@ -444,6 +444,19 @@ pub fn size(&self) -> usize { self.count * core::mem::size_of ::<T >() } +/// Returns the raw pointer to the allocated region in the CPU's virtual address space. +#[inline] +pub fn as_ptr (&self) -> *const [T ] { + core::ptr::slice_from_raw_parts (self.cpu_addr.as_ptr (), self.count) + } + +/// Returns the raw pointer to the allocated region in the CPU's virtual address space as +/// a mutable pointer. +#[inline] +pub fn as_mut_ptr (&self) -> *mut [T ] { + core::ptr::slice_from_raw_parts_mut (self.cpu_addr.as_ptr (), self.count) + } +/// Returns the base address to the allocated region in the CPU's virtual address space. pub fn start_ptr (&self) -> *const T { self.cpu_addr.as_ptr ()@@ -564,23 +577,6 @@ pub unsafe fn write(&mut self, src: &[T], offset: usize) -> Result {Ok (()) } -/// Returns a pointer to an element from the region with bounds checking. `offset` is in -/// units of `T`, not the number of bytes. -/// -/// Public but hidden since it should only be used from [`dma_read`] and [`dma_write`] macros. -#[doc(hidden)] -pub fn item_from_index (&self, offset:usize ) ->Result <*mut T > { -if offset >= self.count { -return Err (EINVAL ); - } -// SAFETY: -// - The pointer is valid due to type invariant on `CoherentAllocation` -// and we've just checked that the range and index is within bounds. -// - `offset` can't overflow since it is smaller than `self.count` and we've checked -// that `self.count` won't overflow early in the constructor. -Ok (unsafe { self.cpu_addr.as_ptr ().add (offset) }) - } -/// Reads the value of `field` and ensures that its type is [`FromBytes`]. /// /// # Safety @@ -653,6 +649,9 @@ unsafe impl<T: AsBytes + FromBytes + Send> Send for CoherentAllocation<T> {}/// Reads a field of an item from an allocated region of structs. /// +/// The syntax is of the form `kernel::dma_read!(dma, proj)` where `dma` is an expression evaluating +/// to a [`CoherentAllocation`] and `proj` is a [projection specification](kernel::ptr::project!). +/// /// # Examples /// /// ``` @@ -667,36 +666,29 @@ unsafe impl<T: AsBytes + FromBytes + Send> Send for CoherentAllocation<T> {}/// unsafe impl kernel::transmute::AsBytes for MyStruct{}; /// /// # fn test(alloc: &kernel::dma::CoherentAllocation<MyStruct>) -> Result { -/// let whole = kernel::dma_read!(alloc[2]); -/// let field = kernel::dma_read!(alloc[1].field); +/// let whole = kernel::dma_read!(alloc, [2]?); +/// let field = kernel::dma_read!(alloc, [1]?.field); /// # Ok::<(), Error>(()) } /// ``` #[macro_export] macro_rules! dma_read { - ($dma:expr, $idx: expr, $($field:tt)*) => {{ - (|| -> ::core::result::Result <_ , $crate::error::Error > { -let item = $crate::dma::CoherentAllocation ::item_from_index (&$dma, $idx)?; -// SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be -// dereferenced. The compiler also further validates the expression on whether `field` -// is a member of `item` when expanded by the macro. -unsafe { -let ptr_field = ::core::ptr::addr_of!((*item) $($field)*); - ::core::result::Result ::Ok ( - $crate::dma::CoherentAllocation ::field_read (&$dma, ptr_field) - ) - } - })() + ($dma:expr, $($proj:tt)*) => {{ +let dma = &$dma; +let ptr = $crate::ptr::project!( + $crate ::dma::CoherentAllocation ::as_ptr (dma), $($proj)* + ); +// SAFETY: The pointer created by the projection is within the DMA region. +unsafe { $crate::dma::CoherentAllocation ::field_read (dma, ptr) } }}; - ($dma:ident [ $idx:expr ] $($field:tt)* ) => { - $crate::dma_read!($dma, $idx, $($field)*) - }; - ($($dma:ident).* [ $idx:expr ] $($field:tt)* ) => { - $crate::dma_read!($($dma).*, $idx, $($field)*) - }; }/// Writes to a field of an item from an allocated region of structs. /// +/// The syntax is of the form `kernel::dma_write!(dma, proj, val)` where `dma` is an expression +/// evaluating to a [`CoherentAllocation`], `proj` is a +/// [projection specification](kernel::ptr::project!), and `val` is the value to be written to the +/// projected location. +/// /// # Examples /// /// ``` @@ -711,37 +703,31 @@ macro_rules! dma_read {/// unsafe impl kernel::transmute::AsBytes for MyStruct{}; /// /// # fn test(alloc: &kernel::dma::CoherentAllocation<MyStruct>) -> Result { -/// kernel::dma_write!(alloc[2].member = 0xf); -/// kernel::dma_write!(alloc[1] = MyStruct { member: 0xf }); +/// kernel::dma_write!(alloc, [2]?.member, 0xf); +/// kernel::dma_write!(alloc, [1]?, MyStruct { member: 0xf }); /// # Ok::<(), Error>(()) } /// ``` #[macro_export] macro_rules! dma_write { - ($dma:ident [ $idx:expr ] $($field:tt)*) => {{ - $crate::dma_write!($dma, $idx, $($field)*) - }}; - ($($dma:ident).* [ $idx:expr ] $($field:tt)* ) => {{ - $crate::dma_write!($($dma).*, $idx, $($field)*) + (@parse [$dma:expr] [$($proj:tt)*] [, $val:expr]) => {{ +let dma = &$dma; +let ptr = $crate::ptr::project!( +mut $crate ::dma::CoherentAllocation ::as_mut_ptr (dma), $($proj)* + ); +let val = $val; +// SAFETY: The pointer created by the projection is within the DMA region. +unsafe { $crate::dma::CoherentAllocation ::field_write (dma, ptr, val) } }}; - ($dma:expr, $idx: expr, = $val:expr) => { - (|| -> ::core::result::Result <_ , $crate::error::Error > { -let item = $crate::dma::CoherentAllocation ::item_from_index (&$dma, $idx)?; -// SAFETY: `item_from_index` ensures that `item` is always a valid item. -unsafe { $crate::dma::CoherentAllocation ::field_write (&$dma, item, $val) } - ::core::result::Result ::Ok (()) - })() + (@parse [$dma:expr] [$($proj:tt)*] [.$field:tt $($rest:tt)*]) => { + $crate::dma_write!(@parse [$dma] [$($proj)* .$field] [$($rest)*]) + }; + (@parse [$dma:expr] [$($proj:tt)*] [[$index:expr]? $($rest:tt)*]) => { + $crate::dma_write!(@parse [$dma] [$($proj)* [$index]?] [$($rest)*]) + }; + (@parse [$dma:expr] [$($proj:tt)*] [[$index:expr] $($rest:tt)*]) => { + $crate::dma_write!(@parse [$dma] [$($proj)* [$index]] [$($rest)*]) }; - ($dma:expr, $idx: expr, $(.$field:ident)* = $val:expr) => { - (|| -> ::core::result::Result <_ , $crate::error::Error > { -let item = $crate::dma::CoherentAllocation ::item_from_index (&$dma, $idx)?; -// SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be -// dereferenced. The compiler also further validates the expression on whether `field` -// is a member of `item` when expanded by the macro. -unsafe { -let ptr_field = ::core::ptr::addr_of_mut!((*item) $(.$field)*); - $crate::dma::CoherentAllocation ::field_write (&$dma, ptr_field, $val) - } - ::core::result::Result ::Ok (()) - })() + ($dma:expr, $($rest:tt)*) => { + $crate::dma_write!(@parse [$dma] [] [$($rest)*]) }; }samples/rust/rust_dma.rs +16
-14@@ -68,7 +68,7 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, ECoherentAllocation ::alloc_coherent (pdev.as_ref (),TEST_VALUES .len (),GFP_KERNEL )?;for (i, value)in TEST_VALUES .into_iter ().enumerate () { - kernel::dma_write!(ca[i] =MyStruct ::new (value.0 , value.1 ))?; + kernel::dma_write!(ca, [i]?,MyStruct ::new (value.0 , value.1 )); }let size =4 * page::PAGE_SIZE ;@@ -85,6 +85,20 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E } } +impl DmaSampleDriver { +fn check_dma (&self) ->Result { +for (i, value)in TEST_VALUES .into_iter ().enumerate () { +let val0 = kernel::dma_read!(self.ca, [i]?.h); +let val1 = kernel::dma_read!(self.ca, [i]?.b); + +assert_eq! (val0, value.0 ); +assert_eq! (val1, value.1 ); + } + +Ok (()) + } +} +#[pinned_drop] impl PinnedDrop for DmaSampleDriver {fn drop (self:Pin <&mut Self >) {@@ -92,19 +106,7 @@ fn drop(self: Pin<&mut Self>) {dev_info! (dev,"Unload DMA test driver.\n" ); -for (i, value)in TEST_VALUES .into_iter ().enumerate () { -let val0 = kernel::dma_read!(self.ca[i].h); -let val1 = kernel::dma_read!(self.ca[i].b); -assert! (val0.is_ok ()); -assert! (val1.is_ok ()); - -if let Ok (val0) = val0 { -assert_eq! (val0, value.0 ); - } -if let Ok (val1) = val1 { -assert_eq! (val1, value.1 ); - } - } +assert! (self.check_dma ().is_ok ());for (i, entry)in self.sgt.iter ().enumerate () {dev_info! (dev,"Entry[{}]: DMA address: {:#x}" , i, entry.dma_address ());
Greg KH
maintainerQuoted 14 lines · Gary Guo
Quoted 10 lines · Sasha Levin
This is a note to let you know that I've just added the patch titled
rust: dma: use pointer projection infra for
dma_{read,write}macroto the 6.19-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summaryThe filename of the patch is:
rust-dma-use-pointer-projection-infra-for-dma_-read-.patch
and it can be found in the queue-6.19 subdirectory.Hi Sasha,
Quoted 8 lines
commit
08da98f18f4f("rust: ptr: addKnownSizetrait to support DST size info extraction") and commitf41941aab3ac("rust: ptr: add projection infrastructure") are dependencies of this fix.It doesn't look like these commits are currently being picked. They're needed for building.
They're part of the same series: https://lore.kernel.org/rust-for-linux/[email protected]/
Yeah, this breaks the build on my systems. I'll go drop this patch for now, and if you want these in the stable trees, can you provide a backported series of them?
thanks,
greg k-h
Quoted 19 lines · Greg KH
Quoted 18 lines · Gary Guo
Quoted 10 lines · Sasha Levin
This is a note to let you know that I've just added the patch titled
rust: dma: use pointer projection infra for
dma_{read,write}macroto the 6.19-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summaryThe filename of the patch is:
rust-dma-use-pointer-projection-infra-for-dma_-read-.patch
and it can be found in the queue-6.19 subdirectory.Hi Sasha,
commit
08da98f18f4f("rust: ptr: addKnownSizetrait to support DST size info extraction") and commitf41941aab3ac("rust: ptr: add projection infrastructure") are dependencies of this fix.
Quoted 8 lines
Quoted 4 lines
It doesn't look like these commits are currently being picked. They're needed for building.
They're part of the same series: https://lore.kernel.org/rust-for-linux/[email protected]/
Yeah, this breaks the build on my systems. I'll go drop this patch for now, and if you want these in the stable trees, can you provide a backported series of them?
The DMA soundness fix, as a potential bug, is probably not crucial to backport.
There are two nova-core fixes on top of it. One of them fixes a UB condition reading from and writing to a DMA buffer, the other one is a potential stack overflow.
nova-core is still work in progress, so I'm not too worried about this as far as stable trees are concerned.
I think we usually backport soundness fixes anyway, plus everything should apply without conflicts, so it probably doesn't hurt to pick them up regardless.
I can send a separate series if preferred.
Greg KH
maintainerQuoted 35 lines · Danilo Krummrich
Quoted 19 lines · Greg KH
Quoted 18 lines · Gary Guo
Quoted 10 lines · Sasha Levin
This is a note to let you know that I've just added the patch titled
rust: dma: use pointer projection infra for
dma_{read,write}macroto the 6.19-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summaryThe filename of the patch is:
rust-dma-use-pointer-projection-infra-for-dma_-read-.patch
and it can be found in the queue-6.19 subdirectory.Hi Sasha,
commit
08da98f18f4f("rust: ptr: addKnownSizetrait to support DST size info extraction") and commitf41941aab3ac("rust: ptr: add projection infrastructure") are dependencies of this fix.Quoted 8 lines
Quoted 4 lines
It doesn't look like these commits are currently being picked. They're needed for building.
They're part of the same series: https://lore.kernel.org/rust-for-linux/[email protected]/
Yeah, this breaks the build on my systems. I'll go drop this patch for now, and if you want these in the stable trees, can you provide a backported series of them?
The DMA soundness fix, as a potential bug, is probably not crucial to backport.
There are two nova-core fixes on top of it. One of them fixes a UB condition reading from and writing to a DMA buffer, the other one is a potential stack overflow.
Quoted 7 lines
nova-core is still work in progress, so I'm not too worried about this as far as stable trees are concerned.
I think we usually backport soundness fixes anyway, plus everything should apply without conflicts, so it probably doesn't hurt to pick them up regardless.
I can send a separate series if preferred.
I've dropped all the nova-core patches from the queue as well. If you think they are needed in 6.19.y, please send backports.
thanks,
greg k-h