Date: Mon, 16 Jul 2007 18:57:59 +0800 From: Ganbold <ganbold@micom.mng.net> To: Kostik Belousov <kostikbel@gmail.com> Cc: x11@freebsd.org Subject: Re: LOR when running google-earth Message-ID: <469B4F37.1030405@micom.mng.net> In-Reply-To: <20070716105316.GT2200@deviant.kiev.zoral.com.ua> References: <469B17BB.9050305@micom.mng.net> <20070716105316.GT2200@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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...
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?469B4F37.1030405>