From: Danilo Krummrich <dakr@kernel.org>
To: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: phasta@kernel.org, "Lyude Paul" <lyude@redhat.com>,
"David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>,
"Matthew Brost" <matthew.brost@intel.com>,
"Christian König" <ckoenig.leichtzumerken@gmail.com>,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
"Maxime Ripard" <mripard@kernel.org>,
"Thomas Zimmermann" <tzimmermann@suse.de>,
dri-devel@lists.freedesktop.org, nouveau@lists.freedesktop.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/5] drm/sched: Warn if pending list is not empty
Date: Tue, 22 Apr 2025 13:13:38 +0200 [thread overview]
Message-ID: <aAd54jUwBwgc-_g2@cassiopeiae> (raw)
In-Reply-To: <5a5d4a33-2f7b-46e4-8707-7445ac3de376@igalia.com>
On Tue, Apr 22, 2025 at 11:39:11AM +0100, Tvrtko Ursulin wrote:
> Question I raised is if there are other drivers which manage to clean up
> everything correctly (like the mock scheduler does), but trigger that
> warning. Maybe there are not and maybe mock scheduler is the only false
> positive.
So far the scheduler simply does not give any guideline on how to address the
problem, hence every driver simply does something (or nothing, effectively
ignoring the problem). This is what we want to fix.
The mock scheduler keeps it's own list of pending jobs and on tear down stops
the scheduler's workqueues, traverses it's own list and eventually frees the
pending jobs without updating the scheduler's internal pending list.
So yes, it does avoid memory leaks, but it also leaves the schedulers internal
structures with an invalid state, i.e. the pending list of the scheduler has
pointers to already freed memory.
What if the drm_sched_fini() starts touching the pending list? Then you'd end up
with UAF bugs with this implementation. We cannot invalidate the schedulers
internal structures and yet call scheduler functions - e.g. drm_sched_fini() -
subsequently.
Hence, the current implementation of the mock scheduler is fundamentally flawed.
And so would be *every* driver that still has entries within the scheduler's
pending list.
This is not a false positive, it already caught a real bug -- in the mock
scheduler.
- Danilo
next prev parent reply other threads:[~2025-04-22 11:13 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-07 15:22 [PATCH 0/5] drm/sched: Fix memory leaks in drm_sched_fini() Philipp Stanner
2025-04-07 15:22 ` [PATCH 1/5] drm/sched: Fix teardown leaks with waitqueue Philipp Stanner
2025-04-17 7:49 ` Philipp Stanner
2025-04-07 15:22 ` [PATCH 2/5] drm/sched: Prevent teardown waitque from blocking too long Philipp Stanner
2025-04-07 15:22 ` [PATCH 3/5] drm/sched: Warn if pending list is not empty Philipp Stanner
2025-04-17 7:45 ` Philipp Stanner
2025-04-17 11:27 ` Tvrtko Ursulin
2025-04-17 12:11 ` Danilo Krummrich
2025-04-17 14:20 ` Tvrtko Ursulin
2025-04-17 14:48 ` Danilo Krummrich
2025-04-17 16:08 ` Tvrtko Ursulin
2025-04-17 17:07 ` Danilo Krummrich
2025-04-22 6:06 ` Philipp Stanner
2025-04-22 10:39 ` Tvrtko Ursulin
2025-04-22 11:13 ` Danilo Krummrich [this message]
2025-04-22 12:00 ` Philipp Stanner
2025-04-22 13:25 ` Tvrtko Ursulin
2025-04-22 12:07 ` Tvrtko Ursulin
2025-04-22 12:21 ` Philipp Stanner
2025-04-22 12:32 ` Danilo Krummrich
2025-04-22 13:39 ` Tvrtko Ursulin
2025-04-22 13:46 ` Philipp Stanner
2025-04-22 14:08 ` Danilo Krummrich
2025-04-22 14:16 ` Philipp Stanner
2025-04-22 14:52 ` Danilo Krummrich
2025-04-23 7:34 ` Tvrtko Ursulin
2025-04-23 8:48 ` Danilo Krummrich
2025-04-23 10:10 ` Tvrtko Ursulin
2025-04-23 10:26 ` Danilo Krummrich
2025-04-07 15:22 ` [PATCH 4/5] drm/nouveau: Add new callback for scheduler teardown Philipp Stanner
2025-04-07 15:22 ` [PATCH 5/5] drm/nouveau: Remove waitque for sched teardown Philipp Stanner
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=aAd54jUwBwgc-_g2@cassiopeiae \
--to=dakr@kernel.org \
--cc=airlied@gmail.com \
--cc=ckoenig.leichtzumerken@gmail.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lyude@redhat.com \
--cc=maarten.lankhorst@linux.intel.com \
--cc=matthew.brost@intel.com \
--cc=mripard@kernel.org \
--cc=nouveau@lists.freedesktop.org \
--cc=phasta@kernel.org \
--cc=simona@ffwll.ch \
--cc=tvrtko.ursulin@igalia.com \
--cc=tzimmermann@suse.de \
/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;
as well as URLs for NNTP newsgroup(s).