On Fri, Jan 08, 2016 at 11:27:05PM +0000, Chris Wilson wrote:
> When userspace closes a handle, we remove it from the file->object_idr
> and then tell the driver to drop its references to that file/handle.
> However, as the file/handle is already available again for reuse, it may
> be reallocated back to userspace and active on a new object before the
> driver has had a chance to drop the old file/handle references.
Hmm. What's the problem with another object starting to reuse the same
handle while we're still deleting the old one? So far I didn't spot
anything in the code that would go boom if there's another object
already around with the same handle.
>
> Whilst calling back into the driver, we have to drop the
> file->table_lock spinlock and so to prevent reusing the closed handle we
> mark that handle as stale in the idr, perform the callback and then
> remove the handle. We set the stale handle to point to the NULL object,
> then any idr_find() whilst the driver is removing the handle will return
> NULL, just as if the handle is already removed from idr.
>
> v2: Use NULL rather than an ERR_PTR to avoid having to adjust callers.
> idr_alloc() tracks existing handles using an internal bitmap, so we are
> free to use the NULL object as our stale identifier.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: David Airlie <airlied@xxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> Cc: Rob Clark <robdclark@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Thierry Reding <treding@xxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_gem.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 2e8c77e71e1f..d1909d1a1eb4 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -294,18 +294,21 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
> spin_lock(&filp->table_lock);
>
> /* Check if we currently have a reference on the object */
> - obj = idr_find(&filp->object_idr, handle);
> - if (obj == NULL) {
> + obj = idr_replace(&filp->object_idr, NULL, handle);
> + if (IS_ERR(obj)) {
> spin_unlock(&filp->table_lock);
> return -EINVAL;
> }
> dev = obj->dev;
> + spin_unlock(&filp->table_lock);
>
> /* Release reference and decrement refcount. */
> + drm_gem_object_release_handle(handle, obj, filp);
> +
> + spin_lock(&filp->table_lock);
> idr_remove(&filp->object_idr, handle);
> spin_unlock(&filp->table_lock);
>
> - drm_gem_object_release_handle(handle, obj, filp);
> return 0;
> }
> EXPORT_SYMBOL(drm_gem_handle_delete);
> --
> 2.7.0.rc3
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
