linux-kernel - Re: [PATCH 2/2] rust: add dma coherent allocator abstraction.

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ee45ae5f-133d-4d38-bb4a-d3515790feb4@gmail.com>
Date: Thu, 31 Oct 2024 11:45:40 +0200
From: Abdiel Janulgue <abdiel.janulgue@...il.com>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: rust-for-linux@...r.kernel.org, a.hindborg@...nel.org,
 linux-kernel@...r.kernel.org, dakr@...hat.com, airlied@...hat.com,
 miguel.ojeda.sandonis@...il.com, wedsonaf@...il.com,
 Andreas Hindborg <a.hindborg@...sung.com>
Subject: Re: [PATCH 2/2] rust: add dma coherent allocator abstraction.

Hi,

On 28/10/2024 17:38, Daniel Almeida wrote:
> 
> This patch is not a v2, so was anybody against using a raw pointer at some time?

The original idea behind this was to improve a little bit from raw 
pointer manipulation when indexing the data. But yeah, for the proper v2 
I now reintroduced the raw pointer but dynamically created slice from it 
as you suggested below. :)

> 
> Not sure why there’s ’static here. The lifetime of `cpu_addr` is the lifetime of the object.
> 
> This is why keeping a pointer and building the slice as needed is actually a better approach, IMHO.
> That will correctly express the lifetime we want to enforce, i.e.:
> 
> ```
> pub fn cpu(&’a self) -> &’a mut [T];
> ```
> 
> Where ‘a is automatically filled in, of course.
> 

>> +    /// # Examples
>> +    ///
>> +    /// ```
>> +    /// use kernel::device::Device;
>> +    /// use kernel::dma::CoherentAllocation;
>> +    ///
>> +    /// # fn dox(dev: &Device) -> Result<()> {
>> +    /// let c: CoherentAllocation<u64> = CoherentAllocation::alloc_coherent(dev, 4, GFP_KERNEL)?;
> 
> Have you considered ZSTs? What happens if someone writes down:
> 
> ```
> let c = CoherentAllocation<()> = …
> ```
> 
> This doesn’t really make sense and should be forbidden.

Would restricting it to a primitive type make sense?

e.g:

pub struct CoherentAllocation<T: BitAnd + BitOr>

I'm not sure, but are there other ways to enforce that restriction?

> 
>> 2.43.0
>>
>>
> 
> Everything else looks good to me!

Thanks,
Abdiel

> 
> — Daniel
>