PATCH: 2.6.7-rc3 drivers/char/drm/gamma_dma.c: several user/kernel pointer bugs

PATCH: 2.6.7-rc3 drivers/char/drm/gamma_dma.c: several user/kernel pointer bugs

[Robert T. Johnson]

gamma_dma_priority and gamma_dma_send_buffers both deref d->send_indices
and/or d->send_sizes.  When these functions are called from gamma_dma, 
these pointers are user pointers and are thus not safe to deref.  This patch
copies over the pointers inside gamma_dma_priority and 
gamma_dma_send_buffers.  Let me know if you have any questions or if I've 
made a mistake.

Best,
Rob


--- linux-2.6.7-rc3-full/drivers/char/drm/gamma_dma.c.orig	Wed Jun  9 12:04:35 2004
+++ linux-2.6.7-rc3-full/drivers/char/drm/gamma_dma.c	Wed Jun  9 11:58:42 2004
@@ -346,6 +346,9 @@ static int gamma_dma_priority(struct fil
 	drm_buf_t	  *buf;
 	drm_buf_t	  *last_buf = NULL;
 	drm_device_dma_t  *dma	    = dev->dma;
+	int               *send_indices = NULL;
+	int               *send_sizes = NULL;
+
 	DECLARE_WAITQUEUE(entry, current);
 
 				/* Turn off interrupt handling */
@@ -365,11 +368,31 @@ static int gamma_dma_priority(struct fil
 		++must_free;
 	}
 
+	send_indices = DRM(alloc)(d->send_count * sizeof(*send_indices),
+		                  DRM_MEM_DRIVER);
+        if (send_indices == NULL)
+                return -ENOMEM;
+	if (copy_from_user(send_indices, d->send_indices, 
+			   d->send_count * sizeof(*send_indices))) {
+		retcode = -EFAULT;
+                goto cleanup;
+	}
+	
+	send_sizes = DRM(alloc)(d->send_count * sizeof(*send_sizes),
+		                  DRM_MEM_DRIVER);
+        if (send_sizes == NULL)
+                return -ENOMEM;
+	if (copy_from_user(send_sizes, d->send_sizes, 
+			   d->send_count * sizeof(*send_sizes))) {
+		retcode = -EFAULT;
+                goto cleanup;
+	}
+
 	for (i = 0; i < d->send_count; i++) {
-		idx = d->send_indices[i];
+		idx = send_indices[i];
 		if (idx < 0 || idx >= dma->buf_count) {
 			DRM_ERROR("Index %d (of %d max)\n",
-				  d->send_indices[i], dma->buf_count - 1);
+				  send_indices[i], dma->buf_count - 1);
 			continue;
 		}
 		buf = dma->buflist[ idx ];
@@ -391,7 +414,7 @@ static int gamma_dma_priority(struct fil
 				   process closes the /dev/drm? handle, so
 				   it can't also be doing DMA. */
 		buf->list	  = DRM_LIST_PRIO;
-		buf->used	  = d->send_sizes[i];
+		buf->used	  = send_sizes[i];
 		buf->context	  = d->context;
 		buf->while_locked = d->flags & _DRM_DMA_WHILE_LOCKED;
 		address		  = (unsigned long)buf->address;
@@ -402,14 +425,14 @@ static int gamma_dma_priority(struct fil
 		if (buf->pending) {
 			DRM_ERROR("Sending pending buffer:"
 				  " buffer %d, offset %d\n",
-				  d->send_indices[i], i);
+				  send_indices[i], i);
 			retcode = -EINVAL;
 			goto cleanup;
 		}
 		if (buf->waiting) {
 			DRM_ERROR("Sending waiting buffer:"
 				  " buffer %d, offset %d\n",
-				  d->send_indices[i], i);
+				  send_indices[i], i);
 			retcode = -EINVAL;
 			goto cleanup;
 		}
@@ -458,6 +481,12 @@ cleanup:
 		gamma_dma_ready(dev);
 		gamma_free_buffer(dev, last_buf);
 	}
+        if (send_indices)
+                DRM(free)(send_indices, d->send_count * sizeof(*send_indices), 
+			  DRM_MEM_DRIVER);
+        if (send_sizes)
+                DRM(free)(send_sizes, d->send_count * sizeof(*send_sizes), 
+			  DRM_MEM_DRIVER);
 
 	if (must_free && !dev->context_flag) {
 		if (gamma_lock_free(dev, &dev->lock.hw_lock->lock,
@@ -476,9 +505,13 @@ static int gamma_dma_send_buffers(struct
 	drm_buf_t	  *last_buf = NULL;
 	int		  retcode   = 0;
 	drm_device_dma_t  *dma	    = dev->dma;
+	int               send_index;
+
+	if (get_user(send_index, &d->send_indices[d->send_count-1]))
+		return -EFAULT;
 
 	if (d->flags & _DRM_DMA_BLOCK) {
-		last_buf = dma->buflist[d->send_indices[d->send_count-1]];
+		last_buf = dma->buflist[send_index];
 		add_wait_queue(&last_buf->dma_wait, &entry);
 	}
 

Re: PATCH: 2.6.7-rc3 drivers/char/drm/gamma_dma.c: several user/kernel pointer bugs

[viro]

On Wed, Jun 09, 2004 at 03:53:40PM -0700, Robert T. Johnson wrote:
> gamma_dma_priority and gamma_dma_send_buffers both deref d->send_indices
> and/or d->send_sizes.  When these functions are called from gamma_dma, 
> these pointers are user pointers and are thus not safe to deref.  This patch
> copies over the pointers inside gamma_dma_priority and 
> gamma_dma_send_buffers.  Let me know if you have any questions or if I've 
> made a mistake.

ACK.

Re: PATCH: 2.6.7-rc3 drivers/char/drm/gamma_dma.c: several user/kernel pointer bugs

[Dave Airlie]

I'll fix this in the DRM bk tree and push to Linus...

Dave.

On Thu, 10 Jun 2004 viro@parcelfarce.linux.theplanet.co.uk wrote:

> On Wed, Jun 09, 2004 at 03:53:40PM -0700, Robert T. Johnson wrote:
> > gamma_dma_priority and gamma_dma_send_buffers both deref d->send_indices
> > and/or d->send_sizes.  When these functions are called from gamma_dma,
> > these pointers are user pointers and are thus not safe to deref.  This patch
> > copies over the pointers inside gamma_dma_priority and
> > gamma_dma_send_buffers.  Let me know if you have any questions or if I've
> > made a mistake.
>
> ACK.
>
>
> -------------------------------------------------------
> This SF.Net email is sponsored by: GNOME Foundation
> Hackers Unite!  GUADEC: The world's #1 Open Source Desktop Event.
> GNOME Users and Developers European Conference, 28-30th June in Norway
> http://2004/guadec.org
> --
> _______________________________________________
> Dri-devel mailing list
> Dri-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/dri-devel
>

-- 
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
pam_smb / Linux DECstation / Linux VAX / ILUG person

Re: PATCH: 2.6.7-rc3 drivers/char/drm/gamma_dma.c: several user/kernel pointer bugs

[Dave Airlie]

> gamma_dma_priority and gamma_dma_send_buffers both deref d->send_indices
> and/or d->send_sizes.  When these functions are called from gamma_dma,
> these pointers are user pointers and are thus not safe to deref.  This patch
> copies over the pointers inside gamma_dma_priority and
> gamma_dma_send_buffers.  Let me know if you have any questions or if I've
> made a mistake.

okay I've checked this into the drm bk tree and DRM CVS, I've no way to
test it apart from visual inspection and it compiles, I've asked Linus to
sync the drm tree again, I probably need to add some __user annotations in
a few places..

Dave.

-- 
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
pam_smb / Linux DECstation / Linux VAX / ILUG person

Re: PATCH: 2.6.7-rc3 drivers/char/drm/gamma_dma.c: several user/kernel pointer bugs

[Alan Cox]

On Iau, 2004-06-10 at 10:46, Dave Airlie wrote:
> okay I've checked this into the drm bk tree and DRM CVS, I've no way to
> test it apart from visual inspection and it compiles, I've asked Linus to
> sync the drm tree again, I probably need to add some __user annotations in
> a few places..

I've got an Oxygen GMX somewhere, but last time I investigated the
drivers didn't work anyway. Not that fixing the security bits isnt a
good idea regardless.

I'll try and test it at some point