[R300] securing r300 drm
Now that the driver paints usable pictures without lockups on many cards, including AGP versions of X800 and Mobility M10, it would make sense to ready it for inclusion into main DRI codebase. I do not think that elusive lockups of Radeon 9800 cards, or issues with PowerPC will require any drastic changes. As we discussed earlier, the major reason against inclusion into mainstream DRI CVS is that the driver is not secure in its current state. Below, I will attempt to list current known issues - please reply with your additions. * r300_emit_unchecked_state - it is not as unchecked as it has been initially, however a few poorly checked registers remain: from r300_cmdbuf.c: ADD_RANGE(R300_RB3D_COLOROFFSET0, 1); /* Dangerous */ ADD_RANGE(R300_RB3D_COLORPITCH0, 1); /* Dangerous */ /* .. snip ... */ ADD_RANGE(R300_RB3D_DEPTHOFFSET, 2); /* Dangerous */ In principle an attacker can set these to point to AGP or system RAM and then cause a paint operation to overwrite particular memory range. Ideally we should check that these point inside the framebuffer, i.e. are within range specified by MC_FB_LOCATION register. /* Texture offset is dangerous and needs more checking */ ADD_RANGE(R300_TX_OFFSET_0, 16); I don't think texture offsets are ever written to, however if they point in the wrong place they can be used to read memory directly. ideally we would check these to be either with MC_FB_LOCATION or MC_AGP_LOCATION ranges. Problem is what do we do on PCI cards ? use AIC controller settings ? * r300_emit_raw - we do not have code that checks any of bufferred 3d packets, in particular VBUF_2, IMMD_2, INDX_2 and INDX_BUFFER. I think that none of these can be exploited except to cause a lockup - please correct me if I am wrong * r300_emit_raw - RADEON_3D_LOAD_VBPNTR - this sets offsets and so like texture offset registers could be exploited to read protected memory locations. Again, we need to check the offsets against something reasonable. * anything I forgot ? best Vladimir Dergachev --- SF.Net email is sponsored by: Discover Easy Linux Migration Strategies from IBM. Find simple to follow Roadmaps, straightforward articles, informative Webcasts and more! Get everything you need to get up to speed, fast. http://ads.osdn.com/?ad_id=7477alloc_id=16492op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [R300] securing r300 drm
On Tue, 2005-06-21 at 14:57 -0400, Vladimir Dergachev wrote: /* Texture offset is dangerous and needs more checking */ ADD_RANGE(R300_TX_OFFSET_0, 16); I don't think texture offsets are ever written to, however if they point in the wrong place they can be used to read memory directly. ideally we would check these to be either with MC_FB_LOCATION or MC_AGP_LOCATION ranges. Problem is what do we do on PCI cards ? use AIC controller settings ? Just verify that the location is within expected areas of the card's virtual address space, like you do for color/depth offsets, right? -- Eric Anholt [EMAIL PROTECTED] http://people.freebsd.org/~anholt/ [EMAIL PROTECTED] --- SF.Net email is sponsored by: Discover Easy Linux Migration Strategies from IBM. Find simple to follow Roadmaps, straightforward articles, informative Webcasts and more! Get everything you need to get up to speed, fast. http://ads.osdn.com/?ad_id=7477alloc_id=16492op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [R300] securing r300 drm
On Tue, 21 Jun 2005, Eric Anholt wrote: On Tue, 2005-06-21 at 14:57 -0400, Vladimir Dergachev wrote: /* Texture offset is dangerous and needs more checking */ ADD_RANGE(R300_TX_OFFSET_0, 16); I don't think texture offsets are ever written to, however if they point in the wrong place they can be used to read memory directly. ideally we would check these to be either with MC_FB_LOCATION or MC_AGP_LOCATION ranges. Problem is what do we do on PCI cards ? use AIC controller settings ? Just verify that the location is within expected areas of the card's virtual address space, like you do for color/depth offsets, right? Yes, the question is what these are for PCI cards. I guess AIC should have a register similar to MC_AGP_LOCATION. best Vladimir Dergachev -- Eric Anholt [EMAIL PROTECTED] http://people.freebsd.org/~anholt/ [EMAIL PROTECTED] --- SF.Net email is sponsored by: Discover Easy Linux Migration Strategies from IBM. Find simple to follow Roadmaps, straightforward articles, informative Webcasts and more! Get everything you need to get up to speed, fast. http://ads.osdn.com/?ad_id=7477alloc_id=16492op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [R300] securing r300 drm
On Tue, 2005-06-21 at 16:56 -0400, Vladimir Dergachev wrote: On Tue, 21 Jun 2005, Eric Anholt wrote: On Tue, 2005-06-21 at 14:57 -0400, Vladimir Dergachev wrote: /* Texture offset is dangerous and needs more checking */ ADD_RANGE(R300_TX_OFFSET_0, 16); I don't think texture offsets are ever written to, however if they point in the wrong place they can be used to read memory directly. ideally we would check these to be either with MC_FB_LOCATION or MC_AGP_LOCATION ranges. Problem is what do we do on PCI cards ? use AIC controller settings ? Just verify that the location is within expected areas of the card's virtual address space, like you do for color/depth offsets, right? Yes, the question is what these are for PCI cards. I guess AIC should have a register similar to MC_AGP_LOCATION. It looks like you use the same structure entries on both. The check for is not in bad system memory is: if (off = dev_priv-fb_location off (dev_priv-gart_vm_start + dev_priv-gart_size)) return 0; This is assuming that you've got the layout the same as on old radeon DRM, with the gart immediately following the fb aperture. -- Eric Anholt [EMAIL PROTECTED] http://people.freebsd.org/~anholt/ [EMAIL PROTECTED] --- SF.Net email is sponsored by: Discover Easy Linux Migration Strategies from IBM. Find simple to follow Roadmaps, straightforward articles, informative Webcasts and more! Get everything you need to get up to speed, fast. http://ads.osdn.com/?ad_id=7477alloc_id=16492op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [R300] securing r300 drm
On Tuesday 21 June 2005 20:57, Vladimir Dergachev wrote:
Now that the driver paints usable pictures without lockups on many cards,
including AGP versions of X800 and Mobility M10, it would make sense to
ready it for inclusion into main DRI codebase.
I do not think that elusive lockups of Radeon 9800 cards, or issues with
PowerPC will require any drastic changes.
As we discussed earlier, the major reason against inclusion into
mainstream DRI CVS is that the driver is not secure in its current state.
Below, I will attempt to list current known issues - please reply with
your additions.
* r300_emit_unchecked_state - it is not as unchecked as it has been
initially, however a few poorly checked registers remain:
Those poorly checked registers should be moved out of unchecked_state and
into its own function. Adding these checks into unchecked_state will just
add overhead to what should be a fast path.
The idea would be to add something like r300_emit_special_state which
doesn't use register addresses but has subcommands like the state setting
for radeon/r200.
from r300_cmdbuf.c:
ADD_RANGE(R300_RB3D_COLOROFFSET0, 1); /* Dangerous */
ADD_RANGE(R300_RB3D_COLORPITCH0, 1); /* Dangerous */
/* .. snip ... */
ADD_RANGE(R300_RB3D_DEPTHOFFSET, 2); /* Dangerous */
In principle an attacker can set these to point to AGP or system
RAM and then cause a paint operation to overwrite particular
memory range.
Ideally we should check that these point inside the framebuffer,
i.e. are within range specified by MC_FB_LOCATION register.
Right. Actually, to be on the safe side, we'd have to set min/max clipping
rects at the same time as setting those buffer offsets. This is currently
not a problem, but it will become one when (if? ;)) we implement
framebuffer_object etc.
/* Texture offset is dangerous and needs more checking */
ADD_RANGE(R300_TX_OFFSET_0, 16);
I don't think texture offsets are ever written to, however if
they
point in the wrong place they can be used to read memory
directly.
Setting those texture offsets wrong can actually lock up the machine as I
found out when I temporarily put MC_FB_LOCATION into its natural position
(i.e. where it's put on older radeons).
ideally we would check these to be either with MC_FB_LOCATION
or MC_AGP_LOCATION ranges. Problem is what do we do on PCI
cards ?
use AIC controller settings ?
Unfortunately, I don't know enough about PCI to comment, and what's AIC
anyway? I've seen some register names with AIC in it, but they don't really
seem to be used.
* r300_emit_raw - we do not have code that checks any of bufferred 3d
packets, in particular VBUF_2, IMMD_2, INDX_2 and INDX_BUFFER.
I think that none of these can be exploited except to cause a lockup
-
please correct me if I am wrong
* r300_emit_raw - RADEON_3D_LOAD_VBPNTR - this sets offsets and so
like texture offset registers could be exploited to read protected
memory locations.
Again, we need to check the offsets against something reasonable.
Note that by putting the offset at the end of allowed memory and setting the
number of vertices very high, you could read memory that you shouldn't have
access to.
But the more important thing is: What's up with r300_emit_raw anyway? It was
originally supposed to do what the name suggests: Emit raw data into the
ring buffer, purely as a hack for experimentation.
People have bent it in extreme ways, so it has clearly gone beyond that, and
that's a Bad Thing. For one thing, r300_emit_raw doesn't get cliprects
right. If you have more than four cliprects, you need to emit rendering
commands multiple times.
Seriously, all the stuff that uses emit_raw should just be migrated to use
r300_emit_packet3, which will clean this up a lot.
* anything I forgot ?
Talking about security, have a look at radeon_state.c. Where on earth does
*this* come from:
/* Allocate an in-kernel area and copy in the cmdbuf. Do this to
avoid
* races between checking values and using those values in other
code,
* and simply to avoid a lot of function calls to copy in data.
*/
orig_bufsz = cmdbuf.bufsz;
if (orig_bufsz != 0) {
kbuf = drm_alloc(cmdbuf.bufsz, DRM_MEM_DRIVER);
if (kbuf == NULL)
return DRM_ERR(ENOMEM);
if (DRM_COPY_FROM_USER(kbuf, cmdbuf.buf, cmdbuf.bufsz)) {
drm_free(kbuf, orig_bufsz, DRM_MEM_DRIVER);
return DRM_ERR(EFAULT);
}
cmdbuf.buf = kbuf;
}
This just shouts insanity. It calls kmalloc every single time you emit a
command buffer!
The security issue mentioned in the comment is real, but there's a really
simple rule of thumb that eliminates it
