linux-kernel - Re: next: WARNING: CPU: 0 PID: 63 at kernel/workqueue.c:1999 worker_enter

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: <ZGvap5qi0OKAOfqX@slm.duckdns.org>
Date:   Mon, 22 May 2023 11:12:07 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Z qiang <qiang.zhang1211@...il.com>
Cc:     Naresh Kamboju <naresh.kamboju@...aro.org>,
        open list <linux-kernel@...r.kernel.org>,
        Linux-Next Mailing List <linux-next@...r.kernel.org>,
        lkft-triage@...ts.linaro.org,
        clang-built-linux <llvm@...ts.linux.dev>,
        Nathan Chancellor <nathan@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Dan Carpenter <dan.carpenter@...aro.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Anders Roxell <anders.roxell@...aro.org>
Subject: Re: next: WARNING: CPU: 0 PID: 63 at kernel/workqueue.c:1999
 worker_enter_idle+0xb2/0xc0

On Mon, May 22, 2023 at 08:20:38AM -1000, Tejun Heo wrote:
> Hello,
> 
> On Mon, May 22, 2023 at 09:24:09PM +0800, Z qiang wrote:
> > diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> > index 9c5c1cfa478f..f8d739fef311 100644
> > --- a/kernel/workqueue.c
> > +++ b/kernel/workqueue.c
> > @@ -1060,10 +1060,9 @@ void wq_worker_running(struct task_struct *task)
> >          * and leave with an unexpected pool->nr_running == 1 on the newly
> > unbound
> >          * pool. Protect against such race.
> >          */
> > -       preempt_disable();
> > +       local_irq_disable();
> >         if (!(worker->flags & WORKER_NOT_RUNNING))
> >                 worker->pool->nr_running++;
> > -       preempt_enable();
> > 
> >         /*
> >          * CPU intensive auto-detection cares about how long a work item
> > hogged
> > @@ -1072,6 +1071,7 @@ void wq_worker_running(struct task_struct *task)
> >         worker->current_at = worker->task->se.sum_exec_runtime;
> > 
> >         worker->sleeping = 0;
> > +       local_irq_enable();
> >  }
> 
> Ah, yeah, this is correct. Now we're modifying nr_running from timer tick
> too, so if don't block irq, the timer tick can ruin the not-irq-protected
> read-write-modify nr_running update from wq_worker_running(). Naresh, can
> you please confirm the fix?

Z qiang, while waiting for Naresh's test result, can you send the fix as a
proper signed-off-patch?

Thanks.

-- 
tejun