Hey guys, this is from the Stanford checker guys, forwarded from an original posting on linux-kernel. Looks like 4.1.0 DRI in the mga and i810 drivers at least may have some security issues whereby it trusts data copied from userspace and uses it as array indicies or for other purposes. No doubt there are other areas that they haven't caught yet also. I looked at the code to try and figure out how it might be fixed, but nothing was obvious to me as I'm not too familiar with the kernel, nor DRI. Any suggestions on how to clean this up? ---------------------------------------------------------------------- Mike A. Harris Shipping/mailing address: OS Systems Engineer 190 Pittsburgh Ave., Sault Ste. Marie, Red Hat Inc. Ontario, Canada, P6C 5B3 http://www.redhat.com Phone: (705)949-2136 ---------------------------------------------------------------------- Latest XFree86 test RPMS: ftp://people.redhat.com/mharris/testing ---------- Forwarded message ---------- Date: Fri, 8 Jun 2001 22:58:59 +0100 (BST) From: Alan Cox <[EMAIL PROTECTED]> To: [EMAIL PROTECTED] Content-Type: text/plain; charset=us-ascii Subject: I suspect the new 4.1 DRI has the dri holes listed here.. Forwarded message: > From [EMAIL PROTECTED] Fri Jun 08 22:35:28 2001 > Envelope-to: [EMAIL PROTECTED] > Delivery-date: Fri, 08 Jun 2001 22:35:28 +0100 > From: Dawson Engler <[EMAIL PROTECTED]> > Message-Id: <[EMAIL PROTECTED]> > Subject: [CHECKER] 15 probable security holes in 2.4.5-ac8 > To: [EMAIL PROTECTED] > Date: Fri, 8 Jun 2001 14:34:01 -0700 (PDT) > Cc: [EMAIL PROTECTED] > X-Mailer: ELM [version 2.5 PL1] > MIME-Version: 1.0 > Content-Type: text/plain; charset=us-ascii > Content-Transfer-Encoding: 7bit > Sender: [EMAIL PROTECTED] > Precedence: bulk > X-Mailing-List: [EMAIL PROTECTED] > > Hi All, > > here are 15 probable security holes where user input (e.g. data from > copy_from_user, get_user, etc) is: > > 1. passed as a length argument to copy_*user (or passed to a > function that does so), OR > > 2. is used as an array index. > > The main difference between this and past checkers is that we do > "inherited" tainting: if a structure is copied in from user space, > we recursively mark all of it's fields as tainted as well. I'm still > fussing with the extension, so hopefully find a bunch more errors after > this batch. > > You can look at this checker as essentially tracking whether the > information from an untrusted source (e.g., from copy_from_user) can reach > a trusting sink (e.g., an array index). The main limiting factor on its > effectiveness is that we have a very incomplete notion of trusting sinks. > If anyone has suggestions for other general places where it's dangerous > to consume bad data, please send me an email. > > Here's the location summary: > > # Total = 15 > # BUGs | File Name > 3 | drivers/char/drm/mga_state.c > 2 | drivers/scsi/megaraid.c > 2 | drivers/char/drm/i810_dma.c > 2 | drivers/char/raw.c > 1 | drivers/usb/usbvideo.c > 1 | drivers/usb/se401.c > 1 | drivers/net/hamradio/scc.c > 1 | drivers/char/moxa.c > 1 | drivers/media/video/zr36120.c > 1 | drivers/media/video/zr36067.c > > Dawson > --------------------------------------------------------- > [BUG] d.idx is an int: < 0 = bad news. > /u2/engler/mc/oses/linux/2.4.5-ac8/drivers/char/drm/i810_dma.c:1413:i810_copybuf: >ERROR:RANGE:1409:1413: Using user length "idx"as an array index for "buflist" set by >'copy_from_user':1409 [val=400] > if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) { > DRM_ERROR("i810_dma called without lock held\n"); > return -EINVAL; > } > > Start ---> > if (copy_from_user(&d, (drm_i810_copy_t *)arg, sizeof(d))) > return -EFAULT; > > if(d.idx > dma->buf_count) return -EINVAL; > Error ---> > buf = dma->buflist[ d.idx ]; > buf_priv = buf->dev_private; > if (buf_priv->currently_mapped != I810_BUF_MAPPED) return -EPERM; > > --------------------------------------------------------- > [BUG] ouch x 2: no check either way. > /u2/engler/mc/oses/linux/2.4.5-ac8/drivers/char/drm/mga_state.c:835:mga_iload: >ERROR:RANGE:827:835: Using user length "idx"as an array index for "buflist" set by >'copy_from_user':827 [val=800] > drm_buf_t *buf; > drm_mga_buf_priv_t *buf_priv; > drm_mga_iload_t iload; > unsigned long bus_address; > > Start ---> > if (copy_from_user(&iload, (drm_mga_iload_t *) arg, sizeof(iload))) > return -EFAULT; > > if (!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) { > DRM_ERROR("mga_iload called without lock held\n"); > return -EINVAL; > } > > Error ---> > buf = dma->buflist[iload.idx]; > buf_priv = buf->dev_private; > bus_address = buf->bus_address; > > --------------------------------------------------------- > [BUG] > /u2/engler/mc/oses/linux/2.4.5-ac8/drivers/char/drm/mga_state.c:923:mga_indices: >ERROR:RANGE:915:923: Using user length "idx"as an array index for "buflist" set by >'copy_from_user':915 [val=800] > drm_buf_t *buf; > drm_mga_buf_priv_t *buf_priv; > drm_mga_indices_t indices; > > if (copy_from_user(&indices, > Start ---> > (drm_mga_indices_t *)arg, sizeof(indices))) > return -EFAULT; > > if (!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) { > DRM_ERROR("mga_indices called without lock held\n"); > return -EINVAL; > } > > Error ---> > buf = dma->buflist[indices.idx]; > buf_priv = buf->dev_private; > > buf_priv->discard = indices.discard; > --------------------------------------------------------- > [BUG] > /u2/engler/mc/oses/linux/2.4.5-ac8/drivers/char/drm/mga_state.c:877:mga_vertex: >ERROR:RANGE:869:877: Using user length "idx"as an array index for "buflist" set by >'copy_from_user':869 [val=800] > drm_device_dma_t *dma = dev->dma; > drm_buf_t *buf; > drm_mga_buf_priv_t *buf_priv; > drm_mga_vertex_t vertex; > > Start ---> > if (copy_from_user(&vertex, (drm_mga_vertex_t *) arg, sizeof(vertex))) > return -EFAULT; > > if (!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) { > DRM_ERROR("mga_vertex called without lock held\n"); > return -EINVAL; > } > > Error ---> > buf = dma->buflist[vertex.idx]; > buf_priv = buf->dev_private; > > buf->used = vertex.used; > --------------------------------------------------------- > [BUG] > /u2/engler/mc/oses/linux/2.4.5-ac8/drivers/char/drm/i810_dma.c:1291:i810_dma_vertex: >ERROR:RANGE:1278:1291: Using user length "idx"as an array index for "buflist" set by >'copy_from_user':1278 [val=1300] > u32 *hw_status = (u32 *)dev_priv->hw_status_page; > drm_i810_sarea_t *sarea_priv = (drm_i810_sarea_t *) > dev_priv->sarea_priv; > drm_i810_vertex_t vertex; > > Start ---> > if (copy_from_user(&vertex, (drm_i810_vertex_t *)arg, sizeof(vertex))) > return -EFAULT; > > if(!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) { > DRM_ERROR("i810_dma_vertex called without lock held\n"); > return -EINVAL; > } > > DRM_DEBUG("i810 dma vertex, idx %d used %d discard %d\n", > vertex.idx, vertex.used, vertex.discard); > > i810_dma_dispatch_vertex( dev, > dma->buflist[ vertex.idx ], > Error ---> > vertex.discard, vertex.used ); > > atomic_add(vertex.used, &dma->total_bytes); > atomic_inc(&dma->total_dmas); > --------------------------------------------------------- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to [EMAIL PROTECTED] > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > _______________________________________________ Dri-devel mailing list [EMAIL PROTECTED] http://lists.sourceforge.net/lists/listinfo/dri-devel
