linux-kernel - Re: [PATCH v4 2/4] rust: configfs: introduce rust support for configfs

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] [day] [month] [year] [list]

Message-Id: <FD609CD0-D3A1-47D5-8D5C-E8DB57003932@collabora.com>
Date: Tue, 25 Feb 2025 10:01:55 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Andreas Hindborg <a.hindborg@...nel.org>
Cc: Danilo Krummrich <dakr@...nel.org>,
 Miguel Ojeda <ojeda@...nel.org>,
 Alex Gaynor <alex.gaynor@...il.com>,
 Boqun Feng <boqun.feng@...il.com>,
 Gary Guo <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>,
 Benno Lossin <benno.lossin@...ton.me>,
 Alice Ryhl <aliceryhl@...gle.com>,
 Trevor Gross <tmgross@...ch.edu>,
 Joel Becker <jlbec@...lplan.org>,
 Christoph Hellwig <hch@....de>,
 Peter Zijlstra <peterz@...radead.org>,
 Ingo Molnar <mingo@...hat.com>,
 Will Deacon <will@...nel.org>,
 Waiman Long <longman@...hat.com>,
 Fiona Behrens <me@...enk.dev>,
 Charalampos Mitrodimas <charmitro@...teo.net>,
 rust-for-linux@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/4] rust: configfs: introduce rust support for
 configfs


> 
>> 
>>> 
>>> [...]
>>> 
>>>>> +
>>>>> +/// # Safety
>>>>> +///
>>>>> +/// `this` must be a valid pointer.
>>>>> +///
>>>>> +/// If `this` does not represent the root group of a `configfs` subsystem,
>>>>> +/// `this` must be a pointer to a `bindings::config_group` embedded in a
>>>>> +/// `Group<Parent>`.
>>>>> +///
>>>>> +/// Otherwise, `this` must be a pointer to a `bindings::config_group` that
>>>>> +/// is embedded in a `bindings::configfs_subsystem` that is embedded in a
>>>>> +/// `Subsystem<Parent>`.
>>>>> +unsafe fn get_group_data<'a, Parent>(this: *mut bindings::config_group) -> &'a Parent {
>>>>> +    // SAFETY: `this` is a valid pointer.
>>>>> +    let is_root = unsafe { (*this).cg_subsys.is_null() };
>>>>> +
>>>>> +    if !is_root {
>>>>> +        // SAFETY: By C API contact, `this` is a pointer to a
>>>>> +        // `bindings::config_group` that we passed as a return value in from
>>>>> +        // `make_group`. Such a pointer is embedded within a `Group<Parent>`.
>>>> 
>>>> This phrase is confusing.
>>> 
>>> I am not sure how to rephrase it to be less confusing. The pointer is
>>> the thing returned from `make_group`. `make_group` would only return a
>>> pointer into a `Group<Parent>`.
>> 
>> The problem is this: "that we passed as a return value in from”, to pass something as a return value
>> is already hard to parse, and when you reach the “in from” part, it becomes even harder.
>> 
>> Just say a variation of what you said above, that is perfectly understandable.
>> 
>> What about:
>> 
>> ```
>> 
>> `this` is a pointer to a `bindings::config_group` that was returned from a call to `make_group`. The pointer is known
>> to be embedded within a `Group<Parent>`.
>> 
>> ```
> 
> How is this:
> 
>        // SAFETY: By C API contact,`this` was returned from a call to
>        // `make_group`. The pointer is known to be embedded within a
>        // `Group<Parent>`.
> 

This is good.

— Daniel