From 6292b8efe32e6be408af364132f09572aed14382 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Thu, 23 Apr 2020 18:17:43 +0300 Subject: [PATCH 1/9] drm/edid: Fix off-by-one in DispID DTD pixel clock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DispID DTD pixel clock is documented as: "00 00 00 h → FF FF FF h | Pixel clock ÷ 10,000 0.01 → 167,772.16 Mega Pixels per Sec" Which seems to imply that we to add one to the raw value. Reality seems to agree as there are tiled displays in the wild which currently show a 10kHz difference in the pixel clock between the tiles (one tile gets its mode from the base EDID, the other from the DispID block). Cc: stable@vger.kernel.org References: https://gitlab.freedesktop.org/drm/intel/-/issues/27 Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20200423151743.18767-1-ville.syrjala@linux.intel.com Reviewed-by: Manasi Navare --- drivers/gpu/drm/drm_edid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 116451101426..4ede08a84e37 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5111,7 +5111,7 @@ static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *d struct drm_display_mode *mode; unsigned pixel_clock = (timings->pixel_clock[0] | (timings->pixel_clock[1] << 8) | - (timings->pixel_clock[2] << 16)); + (timings->pixel_clock[2] << 16)) + 1; unsigned hactive = (timings->hactive[0] | timings->hactive[1] << 8) + 1; unsigned hblank = (timings->hblank[0] | timings->hblank[1] << 8) + 1; unsigned hsync = (timings->hsync[0] | (timings->hsync[1] & 0x7f) << 8) + 1; From a5bff92eaac45bdf6221badf9505c26792fdf99e Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 7 Apr 2020 15:30:02 +0200 Subject: [PATCH 2/9] dma-buf: Fix SET_NAME ioctl uapi The uapi is the same on 32 and 64 bit, but the number isn't. Everyone who botched this please re-read: https://www.kernel.org/doc/html/v5.4-preprc-cpu/ioctl/botching-up-ioctls.html Also, the type argument for the ioctl macros is for the type the void __user *arg pointer points at, which in this case would be the variable-sized char[] of a 0 terminated string. So this was botched in more than just the usual ways. Cc: Sumit Semwal Cc: Chenbo Feng Cc: Greg Hackmann Cc: Daniel Vetter Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@lists.linaro.org Cc: minchan@kernel.org Cc: surenb@google.com Cc: jenhaochen@google.com Cc: Martin Liu Signed-off-by: Daniel Vetter Tested-by: Martin Liu Reviewed-by: Martin Liu Signed-off-by: Sumit Semwal [sumits: updated some checkpatch fixes, corrected author email] Link: https://patchwork.freedesktop.org/patch/msgid/20200407133002.3486387-1-daniel.vetter@ffwll.ch --- drivers/dma-buf/dma-buf.c | 3 ++- include/uapi/linux/dma-buf.h | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index ccc9eda1bc28..de155d41d274 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -388,7 +388,8 @@ static long dma_buf_ioctl(struct file *file, return ret; - case DMA_BUF_SET_NAME: + case DMA_BUF_SET_NAME_A: + case DMA_BUF_SET_NAME_B: return dma_buf_set_name(dmabuf, (const char __user *)arg); default: diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h index dbc7092e04b5..7f30393b92c3 100644 --- a/include/uapi/linux/dma-buf.h +++ b/include/uapi/linux/dma-buf.h @@ -39,6 +39,12 @@ struct dma_buf_sync { #define DMA_BUF_BASE 'b' #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync) + +/* 32/64bitness of this uapi was botched in android, there's no difference + * between them in actual uapi, they're just different numbers. + */ #define DMA_BUF_SET_NAME _IOW(DMA_BUF_BASE, 1, const char *) +#define DMA_BUF_SET_NAME_A _IOW(DMA_BUF_BASE, 1, u32) +#define DMA_BUF_SET_NAME_B _IOW(DMA_BUF_BASE, 1, u64) #endif From dbc05ae3867628abce5619bfab868a43818bdb0d Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Fri, 24 Apr 2020 15:07:22 -0400 Subject: [PATCH 3/9] drm/dp_mst: Fix drm_dp_send_dpcd_write() return code drm_dp_mst_wait_tx_reply() returns > 1 if time elapsed in wait_event_timeout() before check_txmsg_state(mgr, txmsg) evaluated to true. However, we make the mistake of returning this time from drm_dp_send_dpcd_write() on success instead of returning the number of bytes written - causing spontaneous failures during link probing: [drm:drm_dp_send_link_address [drm_kms_helper]] *ERROR* GUID check on 10:01 failed: 3975 Yikes! So, fix this by returning the number of bytes written on success instead. Signed-off-by: Lyude Paul Fixes: cb897542c6d2 ("drm/dp_mst: Fix W=1 warnings") Cc: Benjamin Gaignard Cc: Sean Paul Acked-by: Alex Deucher Reviewed-by: Sean Paul Link: https://patchwork.freedesktop.org/patch/msgid/20200424190722.775284-1-lyude@redhat.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 283615e44838..9d89ebf3a749 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3442,8 +3442,12 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr, drm_dp_queue_down_tx(mgr, txmsg); ret = drm_dp_mst_wait_tx_reply(mstb, txmsg); - if (ret > 0 && txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK) - ret = -EIO; + if (ret > 0) { + if (txmsg->reply.reply_type == DP_SIDEBAND_REPLY_NAK) + ret = -EIO; + else + ret = size; + } kfree(txmsg); fail_put: From 45c5d2a4f39ce15fabd8e0485854b3448f1ef3b8 Mon Sep 17 00:00:00 2001 From: Gurchetan Singh Date: Wed, 8 Apr 2020 16:29:38 -0700 Subject: [PATCH 4/9] drm/virtio: only destroy created contexts This can happen if userspace doesn't issue any 3D ioctls before closing the DRM fd. Fixes: 72b48ae800da ("drm/virtio: enqueue virtio_gpu_create_context after the first 3D ioctl") Signed-off-by: Gurchetan Singh Link: http://patchwork.freedesktop.org/patch/msgid/20200408232938.55816-1-gurchetansingh@chromium.org Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_kms.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index 023a030ca7b9..a5038298c6ae 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -52,14 +52,6 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work) events_clear, &events_clear); } -static void virtio_gpu_context_destroy(struct virtio_gpu_device *vgdev, - uint32_t ctx_id) -{ - virtio_gpu_cmd_context_destroy(vgdev, ctx_id); - virtio_gpu_notify(vgdev); - ida_free(&vgdev->ctx_id_ida, ctx_id - 1); -} - static void virtio_gpu_init_vq(struct virtio_gpu_queue *vgvq, void (*work_func)(struct work_struct *work)) { @@ -274,14 +266,17 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file) void virtio_gpu_driver_postclose(struct drm_device *dev, struct drm_file *file) { struct virtio_gpu_device *vgdev = dev->dev_private; - struct virtio_gpu_fpriv *vfpriv; + struct virtio_gpu_fpriv *vfpriv = file->driver_priv; if (!vgdev->has_virgl_3d) return; - vfpriv = file->driver_priv; + if (vfpriv->context_created) { + virtio_gpu_cmd_context_destroy(vgdev, vfpriv->ctx_id); + virtio_gpu_notify(vgdev); + } - virtio_gpu_context_destroy(vgdev, vfpriv->ctx_id); + ida_free(&vgdev->ctx_id_ida, vfpriv->ctx_id - 1); mutex_destroy(&vfpriv->context_lock); kfree(vfpriv); file->driver_priv = NULL; From 85e9b88af1e6164f19ec71381efd5e2bcfc17620 Mon Sep 17 00:00:00 2001 From: Vasily Averin Date: Mon, 27 Apr 2020 08:32:46 +0300 Subject: [PATCH 5/9] drm/qxl: qxl_release leak in qxl_draw_dirty_fb() ret should be changed to release allocated struct qxl_release Cc: stable@vger.kernel.org Fixes: 8002db6336dd ("qxl: convert qxl driver to proper use for reservations") Signed-off-by: Vasily Averin Link: http://patchwork.freedesktop.org/patch/msgid/22cfd55f-07c8-95d0-a2f7-191b7153c3d4@virtuozzo.com Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_draw.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_draw.c b/drivers/gpu/drm/qxl/qxl_draw.c index 5bebf1ea1c5d..f8776d60d08e 100644 --- a/drivers/gpu/drm/qxl/qxl_draw.c +++ b/drivers/gpu/drm/qxl/qxl_draw.c @@ -209,9 +209,10 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev, goto out_release_backoff; rects = drawable_set_clipping(qdev, num_clips, clips_bo); - if (!rects) + if (!rects) { + ret = -EINVAL; goto out_release_backoff; - + } drawable = (struct qxl_drawable *)qxl_release_map(qdev, release); drawable->clip.type = SPICE_CLIP_TYPE_RECTS; From a65aa9c3676ffccb21361d52fcfedd5b5ff387d7 Mon Sep 17 00:00:00 2001 From: Vasily Averin Date: Mon, 27 Apr 2020 08:32:51 +0300 Subject: [PATCH 6/9] drm/qxl: qxl_release leak in qxl_hw_surface_alloc() Cc: stable@vger.kernel.org Fixes: 8002db6336dd ("qxl: convert qxl driver to proper use for reservations") Signed-off-by: Vasily Averin Link: http://patchwork.freedesktop.org/patch/msgid/2e5a13ae-9ab2-5401-aa4d-03d5f5593423@virtuozzo.com Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_cmd.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c index d1086b2a6892..fa8762d15d0f 100644 --- a/drivers/gpu/drm/qxl/qxl_cmd.c +++ b/drivers/gpu/drm/qxl/qxl_cmd.c @@ -480,9 +480,10 @@ int qxl_hw_surface_alloc(struct qxl_device *qdev, return ret; ret = qxl_release_reserve_list(release, true); - if (ret) + if (ret) { + qxl_release_free(qdev, release); return ret; - + } cmd = (struct qxl_surface_cmd *)qxl_release_map(qdev, release); cmd->type = QXL_SURFACE_CMD_CREATE; cmd->flags = QXL_SURF_FLAG_KEEP_DATA; From 5b5703dbafae74adfbe298a56a81694172caf5e6 Mon Sep 17 00:00:00 2001 From: Vasily Averin Date: Wed, 29 Apr 2020 12:34:36 +0300 Subject: [PATCH 7/9] drm/qxl: lost qxl_bo_kunmap_atomic_page in qxl_image_init_helper() v2: removed TODO reminder Signed-off-by: Vasily Averin Link: http://patchwork.freedesktop.org/patch/msgid/a4e0ae09-a73c-1c62-04ef-3f990d41bea9@virtuozzo.com Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_image.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/qxl/qxl_image.c b/drivers/gpu/drm/qxl/qxl_image.c index 43688ecdd8a0..60ab7151b84d 100644 --- a/drivers/gpu/drm/qxl/qxl_image.c +++ b/drivers/gpu/drm/qxl/qxl_image.c @@ -212,7 +212,8 @@ qxl_image_init_helper(struct qxl_device *qdev, break; default: DRM_ERROR("unsupported image bit depth\n"); - return -EINVAL; /* TODO: cleanup */ + qxl_bo_kunmap_atomic_page(qdev, image_bo, ptr); + return -EINVAL; } image->u.bitmap.flags = QXL_BITMAP_TOP_DOWN; image->u.bitmap.x = width; From 933db73351d359f74b14f4af095808260aff11f9 Mon Sep 17 00:00:00 2001 From: Vasily Averin Date: Wed, 29 Apr 2020 12:01:24 +0300 Subject: [PATCH 8/9] 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: f64122c1f6ad ("drm: add new QXL driver. (v1.4)") Signed-off-by: Vasily Averin Link: http://patchwork.freedesktop.org/patch/msgid/fa17b338-66ae-f299-68fe-8d32419d9071@virtuozzo.com Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_cmd.c | 5 ++--- drivers/gpu/drm/qxl/qxl_display.c | 6 +++--- drivers/gpu/drm/qxl/qxl_draw.c | 2 +- drivers/gpu/drm/qxl/qxl_ioctl.c | 5 +---- 4 files changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c index fa8762d15d0f..05863b253d68 100644 --- a/drivers/gpu/drm/qxl/qxl_cmd.c +++ b/drivers/gpu/drm/qxl/qxl_cmd.c @@ -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, since it is only released when we ask to destroy the surface and it would never signal otherwise */ - qxl_push_command_ring_release(qdev, release, QXL_CMD_SURFACE, false); qxl_release_fence_buffer_objects(release); + qxl_push_command_ring_release(qdev, release, QXL_CMD_SURFACE, false); surf->hw_surf_alloc = true; spin_lock(&qdev->surf_id_idr_lock); @@ -543,9 +543,8 @@ int qxl_hw_surface_dealloc(struct qxl_device *qdev, cmd->surface_id = id; 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_push_command_ring_release(qdev, release, QXL_CMD_SURFACE, false); return 0; } diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index 09583a08e141..91f398d51cfa 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -510,8 +510,8 @@ static int qxl_primary_apply_cursor(struct drm_plane *plane) cmd->u.set.visible = 1; 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_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false); return ret; @@ -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; 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_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false); if (old_cursor_bo != NULL) qxl_bo_unpin(old_cursor_bo); @@ -700,8 +700,8 @@ static void qxl_cursor_atomic_disable(struct drm_plane *plane, cmd->type = QXL_CURSOR_HIDE; 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_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false); } static void qxl_update_dumb_head(struct qxl_device *qdev, diff --git a/drivers/gpu/drm/qxl/qxl_draw.c b/drivers/gpu/drm/qxl/qxl_draw.c index f8776d60d08e..3599db096973 100644 --- a/drivers/gpu/drm/qxl/qxl_draw.c +++ b/drivers/gpu/drm/qxl/qxl_draw.c @@ -243,8 +243,8 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev, } qxl_bo_kunmap(clips_bo); - qxl_push_command_ring_release(qdev, release, QXL_CMD_DRAW, false); qxl_release_fence_buffer_objects(release); + qxl_push_command_ring_release(qdev, release, QXL_CMD_DRAW, false); out_release_backoff: if (ret) diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c index 8117a45b3610..72f3f1bbb40c 100644 --- a/drivers/gpu/drm/qxl/qxl_ioctl.c +++ b/drivers/gpu/drm/qxl/qxl_ioctl.c @@ -261,11 +261,8 @@ static int qxl_process_single_command(struct qxl_device *qdev, apply_surf_reloc(qdev, &reloc_info[i]); } + qxl_release_fence_buffer_objects(release); 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_release: From 6f49c2515e2258f08f2b905c9772dbf729610415 Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Tue, 7 Apr 2020 21:20:34 -0700 Subject: [PATCH 9/9] dma-buf: fix documentation build warnings Fix documentation warnings in dma-buf.[hc]: ../drivers/dma-buf/dma-buf.c:678: warning: Function parameter or member 'importer_ops' not described in 'dma_buf_dynamic_attach' ../drivers/dma-buf/dma-buf.c:678: warning: Function parameter or member 'importer_priv' not described in 'dma_buf_dynamic_attach' ../include/linux/dma-buf.h:339: warning: Incorrect use of kernel-doc format: * @move_notify Signed-off-by: Randy Dunlap Cc: Sumit Semwal Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linaro-mm-sig@lists.linaro.org Signed-off-by: Sumit Semwal Link: https://patchwork.freedesktop.org/patch/msgid/7bcbe6fe-0b4b-87da-d003-b68a26eb4cf0@infradead.org --- drivers/dma-buf/dma-buf.c | 4 ++-- include/linux/dma-buf.h | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index de155d41d274..07df88f2e305 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -656,8 +656,8 @@ EXPORT_SYMBOL_GPL(dma_buf_put); * calls attach() of dma_buf_ops to allow device-specific attach functionality * @dmabuf: [in] buffer to attach device to. * @dev: [in] device to be attached. - * @importer_ops [in] importer operations for the attachment - * @importer_priv [in] importer private pointer for the attachment + * @importer_ops: [in] importer operations for the attachment + * @importer_priv: [in] importer private pointer for the attachment * * Returns struct dma_buf_attachment pointer for this attachment. Attachments * must be cleaned up by calling dma_buf_detach(). diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 1ade486fc2bb..57bcef6f988a 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -329,13 +329,12 @@ struct dma_buf { /** * struct dma_buf_attach_ops - importer operations for an attachment - * @move_notify: [optional] notification that the DMA-buf is moving * * Attachment operations implemented by the importer. */ struct dma_buf_attach_ops { /** - * @move_notify + * @move_notify: [optional] notification that the DMA-buf is moving * * If this callback is provided the framework can avoid pinning the * backing store while mappings exists.