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: <aTbg6O2HS2fGAow6@redhat.com>
Date: Mon, 8 Dec 2025 15:30:00 +0100
From: Oleg Nesterov <oleg@...hat.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Christian Brauner <christian@...uner.io>,
	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 <lossin@...nel.org>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Trevor Gross <tmgross@...ch.edu>,
	Danilo Krummrich <dakr@...nel.org>,
	Panagiotis Foliadis <pfoliadis@...teo.net>,
	Shankari Anand <shankari.ak0208@...il.com>,
	FUJITA Tomonori <fujita.tomonori@...il.com>,
	Alexey Gladkov <legion@...nel.org>, rust-for-linux@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: rust: wrong SAFETY comments in group_leader() and pid() +
 questions

Alice,

Thanks again for your explanations. Not that I fully understand
them, though ;)

On 12/05, Alice Ryhl wrote:
>
> To start with, it's likely that this comment is not the right choice
> for this function, given our discussion. Most likely group_leader()
> needs to be moved to `impl CurrentTask {}`

I obviously can't comment this proposal,

> and the safety comment needs
> to explain why being the current task ensures that the returned &Task
> lives for long enough.

This is simple. task->group_leader can't change or go away until
this task exits or execs. The "current" task can't exit/exec.

(This also covers the race with mt-exec from current's subthread,
 the execing thread will kill all the threads and wait until they
 all pass release_task(). Only then it will change ->group_leader).

> impl CurrentTask {
>     fn group_leader(&self) -> &Task {
>         // SAFETY: This is the current task, so the task must be alive.
>         // Therefore the group leader cannot change, and thus it will
>         // stay valid as long as self is the current task.
>         unsafe { &*bindings::task_group_leader(self.as_ptr()).cast::<Task>() }
>     }
> }

Yes, the comment looks good to me.

But we don't have the task_group_leader() helper yet, so far I
only sent the trivial initial preparations, see
https://lore.kernel.org/all/aTV1KYdcDGvjXHos@redhat.com/

So if you are going to move Task::group_leader to the
CurrentTask block, please use .group_leader directly, like
the current code does.

Oleg.


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ