From fefaedcfb82d2e57c2320acf60604ab03b750cc0 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sun, 12 May 2013 22:00:51 -0700 Subject: [PATCH 1/7] drm/radeon: check incoming cliprects pointer The "boxes" parameter points into userspace memory. It should be verified like any other operation against user memory. Signed-off-by: Kees Cook Cc: stable@vger.kernel.org Signed-off-by: Dave Airlie --- drivers/gpu/drm/radeon/r300_cmdbuf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/r300_cmdbuf.c b/drivers/gpu/drm/radeon/r300_cmdbuf.c index 865e2c9980db..60170ea5e3a2 100644 --- a/drivers/gpu/drm/radeon/r300_cmdbuf.c +++ b/drivers/gpu/drm/radeon/r300_cmdbuf.c @@ -75,7 +75,7 @@ static int r300_emit_cliprects(drm_radeon_private_t *dev_priv, OUT_RING(CP_PACKET0(R300_RE_CLIPRECT_TL_0, nr * 2 - 1)); for (i = 0; i < nr; ++i) { - if (DRM_COPY_FROM_USER_UNCHECKED + if (DRM_COPY_FROM_USER (&box, &cmdbuf->boxes[n + i], sizeof(box))) { DRM_ERROR("copy cliprect faulted\n"); return -EFAULT; From 95643359f8b31e74b35901f4e36cd069cd67fd48 Mon Sep 17 00:00:00 2001 From: Dave Airlie Date: Mon, 13 May 2013 15:23:49 +1000 Subject: [PATCH 2/7] drm: remove unused wrapper macros We don't use these anymore so nuke them. Signed-off-by: Dave Airlie --- include/drm/drm_os_linux.h | 9 --------- 1 file changed, 9 deletions(-) diff --git a/include/drm/drm_os_linux.h b/include/drm/drm_os_linux.h index 393369147a2d..675ddf4b441f 100644 --- a/include/drm/drm_os_linux.h +++ b/include/drm/drm_os_linux.h @@ -87,15 +87,6 @@ static __inline__ int mtrr_del(int reg, unsigned long base, unsigned long size) /** Other copying of data from kernel space */ #define DRM_COPY_TO_USER(arg1, arg2, arg3) \ copy_to_user(arg1, arg2, arg3) -/* Macros for copyfrom user, but checking readability only once */ -#define DRM_VERIFYAREA_READ( uaddr, size ) \ - (access_ok( VERIFY_READ, uaddr, size ) ? 0 : -EFAULT) -#define DRM_COPY_FROM_USER_UNCHECKED(arg1, arg2, arg3) \ - __copy_from_user(arg1, arg2, arg3) -#define DRM_COPY_TO_USER_UNCHECKED(arg1, arg2, arg3) \ - __copy_to_user(arg1, arg2, arg3) -#define DRM_GET_USER_UNCHECKED(val, uaddr) \ - __get_user(val, uaddr) #define DRM_HZ HZ From a6ac1bc341e499ad5296f265dfa8eba5afbf4191 Mon Sep 17 00:00:00 2001 From: Dave Airlie Date: Tue, 14 May 2013 02:13:24 +0100 Subject: [PATCH 3/7] drm/qxl: fix ioport interactions for kernel submitted commands. So qxl has ioports, but it really really really doesn't want you to write to them twice, but if you write and get a signal before the irq arrives to let you know its completed, you have to think ahead and avoid writing another time. However this works fine for update area where really multiple writes aren't the end of the world, however with create primary surface, you can't ever do multiple writes. So this stop internal kernel writes from doing interruptible waits, because otherwise we have no idea if this write is a new one or a continuation of a previous one. virtual hw sucks more than real hw. This fixes lockups and VM crashes when resizing and starting/stopping X. Signed-off-by: Dave Airlie --- drivers/gpu/drm/qxl/qxl_cmd.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c index 08b0823c93d5..f86771481317 100644 --- a/drivers/gpu/drm/qxl/qxl_cmd.c +++ b/drivers/gpu/drm/qxl/qxl_cmd.c @@ -277,7 +277,7 @@ out_unref: return 0; } -static int wait_for_io_cmd_user(struct qxl_device *qdev, uint8_t val, long port) +static int wait_for_io_cmd_user(struct qxl_device *qdev, uint8_t val, long port, bool intr) { int irq_num; long addr = qdev->io_base + port; @@ -285,20 +285,29 @@ static int wait_for_io_cmd_user(struct qxl_device *qdev, uint8_t val, long port) mutex_lock(&qdev->async_io_mutex); irq_num = atomic_read(&qdev->irq_received_io_cmd); - - if (qdev->last_sent_io_cmd > irq_num) { - ret = wait_event_interruptible(qdev->io_cmd_event, - atomic_read(&qdev->irq_received_io_cmd) > irq_num); - if (ret) + if (intr) + ret = wait_event_interruptible_timeout(qdev->io_cmd_event, + atomic_read(&qdev->irq_received_io_cmd) > irq_num, 5*HZ); + else + ret = wait_event_timeout(qdev->io_cmd_event, + atomic_read(&qdev->irq_received_io_cmd) > irq_num, 5*HZ); + /* 0 is timeout, just bail the "hw" has gone away */ + if (ret <= 0) goto out; irq_num = atomic_read(&qdev->irq_received_io_cmd); } outb(val, addr); qdev->last_sent_io_cmd = irq_num + 1; - ret = wait_event_interruptible(qdev->io_cmd_event, - atomic_read(&qdev->irq_received_io_cmd) > irq_num); + if (intr) + ret = wait_event_interruptible_timeout(qdev->io_cmd_event, + atomic_read(&qdev->irq_received_io_cmd) > irq_num, 5*HZ); + else + ret = wait_event_timeout(qdev->io_cmd_event, + atomic_read(&qdev->irq_received_io_cmd) > irq_num, 5*HZ); out: + if (ret > 0) + ret = 0; mutex_unlock(&qdev->async_io_mutex); return ret; } @@ -308,7 +317,7 @@ static void wait_for_io_cmd(struct qxl_device *qdev, uint8_t val, long port) int ret; restart: - ret = wait_for_io_cmd_user(qdev, val, port); + ret = wait_for_io_cmd_user(qdev, val, port, false); if (ret == -ERESTARTSYS) goto restart; } @@ -340,7 +349,7 @@ int qxl_io_update_area(struct qxl_device *qdev, struct qxl_bo *surf, mutex_lock(&qdev->update_area_mutex); qdev->ram_header->update_area = *area; qdev->ram_header->update_surface = surface_id; - ret = wait_for_io_cmd_user(qdev, 0, QXL_IO_UPDATE_AREA_ASYNC); + ret = wait_for_io_cmd_user(qdev, 0, QXL_IO_UPDATE_AREA_ASYNC, true); mutex_unlock(&qdev->update_area_mutex); return ret; } From d7292a07a1b3d0b31a54a3e949ed4dd99e9a85e8 Mon Sep 17 00:00:00 2001 From: Dave Airlie Date: Mon, 13 May 2013 12:42:26 +1000 Subject: [PATCH 4/7] qxl: drop unused variable. this boolean isn't used anymore so drop it. Signed-off-by: Dave Airlie --- drivers/gpu/drm/qxl/qxl_display.c | 1 - drivers/gpu/drm/qxl/qxl_drv.h | 1 - 2 files changed, 2 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index fcfd4436ceed..735ddd2cb99f 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -604,7 +604,6 @@ static int qxl_crtc_mode_set(struct drm_crtc *crtc, mode->hdisplay, mode->vdisplay); } - qdev->mode_set = true; return 0; } diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h index 52b582c211da..5b7c130f4744 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.h +++ b/drivers/gpu/drm/qxl/qxl_drv.h @@ -270,7 +270,6 @@ struct qxl_device { struct qxl_ring *cursor_ring; struct qxl_ram_header *ram_header; - bool mode_set; bool primary_created; From b2b4465d8bade681491e225fa6a5dc050820b004 Mon Sep 17 00:00:00 2001 From: Dave Airlie Date: Mon, 13 May 2013 12:48:40 +1000 Subject: [PATCH 5/7] drm/qxl: drop active_user_framebuffer as its unneeded This was a bogus way to figure out what the active framebuffer was, just check if the underlying bo is the primary bo. Signed-off-by: Dave Airlie --- drivers/gpu/drm/qxl/qxl_display.c | 16 ++++------------ drivers/gpu/drm/qxl/qxl_drv.h | 6 ------ 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index 735ddd2cb99f..823d29e926ec 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -428,10 +428,10 @@ static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb, int inc = 1; qobj = gem_to_qxl_bo(qxl_fb->obj); - if (qxl_fb != qdev->active_user_framebuffer) { - DRM_INFO("%s: qxl_fb 0x%p != qdev->active_user_framebuffer 0x%p\n", - __func__, qxl_fb, qdev->active_user_framebuffer); - } + /* if we aren't primary surface ignore this */ + if (!qobj->is_primary) + return 0; + if (!num_clips) { num_clips = 1; clips = &norect; @@ -892,7 +892,6 @@ qxl_user_framebuffer_create(struct drm_device *dev, { struct drm_gem_object *obj; struct qxl_framebuffer *qxl_fb; - struct qxl_device *qdev = dev->dev_private; int ret; obj = drm_gem_object_lookup(dev, file_priv, mode_cmd->handles[0]); @@ -908,13 +907,6 @@ qxl_user_framebuffer_create(struct drm_device *dev, return NULL; } - if (qdev->active_user_framebuffer) { - DRM_INFO("%s: active_user_framebuffer %p -> %p\n", - __func__, - qdev->active_user_framebuffer, qxl_fb); - } - qdev->active_user_framebuffer = qxl_fb; - return &qxl_fb->base; } diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h index 5b7c130f4744..43d06ab28a21 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.h +++ b/drivers/gpu/drm/qxl/qxl_drv.h @@ -255,12 +255,6 @@ struct qxl_device { struct qxl_gem gem; struct qxl_mode_info mode_info; - /* - * last created framebuffer with fb_create - * only used by debugfs dumbppm - */ - struct qxl_framebuffer *active_user_framebuffer; - struct fb_info *fbdev_info; struct qxl_framebuffer *fbdev_qfb; void *ram_physical; From b90ed1e931c4d11cf32710c8a310b603effb5b11 Mon Sep 17 00:00:00 2001 From: Dave Airlie Date: Thu, 9 May 2013 05:07:10 +0100 Subject: [PATCH 6/7] qxl: fix bug with object eviction and update area if the surface is evicted, this validation will happen to the wrong place, I noticed this with other work I was doing, haven't seen it go wrong in practice. Signed-off-by: Dave Airlie --- drivers/gpu/drm/qxl/qxl_ioctl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c index 04b64f9cbfdb..6db7370373ea 100644 --- a/drivers/gpu/drm/qxl/qxl_ioctl.c +++ b/drivers/gpu/drm/qxl/qxl_ioctl.c @@ -294,6 +294,7 @@ static int qxl_update_area_ioctl(struct drm_device *dev, void *data, goto out; if (!qobj->pin_count) { + qxl_ttm_placement_from_domain(qobj, qobj->type); ret = ttm_bo_validate(&qobj->tbo, &qobj->placement, true, false); if (unlikely(ret)) From e9ced8e040ebe40e9953db90acbe7d0b58702ebb Mon Sep 17 00:00:00 2001 From: Dave Airlie Date: Wed, 15 May 2013 01:23:36 +0000 Subject: [PATCH 7/7] drm/radeon: restore nomodeset operation (v2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When UMS was deprecated it removed support for nomodeset commandline we really want this in distro land so we can debug stuff, everyone should fallback to vesa correctly. v2: oops -1 isn't used anymore, restore original behaviour -1 is default, so we can boot with nomodeset on the command line, then use radeon.modeset=1 to override it for debugging later. Cc: stable@vger.kernel.org Reviewed-by: Alex Deucher Reviewed-by: Christian König Signed-off-by: Dave Airlie --- drivers/gpu/drm/radeon/radeon_drv.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index d33f484ace48..094e7e5ea39e 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -147,7 +147,7 @@ static inline void radeon_unregister_atpx_handler(void) {} #endif int radeon_no_wb; -int radeon_modeset = 1; +int radeon_modeset = -1; int radeon_dynclks = -1; int radeon_r4xx_atom = 0; int radeon_agpmode = 0; @@ -456,6 +456,16 @@ static struct pci_driver radeon_kms_pci_driver = { static int __init radeon_init(void) { +#ifdef CONFIG_VGA_CONSOLE + if (vgacon_text_force() && radeon_modeset == -1) { + DRM_INFO("VGACON disable radeon kernel modesetting.\n"); + radeon_modeset = 0; + } +#endif + /* set to modesetting by default if not nomodeset */ + if (radeon_modeset == -1) + radeon_modeset = 1; + if (radeon_modeset == 1) { DRM_INFO("radeon kernel modesetting enabled.\n"); driver = &kms_driver;