From 6481a2e4cda67732ce7c6fe30aa4dc50d3cc7ed0 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Tue, 8 Nov 2005 21:40:03 +0000 Subject: Correct a LOR issue on FreeBSD by allocating temporary space and doing a single DRM_COPY_FROM_USER rather than DRM_VERIFYAREA_READ followed by tons of DRM_COPY_FROM_USER_UNCHECKED. I don't like the look of the temporary space allocation, but I like the simplification in the rest of the file. Tested with glxgears, tuxracer, and q3 on a savage4. --- shared-core/savage_state.c | 270 ++++++++++++++++++++++++--------------------- 1 file changed, 144 insertions(+), 126 deletions(-) (limited to 'shared-core/savage_state.c') diff --git a/shared-core/savage_state.c b/shared-core/savage_state.c index 475695a0..e6e3ea7f 100644 --- a/shared-core/savage_state.c +++ b/shared-core/savage_state.c @@ -27,7 +27,7 @@ #include "savage_drv.h" void savage_emit_clip_rect_s3d(drm_savage_private_t *dev_priv, - drm_clip_rect_t *pbox) + const drm_clip_rect_t *pbox) { uint32_t scstart = dev_priv->state.s3d.new_scstart; uint32_t scend = dev_priv->state.s3d.new_scend; @@ -53,7 +53,7 @@ void savage_emit_clip_rect_s3d(drm_savage_private_t *dev_priv, } void savage_emit_clip_rect_s4(drm_savage_private_t *dev_priv, - drm_clip_rect_t *pbox) + const drm_clip_rect_t *pbox) { uint32_t drawctrl0 = dev_priv->state.s4.new_drawctrl0; uint32_t drawctrl1 = dev_priv->state.s4.new_drawctrl1; @@ -113,18 +113,18 @@ static int savage_verify_texaddr(drm_savage_private_t *dev_priv, int unit, #define SAVE_STATE(reg,where) \ if(start <= reg && start+count > reg) \ - DRM_GET_USER_UNCHECKED(dev_priv->state.where, ®s[reg-start]) + dev_priv->state.where = regs[reg - start] #define SAVE_STATE_MASK(reg,where,mask) do { \ if(start <= reg && start+count > reg) { \ uint32_t tmp; \ - DRM_GET_USER_UNCHECKED(tmp, ®s[reg-start]); \ + tmp = regs[reg - start]; \ dev_priv->state.where = (tmp & (mask)) | \ (dev_priv->state.where & ~(mask)); \ } \ } while (0) static int savage_verify_state_s3d(drm_savage_private_t *dev_priv, unsigned int start, unsigned int count, - const uint32_t __user *regs) + const uint32_t *regs) { if (start < SAVAGE_TEXPALADDR_S3D || start+count-1 > SAVAGE_DESTTEXRWWATERMARK_S3D) { @@ -154,7 +154,7 @@ static int savage_verify_state_s3d(drm_savage_private_t *dev_priv, static int savage_verify_state_s4(drm_savage_private_t *dev_priv, unsigned int start, unsigned int count, - const uint32_t __user *regs) + const uint32_t *regs) { int ret = 0; @@ -192,7 +192,7 @@ static int savage_verify_state_s4(drm_savage_private_t *dev_priv, static int savage_dispatch_state(drm_savage_private_t *dev_priv, const drm_savage_cmd_header_t *cmd_header, - const uint32_t __user *regs) + const uint32_t *regs) { unsigned int count = cmd_header->state.count; unsigned int start = cmd_header->state.start; @@ -204,9 +204,6 @@ static int savage_dispatch_state(drm_savage_private_t *dev_priv, if (!count) return 0; - if (DRM_VERIFYAREA_READ(regs, count*4)) - return DRM_ERR(EFAULT); - if (S3_SAVAGE3D_SERIES(dev_priv->chipset)) { ret = savage_verify_state_s3d(dev_priv, start, count, regs); if (ret != 0) @@ -257,7 +254,7 @@ static int savage_dispatch_state(drm_savage_private_t *dev_priv, while (count > 0) { unsigned int n = count < 255 ? count : 255; DMA_SET_REGISTERS(start, n); - DMA_COPY_FROM_USER(regs, n); + DMA_COPY(regs, n); count -= n; start += n; regs += n; @@ -415,8 +412,7 @@ static int savage_dispatch_dma_prim(drm_savage_private_t *dev_priv, static int savage_dispatch_vb_prim(drm_savage_private_t *dev_priv, const drm_savage_cmd_header_t *cmd_header, - const uint32_t __user *vtxbuf, - unsigned int vb_size, + const uint32_t *vtxbuf, unsigned int vb_size, unsigned int vb_stride) { unsigned char reorder = 0; @@ -501,8 +497,7 @@ static int savage_dispatch_vb_prim(drm_savage_private_t *dev_priv, for (i = start; i < start+count; ++i) { unsigned int j = i + reorder[i % 3]; - DMA_COPY_FROM_USER(&vtxbuf[vb_stride*j], - vtx_size); + DMA_COPY(&vtxbuf[vb_stride*j], vtx_size); } DMA_COMMIT(); @@ -511,13 +506,12 @@ static int savage_dispatch_vb_prim(drm_savage_private_t *dev_priv, DMA_DRAW_PRIMITIVE(count, prim, skip); if (vb_stride == vtx_size) { - DMA_COPY_FROM_USER(&vtxbuf[vb_stride*start], - vtx_size*count); + DMA_COPY(&vtxbuf[vb_stride*start], + vtx_size*count); } else { for (i = start; i < start+count; ++i) { - DMA_COPY_FROM_USER( - &vtxbuf[vb_stride*i], - vtx_size); + DMA_COPY(&vtxbuf[vb_stride*i], + vtx_size); } } @@ -535,7 +529,7 @@ static int savage_dispatch_vb_prim(drm_savage_private_t *dev_priv, static int savage_dispatch_dma_idx(drm_savage_private_t *dev_priv, const drm_savage_cmd_header_t *cmd_header, - const uint16_t __user *usr_idx, + const uint16_t *idx, const drm_buf_t *dmabuf) { unsigned char reorder = 0; @@ -625,11 +619,8 @@ static int savage_dispatch_dma_idx(drm_savage_private_t *dev_priv, while (n != 0) { /* Can emit up to 255 indices (85 triangles) at once. */ unsigned int count = n > 255 ? 255 : n; - /* Is it ok to allocate 510 bytes on the stack in an ioctl? */ - uint16_t idx[255]; - /* Copy and check indices */ - DRM_COPY_FROM_USER_UNCHECKED(idx, usr_idx, count*2); + /* check indices */ for (i = 0; i < count; ++i) { if (idx[i] > dmabuf->total/32) { DRM_ERROR("idx[%u]=%u out of range (0-%u)\n", @@ -670,7 +661,7 @@ static int savage_dispatch_dma_idx(drm_savage_private_t *dev_priv, BCI_WRITE(idx[i]); } - usr_idx += count; + idx += count; n -= count; prim |= BCI_CMD_DRAW_CONT; @@ -681,8 +672,8 @@ static int savage_dispatch_dma_idx(drm_savage_private_t *dev_priv, static int savage_dispatch_vb_idx(drm_savage_private_t *dev_priv, const drm_savage_cmd_header_t *cmd_header, - const uint16_t __user *usr_idx, - const uint32_t __user *vtxbuf, + const uint16_t *idx, + const uint32_t *vtxbuf, unsigned int vb_size, unsigned int vb_stride) { @@ -749,11 +740,8 @@ static int savage_dispatch_vb_idx(drm_savage_private_t *dev_priv, while (n != 0) { /* Can emit up to 255 vertices (85 triangles) at once. */ unsigned int count = n > 255 ? 255 : n; - /* Is it ok to allocate 510 bytes on the stack in an ioctl? */ - uint16_t idx[255]; - /* Copy and check indices */ - DRM_COPY_FROM_USER_UNCHECKED(idx, usr_idx, count*2); + /* Check indices */ for (i = 0; i < count; ++i) { if (idx[i] > vb_size / (vb_stride*4)) { DRM_ERROR("idx[%u]=%u out of range (0-%u)\n", @@ -773,8 +761,7 @@ static int savage_dispatch_vb_idx(drm_savage_private_t *dev_priv, for (i = 0; i < count; ++i) { unsigned int j = idx[i + reorder[i % 3]]; - DMA_COPY_FROM_USER(&vtxbuf[vb_stride*j], - vtx_size); + DMA_COPY(&vtxbuf[vb_stride*j], vtx_size); } DMA_COMMIT(); @@ -784,14 +771,13 @@ static int savage_dispatch_vb_idx(drm_savage_private_t *dev_priv, for (i = 0; i < count; ++i) { unsigned int j = idx[i]; - DMA_COPY_FROM_USER(&vtxbuf[vb_stride*j], - vtx_size); + DMA_COPY(&vtxbuf[vb_stride*j], vtx_size); } DMA_COMMIT(); } - usr_idx += count; + idx += count; n -= count; prim |= BCI_CMD_DRAW_CONT; @@ -802,11 +788,11 @@ static int savage_dispatch_vb_idx(drm_savage_private_t *dev_priv, static int savage_dispatch_clear(drm_savage_private_t *dev_priv, const drm_savage_cmd_header_t *cmd_header, - const drm_savage_cmd_header_t __user *data, + const drm_savage_cmd_header_t *data, unsigned int nbox, - const drm_clip_rect_t __user *usr_boxes) + const drm_clip_rect_t *boxes) { - unsigned int flags = cmd_header->clear0.flags, mask, value; + unsigned int flags = cmd_header->clear0.flags; unsigned int clear_cmd; unsigned int i, nbufs; DMA_LOCALS; @@ -814,11 +800,6 @@ static int savage_dispatch_clear(drm_savage_private_t *dev_priv, if (nbox == 0) return 0; - DRM_GET_USER_UNCHECKED(mask, &((const drm_savage_cmd_header_t*)data) - ->clear1.mask); - DRM_GET_USER_UNCHECKED(value, &((const drm_savage_cmd_header_t*)data) - ->clear1.value); - clear_cmd = BCI_CMD_RECT | BCI_CMD_RECT_XP | BCI_CMD_RECT_YP | BCI_CMD_SEND_COLOR | BCI_CMD_DEST_PBD_NEW; BCI_CMD_SET_ROP(clear_cmd,0xCC); @@ -829,21 +810,20 @@ static int savage_dispatch_clear(drm_savage_private_t *dev_priv, if (nbufs == 0) return 0; - if (mask != 0xffffffff) { + if (data->clear1.mask != 0xffffffff) { /* set mask */ BEGIN_DMA(2); DMA_SET_REGISTERS(SAVAGE_BITPLANEWTMASK, 1); - DMA_WRITE(mask); + DMA_WRITE(data->clear1.mask); DMA_COMMIT(); } for (i = 0; i < nbox; ++i) { - drm_clip_rect_t box; unsigned int x, y, w, h; unsigned int buf; - DRM_COPY_FROM_USER_UNCHECKED(&box, &usr_boxes[i], sizeof(box)); - x = box.x1, y = box.y1; - w = box.x2 - box.x1; - h = box.y2 - box.y1; + + x = boxes[i].x1, y = boxes[i].y1; + w = boxes[i].x2 - boxes[i].x1; + h = boxes[i].y2 - boxes[i].y1; BEGIN_DMA(nbufs*6); for (buf = SAVAGE_FRONT; buf <= SAVAGE_DEPTH; buf <<= 1) { if (!(flags & buf)) @@ -863,13 +843,13 @@ static int savage_dispatch_clear(drm_savage_private_t *dev_priv, DMA_WRITE(dev_priv->depth_bd); break; } - DMA_WRITE(value); + DMA_WRITE(data->clear1.value); DMA_WRITE(BCI_X_Y(x, y)); DMA_WRITE(BCI_W_H(w, h)); } DMA_COMMIT(); } - if (mask != 0xffffffff) { + if (data->clear1.mask != 0xffffffff) { /* reset mask */ BEGIN_DMA(2); DMA_SET_REGISTERS(SAVAGE_BITPLANEWTMASK, 1); @@ -881,8 +861,7 @@ static int savage_dispatch_clear(drm_savage_private_t *dev_priv, } static int savage_dispatch_swap(drm_savage_private_t *dev_priv, - unsigned int nbox, - const drm_clip_rect_t __user *usr_boxes) + unsigned int nbox, const drm_clip_rect_t *boxes) { unsigned int swap_cmd; unsigned int i; @@ -896,16 +875,14 @@ static int savage_dispatch_swap(drm_savage_private_t *dev_priv, BCI_CMD_SET_ROP(swap_cmd,0xCC); for (i = 0; i < nbox; ++i) { - drm_clip_rect_t box; - DRM_COPY_FROM_USER_UNCHECKED(&box, &usr_boxes[i], sizeof(box)); - BEGIN_DMA(6); DMA_WRITE(swap_cmd); DMA_WRITE(dev_priv->back_offset); DMA_WRITE(dev_priv->back_bd); - DMA_WRITE(BCI_X_Y(box.x1, box.y1)); - DMA_WRITE(BCI_X_Y(box.x1, box.y1)); - DMA_WRITE(BCI_W_H(box.x2-box.x1, box.y2-box.y1)); + DMA_WRITE(BCI_X_Y(boxes[i].x1, boxes[i].y1)); + DMA_WRITE(BCI_X_Y(boxes[i].x1, boxes[i].y1)); + DMA_WRITE(BCI_W_H(boxes[i].x2-boxes[i].x1, + boxes[i].y2-boxes[i].y1)); DMA_COMMIT(); } @@ -913,29 +890,26 @@ static int savage_dispatch_swap(drm_savage_private_t *dev_priv, } static int savage_dispatch_draw(drm_savage_private_t *dev_priv, - const drm_savage_cmd_header_t __user *start, - const drm_savage_cmd_header_t __user *end, + const drm_savage_cmd_header_t *start, + const drm_savage_cmd_header_t *end, const drm_buf_t *dmabuf, - const unsigned int __user *usr_vtxbuf, + const unsigned int *vtxbuf, unsigned int vb_size, unsigned int vb_stride, unsigned int nbox, - const drm_clip_rect_t __user *usr_boxes) + const drm_clip_rect_t *boxes) { unsigned int i, j; int ret; for (i = 0; i < nbox; ++i) { - drm_clip_rect_t box; - const drm_savage_cmd_header_t __user *usr_cmdbuf; - DRM_COPY_FROM_USER_UNCHECKED(&box, &usr_boxes[i], sizeof(box)); - dev_priv->emit_clip_rect(dev_priv, &box); + const drm_savage_cmd_header_t *cmdbuf; + dev_priv->emit_clip_rect(dev_priv, &boxes[i]); - usr_cmdbuf = start; - while (usr_cmdbuf < end) { + cmdbuf = start; + while (cmdbuf < end) { drm_savage_cmd_header_t cmd_header; - DRM_COPY_FROM_USER_UNCHECKED(&cmd_header, usr_cmdbuf, - sizeof(cmd_header)); - usr_cmdbuf++; + cmd_header = *cmdbuf; + cmdbuf++; switch (cmd_header.cmd.cmd) { case SAVAGE_CMD_DMA_PRIM: ret = savage_dispatch_dma_prim( @@ -944,27 +918,24 @@ static int savage_dispatch_draw(drm_savage_private_t *dev_priv, case SAVAGE_CMD_VB_PRIM: ret = savage_dispatch_vb_prim( dev_priv, &cmd_header, - (const uint32_t __user *)usr_vtxbuf, - vb_size, vb_stride); + vtxbuf, vb_size, vb_stride); break; case SAVAGE_CMD_DMA_IDX: j = (cmd_header.idx.count + 3) / 4; /* j was check in savage_bci_cmdbuf */ - ret = savage_dispatch_dma_idx( - dev_priv, &cmd_header, - (const uint16_t __user *)usr_cmdbuf, + ret = savage_dispatch_dma_idx(dev_priv, + &cmd_header, (const uint16_t *)cmdbuf, dmabuf); - usr_cmdbuf += j; + cmdbuf += j; break; case SAVAGE_CMD_VB_IDX: j = (cmd_header.idx.count + 3) / 4; /* j was check in savage_bci_cmdbuf */ - ret = savage_dispatch_vb_idx( - dev_priv, &cmd_header, - (const uint16_t __user *)usr_cmdbuf, - (const uint32_t __user *)usr_vtxbuf, - vb_size, vb_stride); - usr_cmdbuf += j; + ret = savage_dispatch_vb_idx(dev_priv, + &cmd_header, (const uint16_t *)cmdbuf, + (const uint32_t *)vtxbuf, vb_size, + vb_stride); + cmdbuf += j; break; default: /* What's the best return code? EFAULT? */ @@ -989,10 +960,10 @@ int savage_bci_cmdbuf(DRM_IOCTL_ARGS) drm_device_dma_t *dma = dev->dma; drm_buf_t *dmabuf; drm_savage_cmdbuf_t cmdbuf; - drm_savage_cmd_header_t __user *usr_cmdbuf; - drm_savage_cmd_header_t __user *first_draw_cmd; - unsigned int __user *usr_vtxbuf; - drm_clip_rect_t __user *usr_boxes; + drm_savage_cmd_header_t *kcmd_addr = NULL; + drm_savage_cmd_header_t *first_draw_cmd; + unsigned int *kvb_addr = NULL; + drm_clip_rect_t *kbox_addr = NULL; unsigned int i, j; int ret = 0; @@ -1014,15 +985,53 @@ int savage_bci_cmdbuf(DRM_IOCTL_ARGS) dmabuf = NULL; } - usr_cmdbuf = (drm_savage_cmd_header_t __user *)cmdbuf.cmd_addr; - usr_vtxbuf = (unsigned int __user *)cmdbuf.vb_addr; - usr_boxes = (drm_clip_rect_t __user *)cmdbuf.box_addr; - if ((cmdbuf.size && DRM_VERIFYAREA_READ(usr_cmdbuf, cmdbuf.size*8)) || - (cmdbuf.vb_size && DRM_VERIFYAREA_READ( - usr_vtxbuf, cmdbuf.vb_size)) || - (cmdbuf.nbox && DRM_VERIFYAREA_READ( - usr_boxes, cmdbuf.nbox*sizeof(drm_clip_rect_t)))) - return DRM_ERR(EFAULT); + /* Copy the user buffers into kernel temporary areas. This hasn't been + * a performance loss compared to VERIFYAREA_READ/ + * COPY_FROM_USER_UNCHECKED when done in other drivers, and is correct + * for locking on FreeBSD. + */ + if (cmdbuf.size) { + kcmd_addr = drm_alloc(cmdbuf.size * 8, DRM_MEM_DRIVER); + if (kcmd_addr == NULL) + return ENOMEM; + + if (DRM_COPY_FROM_USER(kcmd_addr, cmdbuf.cmd_addr, + cmdbuf.size * 8)) + { + drm_free(kcmd_addr, cmdbuf.size * 8, DRM_MEM_DRIVER); + return DRM_ERR(EFAULT); + } + cmdbuf.cmd_addr = kcmd_addr; + } + if (cmdbuf.vb_size) { + kvb_addr = drm_alloc(cmdbuf.vb_size, DRM_MEM_DRIVER); + if (kvb_addr == NULL) { + ret = DRM_ERR(ENOMEM); + goto done; + } + + if (DRM_COPY_FROM_USER(kvb_addr, cmdbuf.vb_addr, + cmdbuf.vb_size)) { + ret = DRM_ERR(EFAULT); + goto done; + } + cmdbuf.vb_addr = kvb_addr; + } + if (cmdbuf.nbox) { + kbox_addr = drm_alloc(cmdbuf.nbox * sizeof(drm_clip_rect_t), + DRM_MEM_DRIVER); + if (kbox_addr == NULL) { + ret = DRM_ERR(ENOMEM); + goto done; + } + + if (DRM_COPY_FROM_USER(kbox_addr, cmdbuf.box_addr, + cmdbuf.nbox * sizeof(drm_clip_rect_t))) { + ret = DRM_ERR(EFAULT); + goto done; + } + cmdbuf.box_addr = kbox_addr; + } /* Make sure writes to DMA buffers are finished before sending * DMA commands to the graphics hardware. */ @@ -1036,9 +1045,8 @@ int savage_bci_cmdbuf(DRM_IOCTL_ARGS) first_draw_cmd = NULL; while (i < cmdbuf.size) { drm_savage_cmd_header_t cmd_header; - DRM_COPY_FROM_USER_UNCHECKED(&cmd_header, usr_cmdbuf, - sizeof(cmd_header)); - usr_cmdbuf++; + cmd_header = *(drm_savage_cmd_header_t *)cmdbuf.cmd_addr; + cmdbuf.cmd_addr++; i++; /* Group drawing commands with same state to minimize @@ -1058,17 +1066,18 @@ int savage_bci_cmdbuf(DRM_IOCTL_ARGS) case SAVAGE_CMD_DMA_PRIM: case SAVAGE_CMD_VB_PRIM: if (!first_draw_cmd) - first_draw_cmd = usr_cmdbuf-1; - usr_cmdbuf += j; + first_draw_cmd = cmdbuf.cmd_addr-1; + cmdbuf.cmd_addr += j; i += j; break; default: if (first_draw_cmd) { ret = savage_dispatch_draw ( - dev_priv, first_draw_cmd, usr_cmdbuf-1, - dmabuf, usr_vtxbuf, cmdbuf.vb_size, + dev_priv, first_draw_cmd, + cmdbuf.cmd_addr-1, + dmabuf, cmdbuf.vb_addr, cmdbuf.vb_size, cmdbuf.vb_stride, - cmdbuf.nbox, usr_boxes); + cmdbuf.nbox, cmdbuf.box_addr); if (ret != 0) return ret; first_draw_cmd = NULL; @@ -1084,12 +1093,12 @@ int savage_bci_cmdbuf(DRM_IOCTL_ARGS) DRM_ERROR("command SAVAGE_CMD_STATE extends " "beyond end of command buffer\n"); DMA_FLUSH(); - return DRM_ERR(EINVAL); + ret = DRM_ERR(EINVAL); + goto done; } - ret = savage_dispatch_state( - dev_priv, &cmd_header, - (uint32_t __user *)usr_cmdbuf); - usr_cmdbuf += j; + ret = savage_dispatch_state(dev_priv, &cmd_header, + (const uint32_t *)cmdbuf.cmd_addr); + cmdbuf.cmd_addr += j; i += j; break; case SAVAGE_CMD_CLEAR: @@ -1097,38 +1106,40 @@ int savage_bci_cmdbuf(DRM_IOCTL_ARGS) DRM_ERROR("command SAVAGE_CMD_CLEAR extends " "beyond end of command buffer\n"); DMA_FLUSH(); - return DRM_ERR(EINVAL); + ret = DRM_ERR(EINVAL); + goto done; } ret = savage_dispatch_clear(dev_priv, &cmd_header, - usr_cmdbuf, - cmdbuf.nbox, usr_boxes); - usr_cmdbuf++; + cmdbuf.cmd_addr, + cmdbuf.nbox, cmdbuf.box_addr); + cmdbuf.cmd_addr++; i++; break; case SAVAGE_CMD_SWAP: - ret = savage_dispatch_swap(dev_priv, - cmdbuf.nbox, usr_boxes); + ret = savage_dispatch_swap(dev_priv, cmdbuf.nbox, + cmdbuf.box_addr); break; default: DRM_ERROR("invalid command 0x%x\n", cmd_header.cmd.cmd); DMA_FLUSH(); - return DRM_ERR(EINVAL); + ret = DRM_ERR(EINVAL); + goto done; } if (ret != 0) { DMA_FLUSH(); - return ret; + goto done; } } if (first_draw_cmd) { ret = savage_dispatch_draw ( - dev_priv, first_draw_cmd, usr_cmdbuf, dmabuf, - usr_vtxbuf, cmdbuf.vb_size, cmdbuf.vb_stride, - cmdbuf.nbox, usr_boxes); + dev_priv, first_draw_cmd, cmdbuf.cmd_addr, dmabuf, + cmdbuf.vb_addr, cmdbuf.vb_size, cmdbuf.vb_stride, + cmdbuf.nbox, cmdbuf.box_addr); if (ret != 0) { DMA_FLUSH(); - return ret; + goto done; } } @@ -1142,5 +1153,12 @@ int savage_bci_cmdbuf(DRM_IOCTL_ARGS) savage_freelist_put(dev, dmabuf); } - return 0; +done: + /* If we didn't need to allocate them, these'll be NULL */ + drm_free(kcmd_addr, cmdbuf.size * 8, DRM_MEM_DRIVER); + drm_free(kvb_addr, cmdbuf.vb_size, DRM_MEM_DRIVER); + drm_free(kbox_addr, cmdbuf.nbox * sizeof(drm_clip_rect_t), + DRM_MEM_DRIVER); + + return ret; } -- cgit v1.2.3