linux-kernel - Re: [PATCH -rc] workqueue: Reimplement UAF fix to avoid lockdep worning

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: <Zl9BOaPDsQBc8hSL@slm.duckdns.org>
Date: Tue, 4 Jun 2024 06:30:49 -1000
From: Tejun Heo <tj@...nel.org>
To: Leon Romanovsky <leon@...nel.org>
Cc: Hillf Danton <hdanton@...a.com>, Peter Zijlstra <peterz@...radead.org>,
	Lai Jiangshan <jiangshanlai@...il.com>,
	Zqiang <qiang.zhang1211@...il.com>, linux-kernel@...r.kernel.org,
	Gal Pressman <gal@...dia.com>, Tariq Toukan <tariqt@...dia.com>,
	RDMA mailing list <linux-rdma@...r.kernel.org>
Subject: Re: [PATCH -rc] workqueue: Reimplement UAF fix to avoid lockdep
 worning

Hello, Leon.

On Tue, Jun 04, 2024 at 02:38:34PM +0300, Leon Romanovsky wrote:
> Thanks, it is very rare situation where call to flush/drain queue
> (in our case kthread_flush_worker) in the middle of the allocation
> flow can be correct. I can't remember any such case.
>
> So even we don't fully understand the root cause, the reimplementation
> is still valid and improves existing code.

It's not valid. pwq release is async and while wq free in the error path
isn't. The flush is there so that we finish the async part before
synchronize error handling. The patch you posted will can lead to double
free after a pwq allocation failure. We can make the error path synchronous
but the pwq free path should be updated first so that it stays synchronous
in the error path. Note that it *needs* to be asynchronous in non-error
paths, so it's going to be a bit subtle one way or the other.

Thanks.

-- 
tejun