linux-kernel - Re: [PATCH] kthread: fix task state in kthread worker if being frozen

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: <20240819185807.ff5004ade2661c46740fc459@linux-foundation.org>
Date: Mon, 19 Aug 2024 18:58:07 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: Chen Yu <yu.c.chen@...el.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Andreas Gruenbacher
 <agruenba@...hat.com>, Tejun Heo <tj@...nel.org>, Shuah Khan
 <skhan@...uxfoundation.org>, "Mickael Salaun" <mic@...ikod.net>,
 linux-kernel@...r.kernel.org, Chen Yu <yu.chen.surf@...il.com>, kernel test
 robot <oliver.sang@...el.com>
Subject: Re: [PATCH] kthread: fix task state in kthread worker if being
 frozen

On Mon, 19 Aug 2024 22:15:51 +0800 Chen Yu <yu.c.chen@...el.com> wrote:

> It was reported that during cpu hotplug test, the following
> error was triggered:
> 
>  do not call blocking ops when !TASK_RUNNING; state=1 set at kthread_worker_fn (kernel/kthread.c:?)
>  WARNING: CPU: 1 PID: 674 at kernel/sched/core.c:8469 __might_sleep
> 
>  handle_bug
>  exc_invalid_op
>  asm_exc_invalid_op
>  __might_sleep
>  __might_sleep
>  kthread_worker_fn
>  kthread_worker_fn
>  kthread
>  __cfi_kthread_worker_fn
>  ret_from_fork
>  __cfi_kthread
>  ret_from_fork_asm
> 
> Peter pointed out that there is a race condition when the kworker is being
> frozen and falls into try_to_freeze() with TASK_INTERRUPTIBLE, which
> triggeres the warning.

OK.  A full description of this race would be better than simply
asserting that it exists, please.

> Fix this by explicitly set the TASK_RUNNING before entering try_to_freeze().

OK.

> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -848,6 +848,12 @@ int kthread_worker_fn(void *worker_ptr)
>  	} else if (!freezing(current))
>  		schedule();
>  
> +	/*
> +	 * Explicitly set the running state in case we are being
> +	 * frozen and skip the schedule() above. try_to_freeze()
> +	 * expects the current task to be in the running state.
> +	 */
> +	__set_current_state(TASK_RUNNING);
>  	try_to_freeze();
>  	cond_resched();
>  	goto repeat;

Comment is helpful, but why express in a comment that which can be
expressed in code?

--- a/kernel/kthread.c~kthread-fix-task-state-in-kthread-worker-if-being-frozen
+++ a/kernel/kthread.c
@@ -847,6 +847,12 @@ repeat:
 		trace_sched_kthread_work_execute_end(work, func);
 	} else if (!freezing(current))
 		schedule();
+	} else {
+		/*
+		 * Handle the case where X happens
+		 */
+		__set_current_state(TASK_RUNNING);
+	}
 
 	try_to_freeze();
 	cond_resched();
_