From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from bali.collaboradmins.com (bali.collaboradmins.com [148.251.105.195]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0AD463859C3 for ; Mon, 3 Aug 2026 09:30:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=148.251.105.195 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785749409; cv=none; b=SCdOoyZbngSJAmmdFODGN7AWaiz0jrEYuCgoIOluhg1MwyjiW7zNC51MgYERndovpbk1l5xtPbdbIVG6dM1kLVpAjYN3Xu8CqJ5DUNHP577Nn5XlD2ghUxSwTXLZvFG8fUPF9TG9TSRaEmKS7HVFgg5fZGjEgN8QkWaFj2hTVh4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785749409; c=relaxed/simple; bh=zoaM8tKoFUvw0g8W7cpM9crcYV/t5OzS0yVXiTNuVY4=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=mmXoPdMcqpuKErjtp22vouh82xh0/+A8dtc9kKX4HlNMTNa3hoSeQMgABUOqctAU8HSCObK9HeZ7ftLQLoBgiZMgHK+IX1LmKJ6QT4F0YrEdD03OSVkAYa3F8FQtMwxD/hXkb/sWHeDN8YQFtw4Y+RctbFG4xJ9Mi+F9vcuzr1o= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com; spf=pass smtp.mailfrom=collabora.com; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b=Yg4UesiG; arc=none smtp.client-ip=148.251.105.195 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=collabora.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b="Yg4UesiG" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1785749399; bh=zoaM8tKoFUvw0g8W7cpM9crcYV/t5OzS0yVXiTNuVY4=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Yg4UesiGWMbG3g3hhxQzQcl8pXd0+dNFiNt22nBZ7V3ZQwjwZrWaWYYg9oJDZkhdz lYJUCLVcCGmhze4/FCYYibD/mPnBJ9gXmjVqtBI+BpTw/9zielvt4NQKPLnG6btB1F jJF05ymlKHPlKVB9hp61ezSAoCS4ftCEc5RQaXEhjmaexqy0ptSs9lH/HWAjHPWL1G QSk3N7UuW3q8k402j5389VGsu2CQbvHQ29zxkEdikoFSdi60cBQ/1pncawnT3ExH8f daZttOwJwl2NvWztQwDjO17H74+0PZamHK58X2wenZYHL/ZDM9YO7rf8eJAhTSu+lX jKRiZrYo63WBQ== Received: from fedora1.home (unknown [100.64.0.11]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange secp256r1 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: bbrezillon) by bali.collaboradmins.com (Postfix) with ESMTPSA id 87B6217E08E1; Mon, 03 Aug 2026 11:29:58 +0200 (CEST) Date: Mon, 3 Aug 2026 11:29:54 +0200 From: Boris Brezillon To: Nicolas Frattaroli Cc: Steven Price , Liviu Dudau , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Chia-I Wu , Karunika Choo , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, sashiko-bot@kernel.org Subject: Re: [PATCH 03/11] drm/panthor: Fix UAF on works queued to panthor_cleanup_wq Message-ID: <20260803112954.69bf18f5@fedora1.home> In-Reply-To: References: <20260625-panthor-misc-fixes-v1-0-b67ed973fea6@collabora.com> <20260625-panthor-misc-fixes-v1-3-b67ed973fea6@collabora.com> Organization: Collabora X-Mailer: Claws Mail 4.4.0 (GTK 3.24.52; x86_64-redhat-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Mon, 20 Jul 2026 11:47:59 +0200 Nicolas Frattaroli wrote: > On Thursday, 25 June 2026 14:40:29 Central European Summer Time Boris Brezillon wrote: > > If we don't drain the panthor_cleanup_wq before > > unregistering/destroying the panthor_device, we might end up with > > cleanup works that are executed after the device is gone. > > > > Add a drain_workqueue() call in panthor_device_unplug() to prevent > > that. > > > > Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block") > > Fixes: 647810ec2476 ("drm/panthor: Add the MMU/VM logical block") > > Reported-by: sashiko-bot@kernel.org > > Closes: https://sashiko.dev/#/patchset/20260625-panthor-signal-from-irq-v5-0-8836a74e0ef9@collabora.com?part=2 > > Signed-off-by: Boris Brezillon > > --- > > drivers/gpu/drm/panthor/panthor_device.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c > > index 0b25abebb803..a0774e28aa94 100644 > > --- a/drivers/gpu/drm/panthor/panthor_device.c > > +++ b/drivers/gpu/drm/panthor/panthor_device.c > > @@ -101,6 +101,11 @@ void panthor_device_unplug(struct panthor_device *ptdev) > > panthor_gpu_unplug(ptdev); > > panthor_pwr_unplug(ptdev); > > > > + /* Make sure works queued to panthor_cleanup_wq are executed > > + * before the device is destroyed. > > + */ > > + drain_workqueue(panthor_cleanup_wq); > > + > > pm_runtime_dont_use_autosuspend(ptdev->base.dev); > > pm_runtime_put_sync_suspend(ptdev->base.dev); > > > > > > > > I feel like nothing in the driver instance should be allowed to queue new > work to that workqueue while the unplug mutex is being held, and > panthor_sched_unplug should just disable_work_sync all the associated group > release_work items as well. If there's not a strong enough relationship > between a scheduler and the groups to ensure we get all the groups in a > non-racey manner that might not work out cleanly though. Release is a bit a special work though. It's something that happens once we know the group has left the scheduler, and all it's supposed to do is free the resources the group might hold (mostly memory). > > Having a scheduler unplug be completely disconnected from the associated > groups being gone seems counter-intuitive to me. It's actually the other way around: unplugging the scheduler might result in new group_release work items being queued to the cleanup_wq. The drain_worqueue() (which should actually be a flush_worqueue) is here to guarantee that we're not leaving objects behind that would still have references to objects that are about to be destroyed (in the case of group_release, that's the drm_device, which holds all the GEM database that we need to return the GEM refs the destroyed groups need to release).