linux-stable.git/drivers/gpu/drm/drm_gem.c, branch linux-5.15.y

drm/gem: Fix race in drm_gem_handle_create_tail()

2025-07-17T16:30:49+00:00

commit bd46cece51a36ef088f22ef0416ac13b0a46d5b0 upstream.

Object creation is a careful dance where we must guarantee that the
object is fully constructed before it is visible to other threads, and
GEM buffer objects are no difference.

Final publishing happens by calling drm_gem_handle_create(). After
that the only allowed thing to do is call drm_gem_object_put() because
a concurrent call to the GEM_CLOSE ioctl with a correctly guessed id
(which is trivial since we have a linear allocator) can already tear
down the object again.

Luckily most drivers get this right, the very few exceptions I've
pinged the relevant maintainers for. Unfortunately we also need
drm_gem_handle_create() when creating additional handles for an
already existing object (e.g. GETFB ioctl or the various bo import
ioctl), and hence we cannot have a drm_gem_handle_create_and_put() as
the only exported function to stop these issues from happening.

Now unfortunately the implementation of drm_gem_handle_create() isn't
living up to standards: It does correctly finishe object
initialization at the global level, and hence is safe against a
concurrent tear down. But it also sets up the file-private aspects of
the handle, and that part goes wrong: We fully register the object in
the drm_file.object_idr before calling drm_vma_node_allow() or
obj->funcs->open, which opens up races against concurrent removal of
that handle in drm_gem_handle_delete().

Fix this with the usual two-stage approach of first reserving the
handle id, and then only registering the object after we've completed
the file-private setup.

Jacek reported this with a testcase of concurrently calling GEM_CLOSE
on a freshly-created object (which also destroys the object), but it
should be possible to hit this with just additional handles created
through import or GETFB without completed destroying the underlying
object with the concurrent GEM_CLOSE ioctl calls.

Note that the close-side of this race was fixed in f6cd7daecff5 ("drm:
Release driver references to handle before making it available
again"), which means a cool 9 years have passed until someone noticed
that we need to make this symmetry or there's still gaps left :-/
Without the 2-stage close approach we'd still have a race, therefore
that's an integral part of this bugfix.

More importantly, this means we can have NULL pointers behind
allocated id in our drm_file.object_idr. We need to check for that
now:

- drm_gem_handle_delete() checks for ERR_OR_NULL already

- drm_gem.c:object_lookup() also chekcs for NULL

- drm_gem_release() should never be called if there's another thread
  still existing that could call into an IOCTL that creates a new
  handle, so cannot race. For paranoia I added a NULL check to
  drm_gem_object_release_handle() though.

- most drivers (etnaviv, i915, msm) are find because they use
  idr_find(), which maps both ENOENT and NULL to NULL.

- drivers using idr_for_each_entry() should also be fine, because
  idr_get_next does filter out NULL entries and continues the
  iteration.

- The same holds for drm_show_memory_stats().

v2: Use drm_WARN_ON (Thomas)

Reported-by: Jacek Lawrynowicz 
Tested-by: Jacek Lawrynowicz 
Reviewed-by: Thomas Zimmermann 
Cc: stable@vger.kernel.org
Cc: Jacek Lawrynowicz 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: David Airlie 
Cc: Simona Vetter 
Signed-off-by: Simona Vetter 
Signed-off-by: Simona Vetter 
Link: https://patchwork.freedesktop.org/patch/msgid/20250707151814.603897-1-simona.vetter@ffwll.ch
Signed-off-by: Greg Kroah-Hartman

drm/gem: Fix GEM handle release errors

2022-09-15T09:30:01+00:00

[ Upstream commit ea2aa97ca37a9044ade001aef71dbc06318e8d44 ]

Currently we are assuming a one to one mapping between dmabuf and
GEM handle when releasing GEM handles.

But that is not always true, since we would create extra handles for the
GEM obj in cases like gem_open() and getfb{,2}().

A similar issue was reported at:
https://lore.kernel.org/all/20211105083308.392156-1-jay.xu@rock-chips.com/

Another problem is that the imported dmabuf might not always have
gem_obj->dma_buf set, which would cause leaks in
drm_gem_remove_prime_handles().

Let's fix these for now by using handle to find the exact map to remove.

Signed-off-by: Jeffy Chen 
Reviewed-by: Christian König 
Signed-off-by: Christian König 
Link: https://patchwork.freedesktop.org/patch/msgid/20220819072834.17888-1-jeffy.chen@rock-chips.com
Signed-off-by: Sasha Levin

drm/gem: Properly annotate WW context on drm_gem_lock_reservations() error

2022-08-17T12:22:53+00:00

commit 2939deac1fa220bc82b89235f146df1d9b52e876 upstream.

Use ww_acquire_fini() in the error code paths. Otherwise lockdep
thinks that lock is held when lock's memory is freed after the
drm_gem_lock_reservations() error. The ww_acquire_context needs to be
annotated as "released", which fixes the noisy "WARNING: held lock freed!"
splat of VirtIO-GPU driver with CONFIG_DEBUG_MUTEXES=y and enabled lockdep.

Cc: stable@vger.kernel.org
Fixes: 7edc3e3b975b5 ("drm: Add helpers for locking an array of BO reservations.")
Reviewed-by: Thomas Hellström 
Reviewed-by: Christian König 
Signed-off-by: Dmitry Osipenko 
Signed-off-by: Daniel Vetter 
Link: https://patchwork.freedesktop.org/patch/msgid/20220630200405.1883897-2-dmitry.osipenko@collabora.com
Signed-off-by: Greg Kroah-Hartman

drm: Fix typo in comments

2021-08-02T08:19:43+00:00

fix typo for drm

v1->v2:
respin with the change "iff ==> implies that"

Signed-off-by: Cai Huoqing 
Signed-off-by: Daniel Vetter 
Link: https://patchwork.freedesktop.org/patch/msgid/20210730132729.376-1-caihuoqing@baidu.com

Merge tag 'drm-msm-next-2021-07-28' of https://gitlab.freedesktop.org/drm/msm into drm-next

2021-07-30T06:24:01+00:00

An early pull for v5.15 (there'll be more coming in a week or two),
consisting of the drm/scheduler conversion and a couple other small
series that one was based one.  Mostly sending this now because IIUC
danvet wanted it in drm-next so he could rebase on it.  (Daniel, if
you disagree then speak up, and I'll instead include this in the main
pull request once that is ready.)

This also has a core patch to drop drm_gem_object_put_locked() now
that the last use of it is removed.

[airlied: add NULL to drm_sched_init]

Signed-off-by: Dave Airlie 
From: Rob Clark 
Link: https://patchwork.freedesktop.org/patch/msgid/CAF6AEGumRk7H88bqV=H9Fb1SM0zPBo5B7NsCU3jFFKBYxf5k+Q@mail.gmail.com

drm: Drop drm_gem_object_put_locked()

2021-07-28T01:09:18+00:00

Now that no one is using it, remove it.

Signed-off-by: Rob Clark 
Acked-by: Christian König 
Reviewed-by: Daniel Vetter 
Link: https://lore.kernel.org/r/20210728010632.2633470-5-robdclark@gmail.com
Signed-off-by: Rob Clark

drm/gem: Tiny kernel clarification for drm_gem_fence_array_add

2021-06-24T12:53:42+00:00

Spotted while trying to convert panfrost to these.

Reviewed-by: Christian König 
Signed-off-by: Daniel Vetter 
Cc: "Christian König" 
Cc: Lucas Stach 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: David Airlie 
Cc: Daniel Vetter 
Link: https://patchwork.freedesktop.org/patch/msgid/20210622165511.3169559-15-daniel.vetter@ffwll.ch

drm/vma: Add a driver_private member to vma_node.

2021-06-11T08:53:18+00:00

This allows drivers to distinguish between different types of vma_node's.
The readonly flag was unused and is thus removed.

This is a temporary solution, until i915 is converted completely to
use ttm for bo's.

Signed-off-by: Maarten Lankhorst 
Reviewed-by: Thomas Hellström 
Acked-by: Daniel Vetter  #irc
Signed-off-by: Maarten Lankhorst 
Link: https://patchwork.freedesktop.org/patch/msgid/20210610070152.572423-4-thomas.hellstrom@linux.intel.com

dma-buf: drop the _rcu postfix on function names v3

2021-06-06T09:19:51+00:00

The functions can be called both in _rcu context as well
as while holding the lock.

v2: add some kerneldoc as suggested by Daniel
v3: fix indentation

Signed-off-by: Christian König 
Reviewed-by: Jason Ekstrand 
Acked-by: Daniel Vetter 
Link: https://patchwork.freedesktop.org/patch/msgid/20210602111714.212426-7-christian.koenig@amd.com

dma-buf: rename dma_resv_get_excl_rcu to _unlocked

2021-06-06T09:19:30+00:00

That describes much better what the function is doing here.

Signed-off-by: Christian König 
Reviewed-by: Jason Ekstrand 
Acked-by: Daniel Vetter 
Link: https://patchwork.freedesktop.org/patch/msgid/20210602111714.212426-6-christian.koenig@amd.com