From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56]) (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 C5246333752 for ; Fri, 5 Dec 2025 13:18:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.97.179.56 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764940721; cv=none; b=RIFSiYfZTMsnbpWj1z3NgqHh7+9ygxTxvwL0lnimGQyCphkGuDkVP5RGWs27t//sfohTOgavcpGj9le8TGevyQ2xLuT+b/C6GKGqOjT+nlMKHIg/zWqAD9CDi7DJo/vV102EQQ5/HmDwXMKXvmn6smUZcAPiKzEl1UTmCmyI3B8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764940721; c=relaxed/simple; bh=S5NRY3tSqikOZ3aHm4pADpIqPsHbCvsIuoyHCYH11AY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=O3IfauTJSRBl7j+0zZnrImYJe05xz1yFiJ48V5v82opK22mDw7axx5BaFDAoBaYAchhGZo6ftJk9+nccyS9XPH11Jt+v+Q0XAw4eVIhzXzEWAVc9V96mrPxZs3mFMqzk9UeyBILETnb1LyD+dVTUu/7dNOp721KAwe2S5d1N3Ls= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=igalia.com; spf=pass smtp.mailfrom=igalia.com; dkim=pass (2048-bit key) header.d=igalia.com header.i=@igalia.com header.b=X266o0EB; arc=none smtp.client-ip=213.97.179.56 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=igalia.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=igalia.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=igalia.com header.i=@igalia.com header.b="X266o0EB" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=Mxf6F5azhlLiM73Ar4oF/jjdMNj+7xqHba/Bo3gvuCs=; b=X266o0EB4uxo/WNCkFVmIIyz3R vFLTpNGqONpzU4eAXZMXDFPHJUKRp/AVgGnwBMKrFD5Y7p3lGYW9+l1fm4d2Htt5VJkXqbfElBpW6 hh70bVlavUaB5XQMNAfTmsawSxmw9MwkIWTGGWJ9+9L67sc3My1bdpX3wqKBZG4lPvqbYGMRFmRWW C/FIYbtAyDY2gNbxKyhmb8qajTjDDR7L2KONyonG5EC77RTdgUnQ6HHauodHeiTaj3CarPsOcOHTi lD78mbhEPW0HUs5Up03gudK7n72rLJNF5Y9MeU1hRGRfUi8MnMpTF6d6S5pmaM+AxdwsmMgvZAzeX 15CrOo6g==; Received: from [90.240.106.137] (helo=[192.168.0.101]) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128) (Exim) id 1vRVGN-0094qC-Kg; Fri, 05 Dec 2025 13:50:19 +0100 Message-ID: <34d18685-d07f-4e13-b5ca-d4d75d33d6f6@igalia.com> Date: Fri, 5 Dec 2025 12:50:18 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] drm/panthor: fix for dma-fence safe access rules To: Boris Brezillon , Chia-I Wu Cc: Steven Price , Liviu Dudau , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Grant Likely , Heiko Stuebner , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20251204015034.841235-1-olvaffe@gmail.com> <4e59c6f8-bc9b-4fd5-9b0f-511cce760ac2@igalia.com> <20251205134629.608df99b@fedora> Content-Language: en-GB From: Tvrtko Ursulin In-Reply-To: <20251205134629.608df99b@fedora> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 05/12/2025 12:46, Boris Brezillon wrote: > On Thu, 4 Dec 2025 09:42:37 -0800 > Chia-I Wu wrote: > >> On Thu, Dec 4, 2025 at 1:27 AM Tvrtko Ursulin wrote: >>> >>> >>> On 04/12/2025 01:50, Chia-I Wu wrote: >>>> Commit 506aa8b02a8d6 ("dma-fence: Add safe access helpers and document >>>> the rules") details the dma-fence safe access rules. The most common >>>> culprit is that drm_sched_fence_get_timeline_name may race with >>>> group_free_queue. >>>> >>>> Fixes: d2624d90a0b77 ("drm/panthor: assign unique names to queues") >>>> Signed-off-by: Chia-I Wu >>>> --- >>>> drivers/gpu/drm/panthor/panthor_sched.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c >>>> index 33b9ef537e359..a8b1347e4da71 100644 >>>> --- a/drivers/gpu/drm/panthor/panthor_sched.c >>>> +++ b/drivers/gpu/drm/panthor/panthor_sched.c >>>> @@ -23,6 +23,7 @@ >>>> #include >>>> #include >>>> #include >>>> +#include >>>> >>>> #include "panthor_devfreq.h" >>>> #include "panthor_device.h" >>>> @@ -923,6 +924,9 @@ static void group_release_work(struct work_struct *work) >>>> release_work); >>>> u32 i; >>>> >>>> + /* dma-fences may still be accessing group->queues under rcu lock. */ >>>> + synchronize_rcu(); >>>> + >>>> for (i = 0; i < group->queue_count; i++) >>>> group_free_queue(group, group->queues[i]); >>>> >>> >>> This handles the shared queue->fence_ctx.lock as well (which is also >>> unsafe until Christian lands the inline lock, etc patch series) so it >>> looks good to me as well. >> Yeah, I will send v2 to drop the misleading "Fixes:" tag. >> >> FWIW, the UAF I saw was from accessing the string returned by >> >> static const char *drm_sched_fence_get_timeline_name(struct dma_fence *f) >> { >> struct drm_sched_fence *fence = to_drm_sched_fence(f); >> return (const char *)fence->sched->name; >> } > > IIRC, the only place calling this callback is some debugsfs knob > dumping fences attached to dma_buf resvs, and we're not supposed to > expose our driver fences to the outside world (we use the > drm_sched_fence proxy for that), so I'm curious where the access was > coming from. Via the sync_file uapi. For reference here is a reproducer for xe: https://patchwork.freedesktop.org/patch/642709/?series=146211&rev=2 Regards, Tvrtko > >> >> I thought it was "name" and added the "Fixes:" tag. But actually >> "sched" was also freed by group_release_work. >> >>> >>> Just to mention an alternative could be to simply switch release_work to >>> INIT_RCU_WORK/queue_rcu_work, but I am not sure if that has an advantage. >>> >>> Regards, >>> >>> Tvrtko >>> >