linux-kernel - Re: [PATCH] kthread: Fix the race condition when kthread is parked

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]

Date:	Wed, 25 Jun 2014 22:03:16 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Subbaraman Narayanamurthy <subbaram@...eaurora.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kthread: Fix the race condition when kthread is parked

On Wed, Jun 25, 2014 at 10:00:22PM -0400, Steven Rostedt wrote:
> On Thu, Jun 26, 2014 at 02:43:56AM +0200, Thomas Gleixner wrote:
> > 
> > Subject: kthread: Plug park/ unplug race
> > From: Thomas Gleixner <tglx@...utronix.de>
> > Date: Thu, 26 Jun 2014 01:24:36 +0200
> > 
> > The kthread park/unpark logic has the following issue:
> > 
> > Task   CPU 0				CPU 1
> > 
> > T1     unplug cpu1
> >        kthread_park(T2)
> >        set_bit(KTHREAD_SHOULD_PARK);
> > 	  wait_for_completion()
> > T2					parkme(X)
> 
> But with your patch, isn't it possible for T1 to call thread_unpark here?

Let me answer that.... No, it can't.

I missed the wait_for_completion() above, which will prevent this from happening.

Nevermind, I'll go work on something less brain intensive.

-- Steve

> 
> Then looking at the code I see this turn of events:
> 
>   if (test_bit(KTHREAD_IS_PER_CPU, &kthread->flags))
>         __kthread_bind(k, kthread->cpu, TASK_PARKED);
> 
> Which in __kthread_bind() (state == TASK_PARKED)
> 
> 	if (!wait_task_inactive(p, state)) {
> 		WARN_ON(1);
> 		return;
> 	}
> 
> Where wait_task_inactive() does:
> 
> 		while (task_running(rq, p)) {
> 			if (match_state && unlikely(p->state != match_state))
> 				return 0;
> 
> As match_state is non zero and p->state != match_state because it hasn't been
> set yet. The wait_task_inactive() returns zero, and then we hit the WARN_ON()
> in __kthread_bind().
> 
> -- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/