linux-kernel - Re: [PATCH 00/14] Fix bug about invalid wake up problem

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130829141037.GB3985@htj.dyndns.org>
Date:	Thu, 29 Aug 2013 10:10:37 -0400
From:	Tejun Heo <tj@...nel.org>
To:	Libin <huawei.libin@...wei.com>
Cc:	akpm@...ux-foundation.org, viro@...iv.linux.org.uk,
	eparis@...hat.com, tglx@...utronix.de, rusty@...tcorp.com.au,
	ebiederm@...ssion.com, paulmck@...ux.vnet.ibm.com,
	john.stultz@...aro.org, mingo@...hat.com, peterz@...radead.org,
	gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
	lizefan@...wei.com, jovi.zhangwei@...wei.com, guohanjun@...wei.com,
	zhangdianfang@...wei.com, wangyijing@...wei.com
Subject: Re: [PATCH 00/14] Fix bug about invalid wake up problem

Hello, Libin.

I'm completely confused by this series....

On Thu, Aug 29, 2013 at 09:57:35PM +0800, Libin wrote:
> 1)Problem Description:
> The prototype of invalid wakeup problem is as follows:
> ========================================================================
> ----------------------------
> Consumer Thread
> ----------------------------
> ...
> if (list_empty(&list)){
> 	//location a 
> 	set_current_state(TASK_INTERRUPTIBLE);
> 	schedule();
> }
> ...
> ----------------------------
> Producer Thread
> ----------------------------
> ...
> list_add_tail(&item, &list);
> wake_up_process(A);
> ...

This is of course broken.  set_current_state() should of course come
*before* the conditions is checked.  This is just plain broken code.

> In most cases, the kernel codes use a form similar to the following:
> --------------------------------------------
>  ...
>  //location a
>  ...
>  set_current_state(TASK_INTERRUPTIBLE);
>  //location b
>  while (list_empty(&product_list)){
> 	schedule();	//location c
> 	set_current_state(TASK_INTERRUPTIBLE);
> 	//location d
> }
> __set_current_state(TASK_RUNNING);
> ...
> --------------------------------------------
> At location a, consumer is preempted, and producer is scheduled,
> adding item to product_list and waking up consumer. After consumer is
> re-scheduled, calling set_current_state to set itself as state
> TASK_INTERRUPTIBLE, if it is preempted again before __set_current_state(TASK_RUNNING),
> also trigger the invalid wakeup problem that the consumer will not be scheduled

Why?  Getting preempted != calling schedule().  A preempted task will
be rescheduled *regardless* of ifs current->state; otherwise, the
whole kernel is severely broken.  Tasks never get deactivated while
preempted.

> and the item be added by producer can't be consumed.
> 
> The following circumstance will also trigger this issue:
> At location c, consumer is scheduled out, and be preempted after calling
> set_current_state(TASK_INTERRUPTIBLE) when it be re-schdeuled.

What?

> 2)Test:
> I have written a test module to trigger the problem by adding some
> synchronization condition. I will post it in the form of an attachment soon.
> 
> Test result as follows:
> [103135.332683] wakeup_test: create two kernel threads - producer & consumer
> [103135.332686] wakeup_test: module loaded successfully
> [103135.332692] wakeup_test: kthread producer try to wake up the kthread consumer
> [103165.299865] wakeup_test: kthread consumer have waited for 30s, indicating
> trigger an invalid wakeup problem!

The most interesting question here is "what were you testing?".  Can
you please post the test code?

For the whole series,

 Nacked-by: Tejun Heo <tj@...nel.org>

Thanks.

-- 
tejun
--
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/