From b99e332236ca5fcc11e8d7c89566bbf3bcf959ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20D=C3=A4nzer?= Date: Sat, 26 Aug 2006 12:21:11 +0200 Subject: Bug #7595: Avoid u32 overflows in radeon_check_and_fixup_offset(). The overflows could cause valid offsets to get rejected under some circumstances, e.g. when the framebuffer resides at the very end of the card's address space. --- shared-core/radeon_state.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) (limited to 'shared-core/radeon_state.c') diff --git a/shared-core/radeon_state.c b/shared-core/radeon_state.c index 58251dd7..38b87148 100644 --- a/shared-core/radeon_state.c +++ b/shared-core/radeon_state.c @@ -42,7 +42,11 @@ static __inline__ int radeon_check_and_fixup_offset(drm_radeon_private_t * drm_file_t * filp_priv, u32 * offset) { - u32 off = *offset; + u64 off = *offset; + u32 fb_start = dev_priv->fb_location; + u32 fb_end = fb_start + dev_priv->fb_size - 1; + u32 gart_start = dev_priv->gart_vm_start; + u32 gart_end = gart_start + dev_priv->gart_size - 1; struct drm_radeon_driver_file_fields *radeon_priv; /* Hrm ... the story of the offset ... So this function converts @@ -62,10 +66,8 @@ static __inline__ int radeon_check_and_fixup_offset(drm_radeon_private_t * /* First, the best case, the offset already lands in either the * framebuffer or the GART mapped space */ - if ((off >= dev_priv->fb_location && - off < (dev_priv->fb_location + dev_priv->fb_size)) || - (off >= dev_priv->gart_vm_start && - off < (dev_priv->gart_vm_start + dev_priv->gart_size))) + if ((off >= fb_start && off <= fb_end) || + (off >= gart_start && off <= gart_end)) return 0; /* Ok, that didn't happen... now check if we have a zero based @@ -78,16 +80,13 @@ static __inline__ int radeon_check_and_fixup_offset(drm_radeon_private_t * } /* Finally, assume we aimed at a GART offset if beyond the fb */ - if (off > (dev_priv->fb_location + dev_priv->fb_size)) - off = off - (dev_priv->fb_location + dev_priv->fb_size) + - dev_priv->gart_vm_start; + if (off > fb_end) + off = off - fb_end - 1 + gart_start; /* Now recheck and fail if out of bounds */ - if ((off >= dev_priv->fb_location && - off < (dev_priv->fb_location + dev_priv->fb_size)) || - (off >= dev_priv->gart_vm_start && - off < (dev_priv->gart_vm_start + dev_priv->gart_size))) { - DRM_DEBUG("offset fixed up to 0x%x\n", off); + if ((off >= fb_start && off <= fb_end) || + (off >= gart_start && off <= gart_end)) { + DRM_DEBUG("offset fixed up to 0x%x\n", (unsigned int)off); *offset = off; return 0; } -- cgit v1.2.3 From 9b984b34e99f694e10251e15bc2ec1bc844dcca4 Mon Sep 17 00:00:00 2001 From: Dave Airlie Date: Sat, 19 Aug 2006 17:59:18 +1000 Subject: drm: lots of small cleanups and whitespace issues fixed up remove a mach64 warning, align a lot of things from linux kernel --- shared-core/radeon_state.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'shared-core/radeon_state.c') diff --git a/shared-core/radeon_state.c b/shared-core/radeon_state.c index 38b87148..8c1a4065 100644 --- a/shared-core/radeon_state.c +++ b/shared-core/radeon_state.c @@ -2652,10 +2652,10 @@ static __inline__ int radeon_emit_veclinear(drm_radeon_private_t *dev_priv, int start = header.veclinear.addr_lo | (header.veclinear.addr_hi << 8); RING_LOCALS; - if (!sz) - return 0; - if (sz * 4 > cmdbuf->bufsz) - return DRM_ERR(EINVAL); + if (!sz) + return 0; + if (sz * 4 > cmdbuf->bufsz) + return DRM_ERR(EINVAL); BEGIN_RING(5 + sz); OUT_RING_REG(RADEON_SE_TCL_STATE_FLUSH, 0); -- cgit v1.2.3 From 3cc64a943a7240c73c92ab103ba0502b9ec07fee Mon Sep 17 00:00:00 2001 From: Dave Airlie Date: Tue, 12 Sep 2006 06:13:14 +1000 Subject: drm: use radeon specific names for radeon flags --- shared-core/radeon_state.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'shared-core/radeon_state.c') diff --git a/shared-core/radeon_state.c b/shared-core/radeon_state.c index 8c1a4065..0a196c06 100644 --- a/shared-core/radeon_state.c +++ b/shared-core/radeon_state.c @@ -861,7 +861,7 @@ static void radeon_cp_dispatch_clear(drm_device_t * dev, */ dev_priv->sarea_priv->ctx_owner = 0; - if ((dev_priv->flags & CHIP_HAS_HIERZ) + if ((dev_priv->flags & RADEON_HAS_HIERZ) && (flags & RADEON_USE_HIERZ)) { /* FIXME : reverse engineer that for Rx00 cards */ /* FIXME : the mask supposedly contains low-res z values. So can't set @@ -906,7 +906,7 @@ static void radeon_cp_dispatch_clear(drm_device_t * dev, for (i = 0; i < nbox; i++) { int tileoffset, nrtilesx, nrtilesy, j; /* it looks like r200 needs rv-style clears, at least if hierz is not enabled? */ - if ((dev_priv->flags & CHIP_HAS_HIERZ) + if ((dev_priv->flags & RADEON_HAS_HIERZ) && !(dev_priv->microcode_version == UCODE_R200)) { /* FIXME : figure this out for r200 (when hierz is enabled). Or maybe r200 actually doesn't need to put the low-res z value into @@ -990,7 +990,7 @@ static void radeon_cp_dispatch_clear(drm_device_t * dev, } /* TODO don't always clear all hi-level z tiles */ - if ((dev_priv->flags & CHIP_HAS_HIERZ) + if ((dev_priv->flags & RADEON_HAS_HIERZ) && (dev_priv->microcode_version == UCODE_R200) && (flags & RADEON_USE_HIERZ)) /* r100 and cards without hierarchical z-buffer have no high-level z-buffer */ @@ -3031,9 +3031,9 @@ static int radeon_cp_getparam(DRM_IOCTL_ARGS) break; case RADEON_PARAM_CARD_TYPE: - if (dev_priv->flags & CHIP_IS_PCIE) + if (dev_priv->flags & RADEON_IS_PCIE) value = RADEON_CARD_PCIE; - else if (dev_priv->flags & CHIP_IS_AGP) + else if (dev_priv->flags & RADEON_IS_AGP) value = RADEON_CARD_AGP; else value = RADEON_CARD_PCI; -- cgit v1.2.3 From 6ba9127753eff7615ba553fbc567aec98ecf8104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20D=C3=A4nzer?= Date: Fri, 15 Sep 2006 16:37:47 +0200 Subject: Use register writes instead of BITBLT_MULTI packets for buffer swap blits. This takes up two more ring buffer entries per rectangle blitted but makes sure the blit is performed top to bottom, reducing the likelyhood of tearing. --- shared-core/radeon_state.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'shared-core/radeon_state.c') diff --git a/shared-core/radeon_state.c b/shared-core/radeon_state.c index 0a196c06..5b1ca539 100644 --- a/shared-core/radeon_state.c +++ b/shared-core/radeon_state.c @@ -1262,9 +1262,9 @@ static void radeon_cp_dispatch_swap(drm_device_t * dev) DRM_DEBUG("dispatch swap %d,%d-%d,%d\n", x, y, w, h); - BEGIN_RING(7); + BEGIN_RING(9); - OUT_RING(CP_PACKET3(RADEON_CNTL_BITBLT_MULTI, 5)); + OUT_RING(CP_PACKET0(RADEON_DP_GUI_MASTER_CNTL, 0)); OUT_RING(RADEON_GMC_SRC_PITCH_OFFSET_CNTL | RADEON_GMC_DST_PITCH_OFFSET_CNTL | RADEON_GMC_BRUSH_NONE | @@ -1276,6 +1276,7 @@ static void radeon_cp_dispatch_swap(drm_device_t * dev) /* Make this work even if front & back are flipped: */ + OUT_RING(CP_PACKET0(RADEON_SRC_PITCH_OFFSET, 1)); if (dev_priv->current_page == 0) { OUT_RING(dev_priv->back_pitch_offset); OUT_RING(dev_priv->front_pitch_offset); @@ -1284,6 +1285,7 @@ static void radeon_cp_dispatch_swap(drm_device_t * dev) OUT_RING(dev_priv->back_pitch_offset); } + OUT_RING(CP_PACKET0(RADEON_SRC_X_Y, 2)); OUT_RING((x << 16) | y); OUT_RING((x << 16) | y); OUT_RING((w << 16) | h); -- cgit v1.2.3 From 1f71b8d7a456fe3ec4bfc2fed70b7420cdd0d55a Mon Sep 17 00:00:00 2001 From: Roland Scheidegger Date: Wed, 20 Sep 2006 19:44:57 +0200 Subject: do a TCL state flush before accessing VAP_CNTL to prevent lockups on r200 when enabling/disabling vertex programs --- shared-core/radeon_state.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'shared-core/radeon_state.c') diff --git a/shared-core/radeon_state.c b/shared-core/radeon_state.c index 5b1ca539..b4478019 100644 --- a/shared-core/radeon_state.c +++ b/shared-core/radeon_state.c @@ -174,6 +174,14 @@ static __inline__ int radeon_check_and_fixup_packets(drm_radeon_private_t * } break; + case R200_EMIT_VAP_CTL: { + RING_LOCALS; + BEGIN_RING(2); + OUT_RING_REG(RADEON_SE_TCL_STATE_FLUSH, 0); + ADVANCE_RING(); + } + break; + case RADEON_EMIT_RB3D_COLORPITCH: case RADEON_EMIT_RE_LINE_PATTERN: case RADEON_EMIT_SE_LINE_WIDTH: @@ -201,7 +209,6 @@ static __inline__ int radeon_check_and_fixup_packets(drm_radeon_private_t * case R200_EMIT_TCL_LIGHT_MODEL_CTL_0: case R200_EMIT_TFACTOR_0: case R200_EMIT_VTX_FMT_0: - case R200_EMIT_VAP_CTL: case R200_EMIT_MATRIX_SELECT_0: case R200_EMIT_TEX_PROC_CTL_2: case R200_EMIT_TCL_UCP_VERT_BLEND_CTL: -- cgit v1.2.3 From a9f57a2b9c5897cbf568bf75342204b780566de0 Mon Sep 17 00:00:00 2001 From: Roland Scheidegger Date: Tue, 10 Oct 2006 02:24:19 +0200 Subject: only allow specific type-3 packets to pass the verifier instead of all for r100/r200 as others might be unsafe (r300 already does this), and add checking for these we need but aren't safe. Check the RADEON_CP_INDX_BUFFER packet on both r200 and r300 as it isn't safe neither. --- shared-core/radeon_state.c | 109 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 106 insertions(+), 3 deletions(-) (limited to 'shared-core/radeon_state.c') diff --git a/shared-core/radeon_state.c b/shared-core/radeon_state.c index b4478019..bf5e3d29 100644 --- a/shared-core/radeon_state.c +++ b/shared-core/radeon_state.c @@ -275,6 +275,8 @@ static __inline__ int radeon_check_and_fixup_packet3(drm_radeon_private_t * unsigned int *cmdsz) { u32 *cmd = (u32 *) cmdbuf->buf; + u32 offset, narrays; + int count, i, k; *cmdsz = 2 + ((cmd[0] & RADEON_CP_PACKET_COUNT_MASK) >> 16); @@ -288,10 +290,106 @@ static __inline__ int radeon_check_and_fixup_packet3(drm_radeon_private_t * return DRM_ERR(EINVAL); } - /* Check client state and fix it up if necessary */ - if (cmd[0] & 0x8000) { /* MSB of opcode: next DWORD GUI_CNTL */ - u32 offset; + switch(cmd[0] & 0xff00) { + /* XXX Are there old drivers needing other packets? */ + case RADEON_3D_DRAW_IMMD: + case RADEON_3D_DRAW_VBUF: + case RADEON_3D_DRAW_INDX: + case RADEON_WAIT_FOR_IDLE: + case RADEON_CP_NOP: + case RADEON_3D_CLEAR_ZMASK: +/* case RADEON_CP_NEXT_CHAR: + case RADEON_CP_PLY_NEXTSCAN: + case RADEON_CP_SET_SCISSORS: */ /* probably safe but will never need them? */ + /* these packets are safe */ + break; + + case RADEON_CP_3D_DRAW_IMMD_2: + case RADEON_CP_3D_DRAW_VBUF_2: + case RADEON_CP_3D_DRAW_INDX_2: + case RADEON_3D_CLEAR_HIZ: + /* safe but r200 only */ + if (dev_priv->microcode_version != UCODE_R200) { + DRM_ERROR("Invalid 3d packet for r100-class chip\n"); + return DRM_ERR(EINVAL); + } + break; + + case RADEON_3D_LOAD_VBPNTR: + count = (cmd[0] >> 16) & 0x3fff; + + if (count > 18) { /* 12 arrays max */ + DRM_ERROR("Too large payload in 3D_LOAD_VBPNTR (count=%d)\n", + count); + return DRM_ERR(EINVAL); + } + + /* carefully check packet contents */ + narrays = cmd[1] & ~0xc000; + k = 0; + i = 2; + while ((k < narrays) && (i < (count + 2))) { + i++; /* skip attribute field */ + if (radeon_check_and_fixup_offset(dev_priv, filp_priv, &cmd[i])) { + DRM_ERROR + ("Invalid offset (k=%d i=%d) in 3D_LOAD_VBPNTR packet.\n", + k, i); + return DRM_ERR(EINVAL); + } + k++; + i++; + if (k == narrays) + break; + /* have one more to process, they come in pairs */ + if (radeon_check_and_fixup_offset(dev_priv, filp_priv, &cmd[i])) { + DRM_ERROR + ("Invalid offset (k=%d i=%d) in 3D_LOAD_VBPNTR packet.\n", + k, i); + return DRM_ERR(EINVAL); + } + k++; + i++; + } + /* do the counts match what we expect ? */ + if ((k != narrays) || (i != (count + 2))) { + DRM_ERROR + ("Malformed 3D_LOAD_VBPNTR packet (k=%d i=%d narrays=%d count+1=%d).\n", + k, i, narrays, count + 1); + return DRM_ERR(EINVAL); + } + break; + + case RADEON_3D_RNDR_GEN_INDX_PRIM: + if (dev_priv->microcode_version != UCODE_R100) { + DRM_ERROR("Invalid 3d packet for r200-class chip\n"); + return DRM_ERR(EINVAL); + } + if (radeon_check_and_fixup_offset(dev_priv, filp_priv, &cmd[1])) { + DRM_ERROR("Invalid rndr_gen_indx offset\n"); + return DRM_ERR(EINVAL); + } + break; + + case RADEON_CP_INDX_BUFFER: + if (dev_priv->microcode_version != UCODE_R200) { + DRM_ERROR("Invalid 3d packet for r100-class chip\n"); + return DRM_ERR(EINVAL); + } + if ((cmd[1] & 0x8000ffff) != 0x80000810) { + DRM_ERROR("Invalid indx_buffer reg address %08X\n", cmd[1]); + return DRM_ERR(EINVAL); + } + if (radeon_check_and_fixup_offset(dev_priv, filp_priv, &cmd[2])) { + DRM_ERROR("Invalid indx_buffer offset is %08X\n", cmd[2]); + return DRM_ERR(EINVAL); + } + break; + + case RADEON_CNTL_HOSTDATA_BLT: + case RADEON_CNTL_PAINT_MULTI: + case RADEON_CNTL_BITBLT_MULTI: + /* MSB of opcode: next DWORD GUI_CNTL */ if (cmd[1] & (RADEON_GMC_SRC_PITCH_OFFSET_CNTL | RADEON_GMC_DST_PITCH_OFFSET_CNTL)) { offset = cmd[2] << 10; @@ -313,6 +411,11 @@ static __inline__ int radeon_check_and_fixup_packet3(drm_radeon_private_t * } cmd[3] = (cmd[3] & 0xffc00000) | offset >> 10; } + break; + + default: + DRM_ERROR("Invalid packet type %x\n", cmd[0] & 0xff00); + return DRM_ERR(EINVAL); } return 0; -- cgit v1.2.3