Commit 85e26dd5 authored by Christian König's avatar Christian König

drm/client: fix circular reference counting issue

We reference dump buffers both by their handle as well as their
object. The problem is now that when anybody iterates over the DRM
framebuffers and exports the underlying GEM objects through DMA-buf
we run into a circular reference count situation.

The result is that the fbdev handling holds the GEM handle preventing
the DMA-buf in the GEM object to be released. This DMA-buf in turn
holds a reference to the driver module which on unload would release
the fbdev.

Break that loop by releasing the handle as soon as the DRM
framebuffer object is created. The DRM framebuffer and the DRM client
buffer structure still hold a reference to the underlying GEM object
preventing its destruction.
Signed-off-by: default avatarChristian König <christian.koenig@amd.com>
Fixes: c76f0f7c ("drm: Begin an API for in-kernel clients")
Cc: <stable@vger.kernel.org>
Reviewed-by: default avatarThomas Zimmermann <tzimmermann@suse.de>
Tested-by: default avatarThomas Zimmermann <tzimmermann@suse.de>
Link: https://patchwork.freedesktop.org/patch/msgid/20230126102814.8722-1-christian.koenig@amd.com
parent 8f20660f
...@@ -233,21 +233,17 @@ void drm_client_dev_restore(struct drm_device *dev) ...@@ -233,21 +233,17 @@ void drm_client_dev_restore(struct drm_device *dev)
static void drm_client_buffer_delete(struct drm_client_buffer *buffer) static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
{ {
struct drm_device *dev = buffer->client->dev;
if (buffer->gem) { if (buffer->gem) {
drm_gem_vunmap_unlocked(buffer->gem, &buffer->map); drm_gem_vunmap_unlocked(buffer->gem, &buffer->map);
drm_gem_object_put(buffer->gem); drm_gem_object_put(buffer->gem);
} }
if (buffer->handle)
drm_mode_destroy_dumb(dev, buffer->handle, buffer->client->file);
kfree(buffer); kfree(buffer);
} }
static struct drm_client_buffer * static struct drm_client_buffer *
drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format) drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height,
u32 format, u32 *handle)
{ {
const struct drm_format_info *info = drm_format_info(format); const struct drm_format_info *info = drm_format_info(format);
struct drm_mode_create_dumb dumb_args = { }; struct drm_mode_create_dumb dumb_args = { };
...@@ -269,16 +265,15 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u ...@@ -269,16 +265,15 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u
if (ret) if (ret)
goto err_delete; goto err_delete;
buffer->handle = dumb_args.handle;
buffer->pitch = dumb_args.pitch;
obj = drm_gem_object_lookup(client->file, dumb_args.handle); obj = drm_gem_object_lookup(client->file, dumb_args.handle);
if (!obj) { if (!obj) {
ret = -ENOENT; ret = -ENOENT;
goto err_delete; goto err_delete;
} }
buffer->pitch = dumb_args.pitch;
buffer->gem = obj; buffer->gem = obj;
*handle = dumb_args.handle;
return buffer; return buffer;
...@@ -365,7 +360,8 @@ static void drm_client_buffer_rmfb(struct drm_client_buffer *buffer) ...@@ -365,7 +360,8 @@ static void drm_client_buffer_rmfb(struct drm_client_buffer *buffer)
} }
static int drm_client_buffer_addfb(struct drm_client_buffer *buffer, static int drm_client_buffer_addfb(struct drm_client_buffer *buffer,
u32 width, u32 height, u32 format) u32 width, u32 height, u32 format,
u32 handle)
{ {
struct drm_client_dev *client = buffer->client; struct drm_client_dev *client = buffer->client;
struct drm_mode_fb_cmd fb_req = { }; struct drm_mode_fb_cmd fb_req = { };
...@@ -377,7 +373,7 @@ static int drm_client_buffer_addfb(struct drm_client_buffer *buffer, ...@@ -377,7 +373,7 @@ static int drm_client_buffer_addfb(struct drm_client_buffer *buffer,
fb_req.depth = info->depth; fb_req.depth = info->depth;
fb_req.width = width; fb_req.width = width;
fb_req.height = height; fb_req.height = height;
fb_req.handle = buffer->handle; fb_req.handle = handle;
fb_req.pitch = buffer->pitch; fb_req.pitch = buffer->pitch;
ret = drm_mode_addfb(client->dev, &fb_req, client->file); ret = drm_mode_addfb(client->dev, &fb_req, client->file);
...@@ -414,13 +410,24 @@ struct drm_client_buffer * ...@@ -414,13 +410,24 @@ struct drm_client_buffer *
drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format) drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format)
{ {
struct drm_client_buffer *buffer; struct drm_client_buffer *buffer;
u32 handle;
int ret; int ret;
buffer = drm_client_buffer_create(client, width, height, format); buffer = drm_client_buffer_create(client, width, height, format,
&handle);
if (IS_ERR(buffer)) if (IS_ERR(buffer))
return buffer; return buffer;
ret = drm_client_buffer_addfb(buffer, width, height, format); ret = drm_client_buffer_addfb(buffer, width, height, format, handle);
/*
* The handle is only needed for creating the framebuffer, destroy it
* again to solve a circular dependency should anybody export the GEM
* object as DMA-buf. The framebuffer and our buffer structure are still
* holding references to the GEM object to prevent its destruction.
*/
drm_mode_destroy_dumb(client->dev, handle, client->file);
if (ret) { if (ret) {
drm_client_buffer_delete(buffer); drm_client_buffer_delete(buffer);
return ERR_PTR(ret); return ERR_PTR(ret);
......
...@@ -126,11 +126,6 @@ struct drm_client_buffer { ...@@ -126,11 +126,6 @@ struct drm_client_buffer {
*/ */
struct drm_client_dev *client; struct drm_client_dev *client;
/**
* @handle: Buffer handle
*/
u32 handle;
/** /**
* @pitch: Buffer pitch * @pitch: Buffer pitch
*/ */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment