OK, I won't argue, but ....
On 08/01, Christian Brauner wrote:
On Wed, Jul 31, 2024 at 04:51:33PM GMT, Oleg Nesterov wrote:
On 07/31, Christian Brauner wrote:
It's currently possible to create pidfds for kthreads but it is unclear what that is supposed to mean. Until we have use-cases for it and we figured out what behavior we want block the creation of pidfds for kthreads.
Hmm... could you explain your concerns? Why do you think we should disallow pidfd_open(pid-of-kthread) ?
It basically just works now and it's not intentional - at least not on my part. You can't send signals to them,
Yes, you can't send signals to kthread. So what?
You can't send signals to the normal processes if check_kill_permission() fails. And even if you are root, you can't send an unhandled signal via pidfd = pidfd_open(1).
you may or may not get notified via poll when a kthread exits.
Why? the exiting kthread should not differ in this respect?
(So imho this causes more confusion then it is actually helpful. If we add supports for kthreads I'd also like pidfs to gain a way to identify them via statx() or fdinfo.)
/proc/$pid/status has a "Kthread" field...
@@ -2403,6 +2416,12 @@ __latent_entropy struct task_struct *copy_process( if (clone_flags & CLONE_PIDFD) { int flags = (clone_flags & CLONE_THREAD) ? PIDFD_THREAD : 0;
/* Don't create pidfds for kernel threads for now. */if (args->kthread) {retval = -EINVAL;goto bad_fork_free_pid;Do we really need this check? Userspace can't use args->kthread != NULL, the kernel users should not use CLONE_PIDFD.
Yeah, I know. That's really just proactive so that user of e.g., copy_process() such as vhost or so on don't start handing out pidfds for stuff without requring changes to the helper itself.
Then I'd suggest WARN_ON_ONCE(args->kthread).
But as I said I won't argue. I see nothing wrong in this patch.
Oleg.