Re: [PATCH 10/10] exit/kthread: Move the exit code for kernel threads into struct kthread

Al Viro
2022-01-07T03:22:18Z

On Wed, Dec 08, 2021 at 02:25:32PM -0600, Eric W. Biederman wrote:
> The exit code of kernel threads has different semantics than the
> exit_code of userspace tasks. To avoid confusion and allow
> the userspace implementation to change as needed move
> the kernel thread exit code into struct kthread.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
> kernel/kthread.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/kthread.c b/kernel/kthread.c
> index 8e5f44bed027..9c6c532047c4 100644
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -52,6 +52,7 @@ struct kthread_create_info
> struct kthread {
> unsigned long flags;
> unsigned int cpu;
> + int result;
> int (*threadfn)(void *);
> void *data;
> mm_segment_t oldfs;
> @@ -287,7 +288,9 @@ EXPORT_SYMBOL_GPL(kthread_parkme);
> */
> void __noreturn kthread_exit(long result)
> {
> - do_exit(result);
> + struct kthread *kthread = to_kthread(current);
> + kthread->result = result;
> + do_exit(0);
> }
>
> /**
> @@ -679,7 +682,7 @@ int kthread_stop(struct task_struct *k)
> kthread_unpark(k);
> wake_up_process(k);
> wait_for_completion(&kthread->exited);
> - ret = k->exit_code;
> + ret = kthread->result;
> put_task_struct(k);
>
> trace_sched_kthread_stop_ret(ret);

Fine, except that you've turned the first two do_exit() in kthread() into
calls of kthread_exit(). If they are hit, you are screwed, especially
the second one - there you have an allocation failure for struct kthread,
so this will instantly oops on attempt to store into ->result.

See reply to your 6/10 regarding the difference between the last
call of do_exit() in kthread() and the first two of them. They
(the first two) should be simply do_exit(0); transmission of error
value happens differently and not in direction of kthread_stop().