| 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: <DDH1DE35H7L0.1Z2R655P701HR@kernel.org>
Date: Mon, 13 Oct 2025 10:03:19 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Markus Probst" <markus.probst@...teo.de>, "Danilo Krummrich"
<dakr@...nel.org>, "Miguel Ojeda" <ojeda@...nel.org>, "Alex Gaynor"
<alex.gaynor@...il.com>, "Lee Jones" <lee@...nel.org>, "Pavel Machek"
<pavel@...nel.org>
Cc: "Lorenzo Stoakes" <lorenzo.stoakes@...cle.com>, "Vlastimil Babka"
<vbabka@...e.cz>, "Liam R. Howlett" <Liam.Howlett@...cle.com>, "Uladzislau
Rezki" <urezki@...il.com>, "Boqun Feng" <boqun.feng@...il.com>, "Gary Guo"
<gary@...yguo.net>, <bjorn3_gh@...tonmail.com>, "Andreas Hindborg"
<a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>, "Trevor
Gross" <tmgross@...ch.edu>, <rust-for-linux@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-leds@...r.kernel.org>
Subject: Re: [PATCH v4 1/2] rust: add basic Pin<Vec<T, A>> abstractions
On Mon Oct 13, 2025 at 12:11 AM CEST, Markus Probst wrote:
> On Sun, 2025-10-12 at 23:31 +0200, Benno Lossin wrote:
>> On Sun Oct 12, 2025 at 6:57 PM CEST, Markus Probst wrote:
>> > From what I can tell, there is no way to get a `Pin<&mut Vec<T,
>> > A>>`
>> > from a `&mut Pin<Vec<T, A>>`. We can only get `Pin<&mut [T]>` which
>> > is
>> > not usable in our case.
>>
>> Hmm yeah that's true.
>>
>> > If there is way, without the extension trait or an extra struct, I
>> > would be happy to implement it.
>>
>> So I tried to look for the usage site of this and I found this usage
>> in
>> your v1:
>>
>> + let mut leds = KPinnedVec::with_capacity(
>> + Atmega1608LedAddress::VALUES.len() *
>> Atmega1608LedId::VALUES.len(),
>> + GFP_KERNEL,
>> + )?;
>> +
>> + let mut i = 0;
>> + for addr in Atmega1608LedAddress::VALUES {
>> + let mode_lock = Arc::pin_init(new_mutex!(()),
>> GFP_KERNEL)?;
>> +
>> + for id in Atmega1608LedId::VALUES {
>> + let Some(child) =
>> +
>> fwnode.get_child_by_name(&CString::try_from_fmt(fmt!("led@{i}"))?)
>> + else {
>> + continue;
>> + };
>> +
>> + let client = ARef::clone(&client);
>> + let mode_lock = Arc::clone(&mode_lock);
>> +
>> + leds.push_pin_init(LedClassDev::new(
>> + Some(idev),
>> + None,
>> + LedInitData::new().fwnode(&child),
>> + Atmega1608Led {
>> + addr,
>> + id,
>> + client,
>> +
>> + mode_lock,
>> + },
>> + ))?;
>> + i += 1;
>> + }
>> + }
>> + Ok(KBox::new(Self { client, leds }, GFP_KERNEL)?.into())
>>
>> And I think using `Vec` for this is just wrong. `Vec` is a data
>> structure that supports growing and shrinking the allocation. But you
>> just need a fixed size buffer that holds all your data. Do you think
>> that `Pin<Box<[LedClassDev]>>` would suffice if it had proper support
>> from pin-init?
> As you can see in v1, the number of leds (or vec entries) depends on
> the fwnode (see the continue statement there). I don't think that
> counts as fixed size. `Pin<KBox<[Option<LedClassDev>]>>` could
> potentially be used instead of `Pin<KVec<LedClassDev>>` in my scenario,
> but that would require an extra byte of allocation for the max leds of
> 24 each and the code would look more ugly. At the point I use Option in
> the slice, its basically an unoptimized Vec (instead of storing the
> length, it stores if an item in the buffer is present or not).
You can just make the length of the slice be the desired length? (also,
`i` is never incremented in the `continue` case, so it will act like a
`break`?)
One option that we have would be storing the initializers in a vec:
fn probe(
pdev: &I2cClient<kernel::device::Core>,
_id_info: Option<&Self::IdInfo>,
) -> Result<Pin<KBox<Self>>> {
let idev = pdev.as_ref();
let Some(fwnode) = idev.fwnode() else {
return Err(EINVAL);
};
let client: ARef<I2cClient> = pdev.into();
client
.write_byte_data(1, 0)
.inspect_err(|err| dev_err!(idev, "unable to remove led mask: {err:?}\n"))?;
let mut led_init = KVec::new();
let mut i = 0;
for addr in Atmega1608LedAddress::VALUES {
let mode_lock = Arc::pin_init(new_mutex!(()), GFP_KERNEL)?;
for id in Atmega1608LedId::VALUES {
let Some(child) =
fwnode.get_child_by_name(&CString::try_from_fmt(fmt!("led@{i}"))?)
else {
continue;
};
let client = ARef::clone(&client);
let mode_lock = Arc::clone(&mode_lock);
led_init.push(LedClassDev::new(
Some(idev),
None,
LedInitData::new().fwnode(&child),
Atmega1608Led {
addr,
id,
client,
mode_lock,
},
))?;
i += 1;
}
}
let leds = Vec::pin_init_slice(led_init, GFP_KERNEL)?;
Ok(KBox::new(Self { client, leds }, GFP_KERNEL)?.into())
}
And `Vec::pin_init_slice` would have the following signature:
fn pin_init_slice<T, I, E>(this: Vec<I>, flags: alloc::Flags) -> Result<Pin<Box<[T]>>>
where
I: PinInit<T, E>,
Error: From<E>;
---
Cheers,
Benno
>
>>
>> Also, please don't top-post [1] and take a look at your mail client
>> configuration, it puts lots of extra `> ` at the end which looks
>> pretty
>> strange [2].
> Yes, I did notice that. It is not present when writing a reply, but
> after it got sent for some reason (most replies, not all). It is GNOME
> Evolution in its default settings basically. My distro ships a 4 months
> outdated version (3.56.2), which shouldn't be too old, but I will
> investiage.
>
> Thanks
> - Markus Probst
>>
>> [1]:
>> https://docs.kernel.org/process/submitting-patches.html#use-trimmed-interleaved-replies-in-email-discussions
>> [2]:
>> https://lore.kernel.org/all/e550b0862e9ea87e50688d1ec8f623638d170a3a.camel@posteo.de
>>
>> ---
>> Cheers,
>> Benno
Powered by blists - more mailing lists