PATCH: 2.6.7-rc3 drivers/char/drm/gamma_dma.c: several user/kernel pointer bugs
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
drivers/char/drm/gamma_dma.c renamed from drivers/char/drm/gamma_dma.c.orig +39 -6
@@ -346,6 +346,9 @@ static int gamma_dma_priority(struct fildrm_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 filprocess 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 filif (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(structdrm_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); }
Quoted 6 lines · Robert T. Johnson
gamma_dma_priorityandgamma_dma_send_buffersboth derefd->send_indicesand/ord->send_sizes. When these functions are called fromgamma_dma, these pointers are user pointers and are thus not safe to deref. This patch copies over the pointers insidegamma_dma_priorityandgamma_dma_send_buffers. Let me know if you have any questions or if I've made a mistake.
ACK.
I'll fix this in the DRM bk tree and push to Linus...
Dave.
Quoted 22 lines · On Thu, 10 Jun 2004 [email protected] wrote:
Quoted 6 lines · Robert T. Johnson
gamma_dma_priorityandgamma_dma_send_buffersboth derefd->send_indicesand/ord->send_sizes. When these functions are called fromgamma_dma, these pointers are user pointers and are thus not safe to deref. This patch copies over the pointers insidegamma_dma_priorityandgamma_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
[email protected]
https://lists.sourceforge.net/lists/listinfo/dri-devel
Signature · 3 lines
David Airlie, Software Engineer http://www.skynet.ie/~airlied / airlied at skynet.ie pam_smb / Linux DECstation / Linux VAX / ILUG person
Quoted 7 lines
gamma_dma_priorityandgamma_dma_send_buffersboth derefd->send_indicesand/ord->send_sizes. When these functions are called fromgamma_dma, these pointers are user pointers and are thus not safe to deref. This patch copies over the pointers insidegamma_dma_priorityandgamma_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.
Signature · 3 lines
David Airlie, Software Engineer http://www.skynet.ie/~airlied / airlied at skynet.ie pam_smb / Linux DECstation / Linux VAX / ILUG person
Quoted 4 lines · Dave Airlie
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
__userannotations 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