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);
}
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.
I'll fix this in the DRM bk tree and push to Linus...
Dave.
On Thu, 10 Jun 2004 [email protected] 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
> [email protected]
> 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
> 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
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