linux-kernel - Re: [PATCH 5/7] rust: file: add `Kuid` wrapper

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: <20231130-neuordnung-matetee-7aefa024f80b@brauner>
Date:   Thu, 30 Nov 2023 11:52:59 +0100
From:   Christian Brauner <brauner@...nel.org>
To:     Alice Ryhl <aliceryhl@...gle.com>
Cc:     a.hindborg@...sung.com, alex.gaynor@...il.com, arve@...roid.com,
        benno.lossin@...ton.me, bjorn3_gh@...tonmail.com,
        boqun.feng@...il.com, cmllamas@...gle.com,
        dan.j.williams@...el.com, dxu@...uu.xyz, gary@...yguo.net,
        gregkh@...uxfoundation.org, joel@...lfernandes.org,
        keescook@...omium.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, maco@...roid.com, ojeda@...nel.org,
        peterz@...radead.org, rust-for-linux@...r.kernel.org,
        surenb@...gle.com, tglx@...utronix.de, tkjos@...roid.com,
        viro@...iv.linux.org.uk, wedsonaf@...il.com, willy@...radead.org
Subject: Re: [PATCH 5/7] rust: file: add `Kuid` wrapper

On Thu, Nov 30, 2023 at 09:36:03AM +0000, Alice Ryhl wrote:
> Christian Brauner <brauner@...nel.org> writes:
> > I'm a bit puzzled by all these rust_helper_*() calls. Can you explain
> > why they are needed? Because they are/can be static inlines and that
> > somehow doesn't work?
> 
> Yes, it's because the methods are inline. Rust can only call C methods
> that are actually exported by the C code.
> 
> >> +    /// Converts this kernel UID into a UID that userspace understands. Uses the namespace of the
> >> +    /// current task.
> >> +    pub fn into_uid_in_current_ns(self) -> bindings::uid_t {
> > 
> > Hm, I wouldn't special-case this. Just expose from_kuid() and let it
> > take a namespace argument, no? You don't need to provide bindings for
> > namespaces ofc.
> 
> To make `from_kuid` safe, I would need to wrap the namespace type too. I
> could do that, but it would be more code than this method because I need
> another wrapper struct and so on.
> 
> Personally I would prefer to special-case it until someone needs the
> non-special-case. Then, they can delete this method when they introduce
> the non-special-case.
> 
> But I'll do it if you think I should.

No, don't start wrapping namespaces as well. You already do parts of LSM
as well.

> 
> >> +impl PartialEq for Kuid {
> >> +    fn eq(&self, other: &Kuid) -> bool {
> >> +        // SAFETY: Just an FFI call.
> >> +        unsafe { bindings::uid_eq(self.kuid, other.kuid) }
> >> +    }
> >> +}
> >> +
> >> +impl Eq for Kuid {}
> > 
> > Do you need that?
> 
> Yes. This is the code that tells the compiler what `==` means for the
> `Kuid` type. Binder uses it here:

Ok, thanks.