linux-kernel - Re: single-threaded wq lockdep is broken

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]

Date:   Fri, 2 Jun 2017 15:03:58 +0800
From:   Lai Jiangshan <jiangshanlai@...il.com>
To:     Johannes Berg <johannes@...solutions.net>
Cc:     Tejun Heo <tj@...nel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: single-threaded wq lockdep is broken

On Wed, May 31, 2017 at 4:36 PM, Johannes Berg
<johannes@...solutions.net> wrote:
> Hi,
>
>> > #include <linux/kernel.h>
>> > #include <linux/mutex.h>
>> > #include <linux/workqueue.h>
>> > #include <linux/module.h>
>> > #include <linux/delay.h>
>> >
>> > DEFINE_MUTEX(mtx);
>> > static struct workqueue_struct *wq;
>> > static struct work_struct w1, w2;
>> >
>> > static void w1_wk(struct work_struct *w)
>> > {
>> >         mutex_lock(&mtx);
>> >         msleep(100);
>> >         mutex_unlock(&mtx);
>> > }
>> >
>> > static void w2_wk(struct work_struct *w)
>> > {
>> > }
>> >
>> > /*
>> >  * if not defined, then lockdep should warn only,
>>
>> I guess when DEADLOCK not defined, there is no
>> work is queued nor executed, therefore, no lock
>> dependence is recorded, and there is no warn
>> either.
>>
>> >  * if defined, the system will really deadlock.
>> >  */
>> >
>> > //#define DEADLOCK
>> >
>> > static int init(void)
>> > {
>> >         wq = create_singlethread_workqueue("test");
>> >         if (!wq)
>> >                 return -ENOMEM;
>> >         INIT_WORK(&w1, w1_wk);
>> >         INIT_WORK(&w2, w2_wk);
>> >
>>
>>         /* add lock dependence, the lockdep should warn */
>>         queue_work(wq, &w1);
>>         queue_work(wq, &w2);
>>         flush_work(&w1);
>>
>> > #ifdef DEADLOCK
>> >         queue_work(wq, &w1);
>> >         queue_work(wq, &w2);
>> > #endif
>> >         mutex_lock(&mtx);
>> >         flush_work(&w2);
>> >         mutex_unlock(&mtx);
>> >
>> > #ifndef DEADLOCK
>> >         queue_work(wq, &w1);
>> >         queue_work(wq, &w2);
>> > #endif
>
> This was "ifndef", so it does in fact run here, just like you
> suggested. It doesn't warn though.
>
> I don't think the order of queue/flush would matter, in fact, if you
> insert it like you did, with the flush outside the mutex, no issue
> exists (until the later flush)
>

the @w2 is not queued before flush_work(&w2), it is expected
that @w2 is not associated with @wq, and the dependence
mtx -> wq will not be recorded. And it is expected no warning.

> Also, even if DEADLOCK *is* defined, lockdep doesn't report anything.

Uhhh..... I have no idea about it yet.