Messages in this thread Patch in this message | | | Date | Wed, 21 Apr 2004 03:34:02 +0200 | From | Andrea Arcangeli <> | Subject | Re: [CHECKER] Probable security holes in 2.6.5 |
| |
On Fri, Apr 16, 2004 at 11:54:06AM -0700, Chris Wright wrote: > * 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 );
this is wrong, idx is signed, so you've to check for negative values too. Credit for noticing this doesn't belong to me though.
Could you just in case review the other fixes too for other potential errors like this? thanks.
this is the correct fix for 2.6 (and it seems 2.4 too):
--- linux/drivers/char/drm/i810_dma.c Mon Apr 12 10:54:26 2004 +++ linux/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 || mc.idx < 0) + return -EINVAL; + i810_dma_dispatch_mc(dev, dma->buflist[mc.idx], mc.used, mc.last_render ); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |