linux-kernel - Re: [PATCH] KVM: x86: switch hugepage recovery thread to vhost

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: <Z2Ss3ALr5QgHo4UP@kbusch-mbp.dhcp.thefacebook.com>
Date: Thu, 19 Dec 2024 16:31:40 -0700
From: Keith Busch <kbusch@...nel.org>
To: Paolo Bonzini <pbonzini@...hat.com>
Cc: linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
	michael.christie@...cle.com, Tejun Heo <tj@...nel.org>,
	Luca Boccassi <bluca@...ian.org>, Jens Axboe <axboe@...com>,
	axboe@...nel.dk
Subject: Re: [PATCH] KVM: x86: switch hugepage recovery thread to vhost_task

On Thu, Dec 19, 2024 at 11:57:40PM +0100, Paolo Bonzini wrote:
> It could be as simple as this on the kernel side: [adding Jens for
> a first look]

Cc'ing his @kernel email; first message:

	https://lore.kernel.org/linux-kernel/20241108130737.126567-1-pbonzini@redhat.com/
 
> =============== 8< ===========
> From: Paolo Bonzini <pbonzini@...hat.com>
> Subject: [PATCH] fs: proc: mark user and I/O workers as "kernel threads"
> 
> A Rust library called "minijail" is looking at procfs to check if
> the current task has multiple threads, and to prevent fork() if it
> does.  This is because fork() is in general ill-advised in
> multi-threaded programs, for example if another thread might have
> taken locks.
> 
> However, this attempt falls afoul of kernel threads that are children
> of the user process that they serve.  These are not a problem when
> forking, but they are still present in procfs.  The library should
> discard them, but there is currently no way for userspace to detect
> PF_USER_WORKER or PF_IO_WORKER threads.
> 
> The closest is the "Kthread" key in /proc/PID/task/TID/status.  Extend
> it instead of introducing another keyl tasks that are marked with
> PF_USER_WORKER or PF_IO_WORKER are not kthreads, but they are close
> enough for basically all intents and purposes.

Yes, this looks good to me. But I also would have thought the original
patch was safe too :). Hopefully nothing depends on the current value
for these... there is often something making it difficult to have nice
things.

> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> 
> diff --git a/fs/proc/array.c b/fs/proc/array.c
> index 34a47fb0c57f..f702fb50c8ef 100644
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -221,7 +221,7 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns,
>  #endif
>  	seq_putc(m, '\n');
> -	seq_printf(m, "Kthread:\t%c\n", p->flags & PF_KTHREAD ? '1' : '0');
> +	seq_printf(m, "Kthread:\t%c\n", p->flags & (PF_KTHREAD | PF_USER_WORKER | PF_IO_WORKER) ? '1' : '0');
>  }
>  void render_sigset_t(struct seq_file *m, const char *header,UBLK_U_IO_REGISTER_ZC_RING