linux-kernel - Re: [PATCH v2 6/7] rust: file: add `DeferredFdCloser`

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]

Date:   Mon, 11 Dec 2023 09:41:28 -0800
From:   Boqun Feng <boqun.feng@...il.com>
To:     Alice Ryhl <aliceryhl@...gle.com>
Cc:     benno.lossin@...ton.me, a.hindborg@...sung.com,
        alex.gaynor@...il.com, arve@...roid.com, bjorn3_gh@...tonmail.com,
        brauner@...nel.org, 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 v2 6/7] rust: file: add `DeferredFdCloser`

On Mon, Dec 11, 2023 at 03:34:40PM +0000, Alice Ryhl wrote:
> Benno Lossin <benno.lossin@...ton.me> writes:
> > On 12/6/23 12:59, Alice Ryhl wrote:
> > > +    /// Schedule a task work that closes the file descriptor when this task returns to userspace.
> > > +    ///
> > > +    /// Fails if this is called from a context where we cannot run work when returning to
> > > +    /// userspace. (E.g., from a kthread.)
> > > +    pub fn close_fd(self, fd: u32) -> Result<(), DeferredFdCloseError> {
> > > +        use bindings::task_work_notify_mode_TWA_RESUME as TWA_RESUME;
> > > +
> > > +        // In this method, we schedule the task work before closing the file. This is because
> > > +        // scheduling a task work is fallible, and we need to know whether it will fail before we
> > > +        // attempt to close the file.
> > > +
> > > +        // SAFETY: Getting a pointer to current is always safe.
> > > +        let current = unsafe { bindings::get_current() };
> > > +
> > > +        // SAFETY: Accessing the `flags` field of `current` is always safe.
> > > +        let is_kthread = (unsafe { (*current).flags } & bindings::PF_KTHREAD) != 0;
> > 
> > Since Boqun brought to my attention that we already have a wrapper for
> > `get_current()`, how about you use it here as well?
> 
> I can use the wrapper, but it seems simpler to not go through a
> reference when we just need a raw pointer.
> 
> Perhaps we should have a safe `Task::current_raw` function that just
> returns a raw pointer? It can still be safe.
> 

I think we can have a `as_ptr` function for `Task`?

	impl Task {
	    pub fn as_ptr(&self) -> *mut bindings::task_struct {
	        self.0.get()
	    }
	}

Regards,
Boqun

[...]