From owner-freebsd-x11@FreeBSD.ORG Mon Jul 16 12:09:54 2007 Return-Path: X-Original-To: x11@freebsd.org Delivered-To: freebsd-x11@FreeBSD.ORG Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 2DA6C16A405 for ; Mon, 16 Jul 2007 12:09:54 +0000 (UTC) (envelope-from ganbold@micom.mng.net) Received: from publicd.ub.mng.net (publicd.ub.mng.net [202.179.0.88]) by mx1.freebsd.org (Postfix) with ESMTP id 7150C13C441 for ; Mon, 16 Jul 2007 12:09:53 +0000 (UTC) (envelope-from ganbold@micom.mng.net) Received: from [202.179.0.164] (helo=daemon.micom.mng.net) by publicd.ub.mng.net with esmtpa (Exim 4.67 (FreeBSD)) (envelope-from ) id 1IAOH9-000D7N-M6; Mon, 16 Jul 2007 18:58:00 +0800 Message-ID: <469B4F37.1030405@micom.mng.net> Date: Mon, 16 Jul 2007 18:57:59 +0800 From: Ganbold User-Agent: Thunderbird 2.0.0.0 (X11/20070425) MIME-Version: 1.0 To: Kostik Belousov References: <469B17BB.9050305@micom.mng.net> <20070716105316.GT2200@deviant.kiev.zoral.com.ua> In-Reply-To: <20070716105316.GT2200@deviant.kiev.zoral.com.ua> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.5 Cc: x11@freebsd.org Subject: Re: LOR when running google-earth X-BeenThere: freebsd-x11@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: X11 on FreeBSD -- maintaining and support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Jul 2007 12:09:54 -0000 Thanks for the patch, I will test it tomorrow morning and let you know. Ganbold Kostik Belousov wrote: > [Redirected to x11@] > > On Mon, Jul 16, 2007 at 03:01:15PM +0800, Ganbold wrote: > >> Hi, >> >> I'm having LOR when I try to run google-earth. I have Intel 945GM >> graphic card. >> >> devil# uname -an >> FreeBSD devil.micom.mng.net 7.0-CURRENT FreeBSD 7.0-CURRENT #1: Tue Jul >> 10 10:44:42 ULAT 2007 >> root@devil.micom.mng.net:/usr/obj/usr/src/sys/DEVIL i386 >> devil# >> >> >> lock order reversal: (sleepable after non-sleepable) >> 1st 0xc3c06cd8 drm device (drm device) @ >> /usr/src/sys/modules/drm/drm/../../../dev/drm/drm_drv.c:907 >> 2nd 0xc3e6ea3c user map (user map) @ /usr/src/sys/vm/vm_glue.c:182 >> KDB: stack backtrace: >> db_trace_self_wrapper(c08a3fa4,e6450a44,c066992e,c08a6485,c3e6ea3c,...) >> at db_trace_self_wrapper+0x26 >> kdb_backtrace(c08a6485,c3e6ea3c,c08be517,c08be517,c08bdfed,...) at >> kdb_backtrace+0x29 >> witness_checkorder(c3e6ea3c,9,c08bdfed,b6,c435a000,...) at >> witness_checkorder+0x6de >> _sx_xlock(c3e6ea3c,0,c08bdfed,b6,e6450aa8,...) at _sx_xlock+0x7d >> _vm_map_lock_read(c3e6e9f8,c08bdfed,b6,c435a000,a60,...) at >> _vm_map_lock_read+0x50 >> useracc(4ce1ede8,8,1,f5,c09ce1bc,...) at useracc+0x65 >> i915_cmdbuffer(c3bf3800,8018644b,c43814a0,3,c435a000,...) at >> i915_cmdbuffer+0x10f >> drm_ioctl(c3bf3800,8018644b,c43814a0,3,c435a000,...) at drm_ioctl+0x357 >> giant_ioctl(c3bf3800,8018644b,c43814a0,3,c435a000,...) at giant_ioctl+0x56 >> devfs_ioctl_f(c4cdd1b0,8018644b,c43814a0,c4356700,c435a000,...) at >> devfs_ioctl_f+0xc9 >> kern_ioctl(c435a000,9,8018644b,c43814a0,450c4c,...) at kern_ioctl+0x243 >> ioctl(c435a000,e6450cfc,e6450c80,c4022e4a,c435a000,...) at ioctl+0x134 >> linux_ioctl_drm(c435a000,e6450cfc,c4030c45,a2f,c435a000,...) at >> linux_ioctl_drm+0x2f >> linux_ioctl(c435a000,e6450cfc,e6450cf8,e6450d1c,c4032dd0,...) at >> linux_ioctl+0xca >> syscall(e6450d38) at syscall+0x2b3 >> Xint0x80_syscall() at Xint0x80_syscall+0x20 >> --- syscall (54, Linux ELF, linux_ioctl), eip = 0x49ea95f4, esp = >> 0xbfbfdf44, ebp = 0xbfbfdf64 --- >> KDB: enter: witness_checkorder >> [thread pid 899 tid 100122 ] >> Stopped at kdb_enter+0x32: leave >> db> bt >> Tracing pid 899 tid 100122 td 0xc435a000 >> kdb_enter(c0861150,c3e6ea3c,c08be517,c08be517,c08bdfed,...) at >> kdb_enter+0x32 >> witness_checkorder(c3e6ea3c,9,c08bdfed,b6,c435a000,...) at >> witness_checkorder+0x6f3 >> _sx_xlock(c3e6ea3c,0,c08bdfed,b6,e6450aa8,...) at _sx_xlock+0x7d >> _vm_map_lock_read(c3e6e9f8,c08bdfed,b6,c435a000,a60,...) at >> _vm_map_lock_read+0x50 >> useracc(4ce1ede8,8,1,f5,c09ce1bc,...) at useracc+0x65 >> i915_cmdbuffer(c3bf3800,8018644b,c43814a0,3,c435a000,...) at >> i915_cmdbuffer+0x10f >> drm_ioctl(c3bf3800,8018644b,c43814a0,3,c435a000,...) at drm_ioctl+0x357 >> giant_ioctl(c3bf3800,8018644b,c43814a0,3,c435a000,...) at giant_ioctl+0x56 >> devfs_ioctl_f(c4cdd1b0,8018644b,c43814a0,c4356700,c435a000,...) at >> devfs_ioctl_f+0xc9 >> kern_ioctl(c435a000,9,8018644b,c43814a0,450c4c,...) at kern_ioctl+0x243 >> ioctl(c435a000,e6450cfc,e6450c80,c4022e4a,c435a000,...) at ioctl+0x134 >> linux_ioctl_drm(c435a000,e6450cfc,c4030c45,a2f,c435a000,...) at >> linux_ioctl_drm+0x2f >> linux_ioctl(c435a000,e6450cfc,e6450cf8,e6450d1c,c4032dd0,...) at >> linux_ioctl+0xca >> syscall(e6450d38) at syscall+0x2b3 >> Xint0x80_syscall() at Xint0x80_syscall+0x20 >> --- syscall (54, Linux ELF, linux_ioctl), eip = 0x49ea95f4, esp = >> 0xbfbfdf44, ebp = 0xbfbfdf64 --- >> db> c >> > > The use of useracc() seems to be racy and not complete. Below is the patch > that could fix the LOR and add missing checks, hopefully, in race free way. > Patch wires the ranges that are accessed under the drm lock with the help of > the vslock(). > > Patch was only compile-tested. > > diff --git a/sys/dev/drm/i915_dma.c b/sys/dev/drm/i915_dma.c > index dfaf334..431a2f9 100644 > --- a/sys/dev/drm/i915_dma.c > +++ b/sys/dev/drm/i915_dma.c > @@ -367,20 +367,14 @@ static int i915_emit_cmds(drm_device_t * dev, int __user * buffer, int dwords) > for (i = 0; i < dwords;) { > int cmd, sz; > > - if (DRM_COPY_FROM_USER_UNCHECKED(&cmd, &buffer[i], sizeof(cmd))) { > - > - return DRM_ERR(EINVAL); > - } > + cmd = buffer[i]; > if ((sz = validate_cmd(cmd)) == 0 || i + sz > dwords) > return DRM_ERR(EINVAL); > > OUT_RING(cmd); > > while (++i, --sz) { > - if (DRM_COPY_FROM_USER_UNCHECKED(&cmd, &buffer[i], > - sizeof(cmd))) { > - return DRM_ERR(EINVAL); > - } > + cmd = buffer[i]; > OUT_RING(cmd); > } > } > @@ -401,10 +395,7 @@ static int i915_emit_box(drm_device_t * dev, > drm_clip_rect_t box; > RING_LOCALS; > > - if (DRM_COPY_FROM_USER_UNCHECKED(&box, &boxes[i], sizeof(box))) { > - return EFAULT; > - } > - > + box = boxes[i]; > if (box.y2 <= box.y1 || box.x2 <= box.x1 || box.y2 <= 0 || box.x2 <= 0) { > DRM_ERROR("Bad box %d,%d..%d,%d\n", > box.x1, box.y1, box.x2, box.y2); > @@ -604,6 +595,7 @@ static int i915_batchbuffer(DRM_IOCTL_ARGS) > drm_i915_sarea_t *sarea_priv = (drm_i915_sarea_t *) > dev_priv->sarea_priv; > drm_i915_batchbuffer_t batch; > + size_t cliplen; > int ret; > > if (!dev_priv->allow_batchbuffer) { > @@ -619,12 +611,22 @@ static int i915_batchbuffer(DRM_IOCTL_ARGS) > > LOCK_TEST_WITH_RETURN(dev, filp); > > + DRM_UNLOCK(); > + cliplen = batch.num_cliprects * sizeof(drm_clip_rect_t); > if (batch.num_cliprects && DRM_VERIFYAREA_READ(batch.cliprects, > - batch.num_cliprects * > - sizeof(drm_clip_rect_t))) > + cliplen)) { > + DRM_LOCK(); > return DRM_ERR(EFAULT); > - > + } > + ret = vslock(batch.cliprects, cliplen); > + if (ret) { > + DRM_ERROR("Fault wiring cliprects\n"); > + DRM_LOCK(); > + return DRM_ERR(EFAULT); > + } > + DRM_LOCK(); > ret = i915_dispatch_batchbuffer(dev, &batch); > + vsunlock(batch.cliprects, cliplen); > > sarea_priv->last_dispatch = (int)hw_status[5]; > return ret; > @@ -638,6 +640,7 @@ static int i915_cmdbuffer(DRM_IOCTL_ARGS) > drm_i915_sarea_t *sarea_priv = (drm_i915_sarea_t *) > dev_priv->sarea_priv; > drm_i915_cmdbuffer_t cmdbuf; > + size_t cliplen; > int ret; > > DRM_COPY_FROM_USER_IOCTL(cmdbuf, (drm_i915_cmdbuffer_t __user *) data, > @@ -648,15 +651,31 @@ static int i915_cmdbuffer(DRM_IOCTL_ARGS) > > LOCK_TEST_WITH_RETURN(dev, filp); > > + DRM_UNLOCK(); > + cliplen = cmdbuf.num_cliprects * sizeof(drm_clip_rect_t); > if (cmdbuf.num_cliprects && > - DRM_VERIFYAREA_READ(cmdbuf.cliprects, > - cmdbuf.num_cliprects * > - sizeof(drm_clip_rect_t))) { > + DRM_VERIFYAREA_READ(cmdbuf.cliprects, cliplen)) { > DRM_ERROR("Fault accessing cliprects\n"); > + DRM_LOCK(); > return DRM_ERR(EFAULT); > } > - > + ret = vslock(cmdbuf.cliprects, cliplen); > + if (ret) { > + DRM_ERROR("Fault wiring cliprects\n"); > + DRM_LOCK(); > + return DRM_ERR(EFAULT); > + } > + ret = vslock(cmdbuf.buf, cmdbuf.sz); > + if (ret) { > + vsunlock(cmdbuf.cliprects, cliplen); > + DRM_ERROR("Fault wiring cmds\n"); > + DRM_LOCK(); > + return DRM_ERR(EFAULT); > + } > + DRM_LOCK(); > ret = i915_dispatch_cmdbuffer(dev, &cmdbuf); > + vsunlock(cmdbuf.buf, cmdbuf.sz); > + vsunlock(cmdbuf.cliprects, cliplen); > if (ret) { > DRM_ERROR("i915_dispatch_cmdbuffer failed\n"); > return ret; > -- The thrill is here, but it won't last long You'd better have your fun before it moves along...