Commit 933db733 authored by Vasily Averin's avatar Vasily Averin Committed by Gerd Hoffmann

drm/qxl: qxl_release use after free

qxl_release should not be accesses after qxl_push_*_ring_release() calls:
userspace driver can process submitted command quickly, move qxl_release
into release_ring, generate interrupt and trigger garbage collector.

It can lead to crashes in qxl driver or trigger memory corruption
in some kmalloc-192 slab object

Gerd Hoffmann proposes to swap the qxl_release_fence_buffer_objects() +
qxl_push_{cursor,command}_ring_release() calls to close that race window.

cc: stable@vger.kernel.org
Fixes: f64122c1 ("drm: add new QXL driver. (v1.4)")
Signed-off-by: default avatarVasily Averin <vvs@virtuozzo.com>
Link: http://patchwork.freedesktop.org/patch/msgid/fa17b338-66ae-f299-68fe-8d32419d9071@virtuozzo.comSigned-off-by: default avatarGerd Hoffmann <kraxel@redhat.com>
parent 5b5703db
...@@ -500,8 +500,8 @@ int qxl_hw_surface_alloc(struct qxl_device *qdev, ...@@ -500,8 +500,8 @@ int qxl_hw_surface_alloc(struct qxl_device *qdev,
/* no need to add a release to the fence for this surface bo, /* no need to add a release to the fence for this surface bo,
since it is only released when we ask to destroy the surface since it is only released when we ask to destroy the surface
and it would never signal otherwise */ and it would never signal otherwise */
qxl_push_command_ring_release(qdev, release, QXL_CMD_SURFACE, false);
qxl_release_fence_buffer_objects(release); qxl_release_fence_buffer_objects(release);
qxl_push_command_ring_release(qdev, release, QXL_CMD_SURFACE, false);
surf->hw_surf_alloc = true; surf->hw_surf_alloc = true;
spin_lock(&qdev->surf_id_idr_lock); spin_lock(&qdev->surf_id_idr_lock);
...@@ -543,9 +543,8 @@ int qxl_hw_surface_dealloc(struct qxl_device *qdev, ...@@ -543,9 +543,8 @@ int qxl_hw_surface_dealloc(struct qxl_device *qdev,
cmd->surface_id = id; cmd->surface_id = id;
qxl_release_unmap(qdev, release, &cmd->release_info); qxl_release_unmap(qdev, release, &cmd->release_info);
qxl_push_command_ring_release(qdev, release, QXL_CMD_SURFACE, false);
qxl_release_fence_buffer_objects(release); qxl_release_fence_buffer_objects(release);
qxl_push_command_ring_release(qdev, release, QXL_CMD_SURFACE, false);
return 0; return 0;
} }
......
...@@ -510,8 +510,8 @@ static int qxl_primary_apply_cursor(struct drm_plane *plane) ...@@ -510,8 +510,8 @@ static int qxl_primary_apply_cursor(struct drm_plane *plane)
cmd->u.set.visible = 1; cmd->u.set.visible = 1;
qxl_release_unmap(qdev, release, &cmd->release_info); qxl_release_unmap(qdev, release, &cmd->release_info);
qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
qxl_release_fence_buffer_objects(release); qxl_release_fence_buffer_objects(release);
qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
return ret; return ret;
...@@ -652,8 +652,8 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane, ...@@ -652,8 +652,8 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane,
cmd->u.position.y = plane->state->crtc_y + fb->hot_y; cmd->u.position.y = plane->state->crtc_y + fb->hot_y;
qxl_release_unmap(qdev, release, &cmd->release_info); qxl_release_unmap(qdev, release, &cmd->release_info);
qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
qxl_release_fence_buffer_objects(release); qxl_release_fence_buffer_objects(release);
qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
if (old_cursor_bo != NULL) if (old_cursor_bo != NULL)
qxl_bo_unpin(old_cursor_bo); qxl_bo_unpin(old_cursor_bo);
...@@ -700,8 +700,8 @@ static void qxl_cursor_atomic_disable(struct drm_plane *plane, ...@@ -700,8 +700,8 @@ static void qxl_cursor_atomic_disable(struct drm_plane *plane,
cmd->type = QXL_CURSOR_HIDE; cmd->type = QXL_CURSOR_HIDE;
qxl_release_unmap(qdev, release, &cmd->release_info); qxl_release_unmap(qdev, release, &cmd->release_info);
qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
qxl_release_fence_buffer_objects(release); qxl_release_fence_buffer_objects(release);
qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
} }
static void qxl_update_dumb_head(struct qxl_device *qdev, static void qxl_update_dumb_head(struct qxl_device *qdev,
......
...@@ -243,8 +243,8 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev, ...@@ -243,8 +243,8 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev,
} }
qxl_bo_kunmap(clips_bo); qxl_bo_kunmap(clips_bo);
qxl_push_command_ring_release(qdev, release, QXL_CMD_DRAW, false);
qxl_release_fence_buffer_objects(release); qxl_release_fence_buffer_objects(release);
qxl_push_command_ring_release(qdev, release, QXL_CMD_DRAW, false);
out_release_backoff: out_release_backoff:
if (ret) if (ret)
......
...@@ -261,11 +261,8 @@ static int qxl_process_single_command(struct qxl_device *qdev, ...@@ -261,11 +261,8 @@ static int qxl_process_single_command(struct qxl_device *qdev,
apply_surf_reloc(qdev, &reloc_info[i]); apply_surf_reloc(qdev, &reloc_info[i]);
} }
qxl_release_fence_buffer_objects(release);
ret = qxl_push_command_ring_release(qdev, release, cmd->type, true); ret = qxl_push_command_ring_release(qdev, release, cmd->type, true);
if (ret)
qxl_release_backoff_reserve_list(release);
else
qxl_release_fence_buffer_objects(release);
out_free_bos: out_free_bos:
out_free_release: out_free_release:
......
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