From: Philipp Stanner <phasta@mailbox.org>
To: "Christian König" <christian.koenig@amd.com>,
"Jonghyuk Kim(MalHyuk)" <malhyuk97@gmail.com>,
phasta@kernel.org, tursulin@ursulin.net, matthew.brost@intel.com,
dakr@kernel.org
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
mdaenzer@redhat.com, alessio.belle@imgtec.com,
luigi.santivetti@imgtec.com, stable@vger.kernel.org
Subject: Re: [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free
Date: Fri, 04 Sep 2026 10:31:31 +0200 [thread overview]
Message-ID: <ee27d37269f609192cdfd7a217d9731d7dca04c8.camel@mailbox.org> (raw)
In-Reply-To: <f25f2a91-b72e-4392-ab53-c11ef60e0fe0@amd.com>
On Fri, 2026-09-04 at 10:20 +0200, Christian König wrote:
>
[…]
> >
> > +/*
> > + * TODO: Both fences implement .release, so dma_fence keeps their ops attached
> > + * after signalling. Dropping the callbacks would let dma_fence detach the ops,
> > + * after which neither get_timeline_name() nor get_driver_name() can run against
> > + * a freed scheduler or an unloaded module - the complete fix. It first requires
> > + * auditing every to_drm_sched_fence() caller, since ops-detach makes the helper
> > + * return NULL for a signalled fence. See Documentation/gpu/todo.rst.
> > + */
>
> That sounds like a bad idea as well.
>
> Dropping the fence->ops is to detach the fence from the module which originally issued it and not solve lifetime problems between the scheduler and the driver.
It can be used to solve that problem though, can it not?
The underlying problem is that the driver has no chance to figure out
when the scheduler is actually done with all the sched_fences.
Remember our lengthy discussions about drm_sched_fini(). Maybe we want
to reconsider providing a function with which the driver can wait until
the scheduler is done with all finished_fences?
>
> I think we should rather re-consider patch 035219a760edb35ae9a9e96beba7f122e26a997b ("dma-buf: dma-fence: Fix potential NULL pointer dereference"):
>
> Here we changed the check in dma_fence_driver_name() and dma_fence_timeline_name():
>
> @@ -1167,7 +1167,7 @@ const char __rcu *dma_fence_driver_name(struct dma_fence *fence)
>
> /* RCU protection is required for safe access to returned string */
> ops = rcu_dereference(fence->ops);
> - if (!dma_fence_test_signaled_flag(fence))
> + if (ops)
> return (const char __rcu *)ops->get_driver_name(fence);
> else
> return (const char __rcu *)"detached-driver";
>
> The problem is that we didn't considered that there a fence implementations which still have a release or wait callbacks but rely on not needing to return a string for a signaled fence.
>
Could we move the signaled check to amdgpu and pvr?
IOW, we keep the solution presented here (removing ops->release for
finished-fence) and the few drivers that check whether a fence is their
own first do a locked dma_fence_is_signaled() check?
P.
next prev parent reply other threads:[~2026-09-04 8:31 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-04 8:06 [PATCH v4 0/3] drm/sched: fix use-after-free of the fence timeline name Jonghyuk Kim(MalHyuk)
2026-09-04 8:06 ` [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free Jonghyuk Kim(MalHyuk)
2026-09-04 8:20 ` Christian König
2026-09-04 8:31 ` Philipp Stanner [this message]
2026-09-04 12:49 ` Christian König
2026-09-04 19:06 ` Philipp Stanner
2026-09-04 8:31 ` Jonghyuk Kim(MalHyuk)
2026-09-04 8:39 ` Philipp Stanner
2026-09-04 9:11 ` Jonghyuk Kim(MalHyuk)
2026-09-04 9:07 ` Tvrtko Ursulin
2026-09-04 9:57 ` Danilo Krummrich
2026-09-04 10:51 ` Philipp Stanner
2026-09-04 8:06 ` [PATCH v4 2/3] drm/sched: add the fence ops-detach cleanup to the TODO list Jonghyuk Kim(MalHyuk)
2026-09-04 8:06 ` [PATCH v4 3/3] drm/sched/tests: add a UAF regression test for the timeline name Jonghyuk Kim(MalHyuk)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ee27d37269f609192cdfd7a217d9731d7dca04c8.camel@mailbox.org \
--to=phasta@mailbox.org \
--cc=alessio.belle@imgtec.com \
--cc=christian.koenig@amd.com \
--cc=dakr@kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luigi.santivetti@imgtec.com \
--cc=malhyuk97@gmail.com \
--cc=matthew.brost@intel.com \
--cc=mdaenzer@redhat.com \
--cc=phasta@kernel.org \
--cc=stable@vger.kernel.org \
--cc=tursulin@ursulin.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®