Hi!
Attached is a patch, that based on the info I have should make the via DRM
ok to go into the mainstream kernel, (save 64-bit problems in the memory manager?)
It implements a simple command verifier for agp and pci writes to the MMIO area.
Currently no 3D commands are implemented, but since the unichrome Mesa driver does
not yet use these IOCTLS, it should be OK. When it does, the pci command parser and the
command verifier needs to be updated or even replaced.
In combination with this, the MMIO drmAddMap in the DDX needs modification to export
read-only. I believe this should provide enough security.
Implications for clients is that XvMC will work as before, since it uses the new IOCTLS,
Current 3D OpenGL will segfault when run as a normal user, since MMIO mapping fails.
It will work when run as root.
I think this is the best to do until someone has time to update the 3D driver to use the
AGP ring-buffer.
Comments would be welcome.
Erdi, Alan, Dave?
/Thomas
Index: shared/via_dma.c
===================================================================
RCS file: /cvs/dri/drm/shared/via_dma.c,v
retrieving revision 1.8
diff -u -r1.8 via_dma.c
--- shared/via_dma.c 27 Sep 2004 20:14:31 -0000 1.8
+++ shared/via_dma.c 7 Oct 2004 12:46:36 -0000
@@ -24,6 +24,46 @@
static void via_cmdbuf_rewind(drm_via_private_t * dev_priv);
static int via_wait_idle(drm_via_private_t * dev_priv);
+
+/*
+ * This function needs to be extended whenever a new command set
+ * is implemented. Currently it works only for the 2D engine
+ * command, which on the Unichrome allows writing to
+ * at least the 2D engine and the mpeg engine, but not the
+ * video engine.
+ *
+ * If you update this function with new commands, please also
+ * consider implementing these commands in
+ * via_parse_pci_cmdbuffer below.
+ *
+ * Carefully review this function for security holes
+ * after an update!!!!!!!!!
+ */
+
+static int via_check_command_stream(const uint32_t *buf,
+ unsigned int size)
+{
+
+ uint32_t offset;
+ unsigned int i;
+
+ size >>=3;
+ for (i=0; i<size; ++i) {
+ offset = *buf;
+ buf += 2;
+ if ((offset > ((0x7FF >> 2) | VIA_2D_CMD)) &&
+ (offset < ((0xC00 >> 2) | VIA_2D_CMD)) ) {
+ DRM_ERROR("Attempt to access Burst Command Area.\n");
+ return DRM_ERR( EINVAL );
+ } else if (offset > ((0xDFF >> 2) | VIA_2D_CMD)) {
+ DRM_ERROR("Attempt to access DMA or VGA registers.\n");
+ return DRM_ERR( EINVAL );
+ }
+ }
+ return 0;
+}
+
+
static inline int
via_cmdbuf_wait(drm_via_private_t * dev_priv, unsigned int size)
{
@@ -163,6 +203,8 @@
{
drm_via_private_t *dev_priv = dev->dev_private;
uint32_t * vb;
+ int ret;
+
vb = via_check_dma(dev_priv, cmd->size);
if (vb == NULL) {
return DRM_ERR(EAGAIN);
@@ -170,6 +212,10 @@
if (DRM_COPY_FROM_USER(vb, cmd->buf, cmd->size)) {
return DRM_ERR(EFAULT);
}
+
+ if ((ret = via_check_command_stream( vb, cmd->size)))
+ return ret;
+
dev_priv->dma_low += cmd->size;
via_cmdbuf_pause(dev_priv);
@@ -225,6 +271,7 @@
return 0;
}
+
static int via_parse_pci_cmdbuffer( drm_device_t *dev, const char *buf,
unsigned int size )
{
@@ -232,22 +279,12 @@
uint32_t offset, value;
const uint32_t *regbuf = (uint32_t *)buf;
unsigned int i;
+ int ret;
+ if ((ret = via_check_command_stream( regbuf, size)))
+ return ret;
+
size >>=3 ;
- for (i=0; i<size; ++i) {
- offset = *regbuf;
- regbuf += 2;
- if ((offset > ((0x7FF >> 2) | VIA_2D_CMD)) &&
- (offset < ((0xC00 >> 2) | VIA_2D_CMD)) ) {
- DRM_DEBUG("Attempt to access Burst Command Area.\n");
- return DRM_ERR( EINVAL );
- } else if (offset > ((0xDFF >> 2) | VIA_2D_CMD)) {
- DRM_DEBUG("Attempt to access DMA or VGA registers.\n");
- return DRM_ERR( EINVAL );
- }
- }
-
- regbuf = (uint32_t *)buf;
for ( i=0; i<size; ++i ) {
offset = (*regbuf++ & ~VIA_2D_CMD) << 2;
value = *regbuf++;
