linux-kernel - Re: [BUG] TASK_DEAD task is able to be woken up in special condition

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: <CAHGf_=riHJSSsf7Q-G9X3L0etV4n+x4LxdXXGCOuXxn3ih6Nww@mail.gmail.com>
Date:	Thu, 22 Dec 2011 15:02:39 -0500
From:	KOSAKI Motohiro <kosaki.motohiro@...il.com>
To:	Yasunori Goto <y-goto@...fujitsu.com>
Cc:	Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...e.hu>,
	Hiroyuki KAMEZAWA <kamezawa.hiroyu@...fujitsu.com>,
	Linux Kernel ML <linux-kernel@...r.kernel.org>
Subject: Re: [BUG] TASK_DEAD task is able to be woken up in special condition

>> I doubt it is not only TASK_DEAD issue, it is rwsem fundamental issue.
>> Because of, a lot of place assume "current->state = newstate" is safe
>> and don't need any synchronization. So, I'm worry about to lost
>> TASK_UNINTERRUPTIBLE can make catastrophe like TASK_DEAD.
>
> I don't understand why this is catastrophe.
> I suppose it is just waken up from TASK_UNINTERRUPTIBLE
> by try_to_wake_up() in race condition. It seems to be normal situation.....

First, yes, try_to_wake_up() has a race. caller must care for spurious wakeup.
But, mutex, rwsem and other synchronization primitives must hide it and ensure
locking semantics. That's one of the reason why we use lock primitive instead
of bare try_to_wake_up(). I don't think "hey, all drivers code must
know scheduler
race and scheduler internal deeply" is practical solution.


> But TASK_DEAD status is special. It must not return to TASK_RUNNING state.

O.K. agree. but your patch seems just hacky. (1) I'm worry about why
out of scheduler
code know pi_lock and take it (2) all of other task->state changing
place don't take
pi_lock. so, your proposed locking semantics is not clear.

So, to remove TASK_DEAD and to add new member task->is_dead is alternative idea.



>> @@ -208,9 +208,9 @@ rwsem_down_failed_common(struct rw_semaphore *sem,
>>
>>          /* wait to be given the lock */
>>          for (;;) {
>> +               schedule();
>>                  if (!waiter.task)
>>                          break;
>> -               schedule();
>>                  set_task_state(tsk, TASK_UNINTERRUPTIBLE);
>>          }
>>
>
> Hmmmmmmm.
> Are you sure there is no route which TASK_DEAD task is waken up like rwsem?

Not sure. but all of them are a bug if exist, I think.
--
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/