Re: [PATCH v3 19/27] rust: pci: make Bar lifetime-parameterized

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `name` pointer passed to `pci_request_region` is stored directly in the kernel's resource tree, causing a Use-After-Free if a dynamically allocated string is passed.
--

commit 288028203fe9a624774ed430783fe427c724785b
Author: Danilo Krummrich <dakr@kernel.org>

rust: pci: make Bar lifetime-parameterized

Convert pci::Bar to be lifetime-parameterized over the device bound.
iomap_region_sized now returns Result<Bar<'bound, SIZE>> directly. Add
Bar::into_devres() to register the bar as a device-managed resource.

> diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs
> index ae78676c927ff..5668394a155bd 100644
> --- a/rust/kernel/pci/io.rs
> +++ b/rust/kernel/pci/io.rs

[ ... ]

> @@ -146,14 +145,14 @@ impl<'a, S: ConfigSpaceKind> IoKnownSize for ConfigSpace<'a, S> {
>  ///
>  /// `Bar` always holds an `IoRaw` instance that holds a valid pointer to the start of the I/O
>  /// memory mapped PCI BAR and its size.
> -pub struct Bar<const SIZE: usize = 0> {
> -    pdev: ARef<Device>,
> +pub struct Bar<'bound, const SIZE: usize = 0> {
> +    pdev: &'bound Device<device::Bound>,
>      io: MmioRaw<SIZE>,
>      num: i32,
>  }
>  
> -impl<const SIZE: usize> Bar<SIZE> {
> -    pub(super) fn new(pdev: &Device, num: u32, name: &CStr) -> Result<Self> {
> +impl<'bound, const SIZE: usize> Bar<'bound, SIZE> {
> +    pub(super) fn new(pdev: &'bound Device<device::Bound>, num: u32, name: &CStr) -> Result<Self> {

Does this name parameter need a 'static bound?

The kernel's pci_request_region() function (via __request_region_locked())
stores the raw string pointer directly into the struct resource tree
(res->name = name) without copying it.

Because the API does not enforce a 'static lifetime on name, a caller
could pass a dynamically allocated CString that is dropped immediately. If
this happens, the kernel resource tree will hold a dangling pointer,
resulting in a use-after-free when /proc/iomem or /proc/ioports are read.

While the previous signature (name: &'a CStr) also failed to guarantee the
string outlived the resource, should we enforce name: &'static CStr here
to prevent memory corruption?

[ ... ]

> @@ -249,20 +257,16 @@ fn deref(&self) -> &Self::Target {
>  impl Device<device::Bound> {
>      /// Maps an entire PCI BAR after performing a region-request on it. I/O operation bound checks
>      /// can be performed on compile time for offsets (plus the requested type size) < SIZE.
> -    pub fn iomap_region_sized<'a, const SIZE: usize>(
> -        &'a self,
> +    pub fn iomap_region_sized<'bound, const SIZE: usize>(
> +        &'bound self,
>          bar: u32,
> -        name: &'a CStr,
> -    ) -> impl PinInit<Devres<Bar<SIZE>>, Error> + 'a {
> -        Devres::new(self.as_ref(), Bar::<SIZE>::new(self, bar, name))
> +        name: &CStr,
> +    ) -> Result<Bar<'bound, SIZE>> {
> +        Bar::new(self, bar, name)
>      }
>  
>      /// Maps an entire PCI BAR after performing a region-request on it.
> -    pub fn iomap_region<'a>(
> -        &'a self,
> -        bar: u32,
> -        name: &'a CStr,
> -    ) -> impl PinInit<Devres<Bar>, Error> + 'a {
> +    pub fn iomap_region<'bound>(&'bound self, bar: u32, name: &CStr) -> Result<Bar<'bound>> {
>          self.iomap_region_sized::<0>(bar, name)
>      }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517000149.3226762-1-dakr@kernel.org?part=19