Hollis Blanchard wrote:
On Wednesday, Jun 4, 2003, at 17:45 US/Central, José Fonseca wrote:
On Wed, Jun 04, 2003 at 05:17:52PM -0500, Hollis Blanchard wrote:
[...]This is what the Stanford checker turned up recently when analyzing the copy_to/from_user calls in the Linux kernel:
This is all because the DRM_COPY_FROM_USER_UNCHECKED is being called in radeon_cp_dispatch_indices. If the copy_from_user is needed, the whole sarea_priv structure must be in user space, in which case all the other direct sarea references are in error. The other possibility is that copy_from_user isn't needed here at all. Can anyone comment?
The SAREA, and hence drm_radeon_sarea_t and 'boxes', lives on a shared memory
segment accessible by all intervenients (kernel, X server, client). So
the copy_from_user shouldn't be used.
I guess that at some point, radeon_cp_dispatch_indices was called on userspace cliprects, but now it appears only to be called on the SAREA. Perhaps Keith can tell more about it.
Any further comments here? I didn't quite follow the explanation of where SAREA lives, but I guess copy_from_user should be replaced? Anyone have a patch?
I started one, but won't be able to finish it off until Monday (probably).
Well, it's later than monday. Here's a simple change to fix the problem.
Keith
? diff
? ff
? sarea_stanford.diff
Index: radeon_state.c
===================================================================
RCS file:
/cvsroot/dri/xc/xc/programs/Xserver/hw/xfree86/os-support/shared/drm/kernel/radeon_state.c,v
retrieving revision 1.21
diff -u -r1.21 radeon_state.c
--- radeon_state.c 10 Jun 2003 18:54:17 -0000 1.21
+++ radeon_state.c 13 Jun 2003 15:06:15 -0000
@@ -888,15 +888,14 @@
static void radeon_cp_dispatch_vertex( drm_device_t *dev,
drm_buf_t *buf,
- drm_radeon_tcl_prim_t *prim,
- drm_clip_rect_t *boxes,
- int nbox )
+ drm_radeon_tcl_prim_t *prim )
{
drm_radeon_private_t *dev_priv = dev->dev_private;
- drm_clip_rect_t box;
+ drm_radeon_sarea_t *sarea_priv = dev_priv->sarea_priv;
int offset = dev_priv->agp_buffers_offset + buf->offset + prim->start;
int numverts = (int)prim->numverts;
+ int nbox = sarea_priv->nbox;
int i = 0;
RING_LOCALS;
@@ -916,10 +915,8 @@
do {
/* Emit the next cliprect */
if ( i < nbox ) {
- if (DRM_COPY_FROM_USER_UNCHECKED( &box, &boxes[i], sizeof(box)
))
- return;
-
- radeon_emit_clip_rect( dev_priv, &box );
+ radeon_emit_clip_rect( dev_priv,
+ &sarea_priv->boxes[i] );
}
/* Emit the vertex buffer rendering commands */
@@ -998,18 +995,17 @@
static void radeon_cp_dispatch_indices( drm_device_t *dev,
drm_buf_t *elt_buf,
- drm_radeon_tcl_prim_t *prim,
- drm_clip_rect_t *boxes,
- int nbox )
+ drm_radeon_tcl_prim_t *prim )
{
drm_radeon_private_t *dev_priv = dev->dev_private;
- drm_clip_rect_t box;
+ drm_radeon_sarea_t *sarea_priv = dev_priv->sarea_priv;
int offset = dev_priv->agp_buffers_offset + prim->offset;
u32 *data;
int dwords;
int i = 0;
int start = prim->start + RADEON_INDEX_PRIM_OFFSET;
int count = (prim->finish - start) / sizeof(u16);
+ int nbox = sarea_priv->nbox;
DRM_DEBUG("hwprim 0x%x vfmt 0x%x %d..%d offset: %x nr %d\n",
prim->prim,
@@ -1048,12 +1044,9 @@
(count << RADEON_NUM_VERTICES_SHIFT) );
do {
- if ( i < nbox ) {
- if (DRM_COPY_FROM_USER_UNCHECKED( &box, &boxes[i], sizeof(box)
))
- return;
-
- radeon_emit_clip_rect( dev_priv, &box );
- }
+ if ( i < nbox )
+ radeon_emit_clip_rect( dev_priv,
+ &sarea_priv->boxes[i] );
radeon_cp_dispatch_indirect( dev, elt_buf,
prim->start,
@@ -1453,9 +1446,7 @@
prim.numverts = vertex.count;
prim.vc_format = dev_priv->sarea_priv->vc_format;
- radeon_cp_dispatch_vertex( dev, buf, &prim,
- dev_priv->sarea_priv->boxes,
- dev_priv->sarea_priv->nbox );
+ radeon_cp_dispatch_vertex( dev, buf, &prim );
}
if (vertex.discard) {
@@ -1553,9 +1544,7 @@
prim.numverts = RADEON_MAX_VB_VERTS; /* duh */
prim.vc_format = dev_priv->sarea_priv->vc_format;
- radeon_cp_dispatch_indices( dev, buf, &prim,
- dev_priv->sarea_priv->boxes,
- dev_priv->sarea_priv->nbox );
+ radeon_cp_dispatch_indices( dev, buf, &prim );
if (elts.discard) {
radeon_cp_discard_buffer( dev, buf );
}
@@ -1772,16 +1761,12 @@
tclprim.offset = prim.numverts * 64;
tclprim.numverts = RADEON_MAX_VB_VERTS; /* duh */
- radeon_cp_dispatch_indices( dev, buf, &tclprim,
- sarea_priv->boxes,
- sarea_priv->nbox);
+ radeon_cp_dispatch_indices( dev, buf, &tclprim );
} else {
tclprim.numverts = prim.numverts;
tclprim.offset = 0; /* not used */
- radeon_cp_dispatch_vertex( dev, buf, &tclprim,
- sarea_priv->boxes,
- sarea_priv->nbox);
+ radeon_cp_dispatch_vertex( dev, buf, &tclprim );
}
if (sarea_priv->nbox == 1)
