Re: [CHECKER] Probable security holes in 2.6.5

[Chris Wright <chrisw@osdl.org>]

* Ken Ashcraft (ken@coverity.com) wrote:
> [BUG]
> /home/kash/linux/linux-2.6.5/drivers/char/drm/i810_dma.c:1276:i810_dma_mc: ERROR:TAINT: 1267:1276:Using user value "((mc).idx * 4)" without first performing bounds checks [SOURCE_MODEL=(lib,copy_from_user,user,taintscalar)] [PATH= "(*((*dev).lock).hw_lock).lock & -2147483648 == 0" on line 1271 is false => "copy_from_user != 0" on line 1267 is false]    
> 	u32 *hw_status = dev_priv->hw_status_page;
> 	drm_i810_sarea_t *sarea_priv = (drm_i810_sarea_t *)
> 		dev_priv->sarea_priv;
> 	drm_i810_mc_t mc;
> 
> Start --->
> 	if (copy_from_user(&mc, (drm_i810_mc_t *)arg, sizeof(mc)))
> 		return -EFAULT;
> 
> 
> 	if (!_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)) {
> 		DRM_ERROR("i810_dma_mc called without lock held\n");
> 		return -EINVAL;
> 	}
> 
> Error --->
> 	i810_dma_dispatch_mc(dev, dma->buflist[mc.idx], mc.used,
> 		mc.last_render );
> 
> 	atomic_add(mc.used, &dev->counts[_DRM_STAT_SECONDARY]);

Looks like a possible bug.  Index shouldn't go off end of buflist.
Perhaps verifying it's below buf_count would do it.  Patch below.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

===== drivers/char/drm/i810_dma.c 1.31 vs edited =====
--- 1.31/drivers/char/drm/i810_dma.c	Mon Apr 12 10:54:26 2004
+++ edited/drivers/char/drm/i810_dma.c	Fri Apr 16 11:46:32 2004
@@ -1275,6 +1275,9 @@
 		return -EINVAL;
 	}
 
+	if (mc.idx >= dma->buf_count)
+		return -EINVAL;
+
 	i810_dma_dispatch_mc(dev, dma->buflist[mc.idx], mc.used,
 		mc.last_render );