Bug 6242: [mach64] Use private DMA buffers.

Map the DMA buffers read-only. This eliminates a security problem where a
client can alter the contents of the DMA buffer after submission to the DRM.

2 files changed, 102 insertions, 57 deletions

diff --git a/src/atidri.c b/src/atidri.c
index 1e936d1..26bee38 100644
--- a/src/atidri.c
+++ b/src/atidri.c
@@ -688,6 +688,36 @@ static int Mach64MinBits(int val)
     return bits;
 }
 
+static Bool ATIDRISetBufSize( ScreenPtr pScreen, unsigned int maxSize )
+{
+   ScrnInfoPtr pScreenInfo = xf86Screens[pScreen->myNum];
+   ATIPtr pATI = ATIPTR(pScreenInfo);
+   ATIDRIServerInfoPtr pATIDRIServer = pATI->pDRIServerInfo;
+
+   if (pATI->OptionBufferSize) {
+      if (pATI->OptionBufferSize < 1 || pATI->OptionBufferSize > maxSize  ) {
+	 xf86DrvMsg( pScreen->myNum, X_ERROR, "[drm] Illegal DMA buffers size: %d MB\n",
+		     pATI->OptionBufferSize );
+	 return FALSE;
+      }
+      if (pATI->OptionBufferSize > 2) {
+	 xf86DrvMsg( pScreen->myNum, X_WARNING, "[drm] Illegal DMA buffers size: %d MB\n",
+		     pATI->OptionBufferSize );
+	 xf86DrvMsg( pScreen->myNum, X_WARNING, "[drm] Clamping DMA buffers size to 2 MB\n");
+	 pATIDRIServer->bufferSize = 2;
+      } else {
+	 pATIDRIServer->bufferSize = pATI->OptionBufferSize;
+	 xf86DrvMsg( pScreen->myNum, X_CONFIG, "[drm] Using %d MB for DMA buffers\n",
+		     pATIDRIServer->bufferSize );
+      }
+   } else {
+      xf86DrvMsg( pScreen->myNum, X_DEFAULT, "[drm] Using %d MB for DMA buffers\n",
+		  pATIDRIServer->bufferSize );
+   }
+
+   return TRUE;
+}
+
 static Bool ATIDRISetAgpMode( ScreenPtr pScreen )
 {
    ScrnInfoPtr pScreenInfo = xf86Screens[pScreen->myNum];
@@ -809,26 +839,8 @@ static Bool ATIDRIAgpInit( ScreenPtr pScreen )
    xf86DrvMsg(pScreen->myNum, X_INFO,
 	      "[agp] Using %d kB for DMA descriptor ring\n", pATIDRIServer->ringSize);
 
-   if (pATI->OptionBufferSize) {
-      if (pATI->OptionBufferSize < 1 || pATI->OptionBufferSize > pATIDRIServer->agpSize  ) {
-	 xf86DrvMsg( pScreen->myNum, X_ERROR, "[agp] Illegal DMA buffers size: %d MB\n",
-		     pATI->OptionBufferSize );
-	 return FALSE;
-      }
-      if (pATI->OptionBufferSize > 2) {
-	 xf86DrvMsg( pScreen->myNum, X_WARNING, "[agp] Illegal DMA buffers size: %d MB\n",
-		     pATI->OptionBufferSize );
-	 xf86DrvMsg( pScreen->myNum, X_WARNING, "[agp] Clamping DMA buffers size to 2 MB\n");
-	 pATIDRIServer->bufferSize = 2;
-      } else {
-	 pATIDRIServer->bufferSize = pATI->OptionBufferSize;
-	 xf86DrvMsg( pScreen->myNum, X_CONFIG, "[agp] Using %d MB for DMA buffers\n",
-		     pATIDRIServer->bufferSize );
-      }
-   } else {
-      xf86DrvMsg( pScreen->myNum, X_DEFAULT, "[agp] Using %d MB for DMA buffers\n",
-		  pATIDRIServer->bufferSize );
-   }
+   if ( !ATIDRISetBufSize( pScreen, pATIDRIServer->agpSize ) )
+      return FALSE;
 
    pATIDRIServer->agpTexSize    = pATIDRIServer->agpSize - pATIDRIServer->bufferSize;
 
@@ -874,7 +886,7 @@ static Bool ATIDRIAgpInit( ScreenPtr pScreen )
 
    /* Map vertex buffers */
    if ( drmAddMap( pATI->drmFD, pATIDRIServer->bufferStart, pATIDRIServer->bufferMapSize,
-		   DRM_AGP, 0, &pATIDRIServer->bufferHandle ) < 0 ) {
+		   DRM_AGP, DRM_READ_ONLY, &pATIDRIServer->bufferHandle ) < 0 ) {
       xf86DrvMsg( pScreen->myNum, X_ERROR,
 		  "[agp] Could not add vertex buffers mapping\n" );
       return FALSE;
@@ -951,6 +963,55 @@ static Bool ATIDRIAgpInit( ScreenPtr pScreen )
    return TRUE;
 }
 
+static Bool ATIDRIPciInit( ScreenPtr pScreen )
+{
+   ScrnInfoPtr pScreenInfo = xf86Screens[pScreen->myNum];
+   ATIPtr pATI = ATIPTR(pScreenInfo);
+   ATIDRIServerInfoPtr pATIDRIServer = pATI->pDRIServerInfo;
+
+   pATIDRIServer->bufferSize = ATI_DEFAULT_BUFFER_SIZE;
+   pATIDRIServer->ringSize = 16; /* 16 kB ring */
+
+   if ( !ATIDRISetBufSize( pScreen, (unsigned)(-1) ) )
+      return FALSE;
+
+   /* Set size of the DMA descriptor ring */
+   pATIDRIServer->ringStart   = 0;
+   pATIDRIServer->ringMapSize = pATIDRIServer->ringSize*1024; /* ringSize is in kB */
+
+   /* Set size of the vertex buffer */
+   pATIDRIServer->bufferStart   = 0;
+   pATIDRIServer->bufferMapSize = pATIDRIServer->bufferSize*1024*1024;
+
+   /* Map DMA descriptor ring */
+   if ( drmAddMap( pATI->drmFD, 0, pATIDRIServer->ringMapSize,
+		   DRM_CONSISTENT, DRM_RESTRICTED, &pATIDRIServer->ringHandle ) < 0 ) {
+      xf86DrvMsg( pScreen->myNum, X_ERROR,
+		  "[pci] Could not add ring mapping\n" );
+      return FALSE;
+   }
+   xf86DrvMsg( pScreen->myNum, X_INFO, "[pci] ring handle = 0x%08x\n",
+	       pATIDRIServer->ringHandle );
+
+   if ( drmMap( pATI->drmFD, pATIDRIServer->ringHandle,
+		pATIDRIServer->ringMapSize, &pATIDRIServer->ringMap ) < 0 ) {
+      xf86DrvMsg( pScreen->myNum, X_ERROR,
+		  "[pci] Could not map ring\n" );
+      return FALSE;
+   }
+   xf86DrvMsg( pScreen->myNum, X_INFO,
+	       "[pci] Ring mapped at 0x%08lx\n",
+	       (unsigned long)pATIDRIServer->ringMap );
+
+   /* Disable AGP for ForcePCIMode */
+   if ( pATI->BusType != ATI_BUS_PCI ) {
+       outm( AGP_BASE, 0 );
+       outm( AGP_CNTL, 0 );
+   }
+
+   return TRUE;
+}
+
 /* Add a map for the MMIO registers that will be accessed by any
  * DRI-based clients.
  */
@@ -1027,9 +1088,9 @@ static Bool ATIDRIAddBuffers( ScreenPtr pScreen )
    /* Initialize vertex buffers */
    if ( pATIDRIServer->IsPCI ) {
       pATIDRIServer->numBuffers = drmAddBufs( pATI->drmFD,
-					      (pATIDRIServer->bufferSize*1024*1024)/MACH64_BUFFER_SIZE,
+					      pATIDRIServer->bufferMapSize/MACH64_BUFFER_SIZE,
 					      MACH64_BUFFER_SIZE,
-					      0,
+					      DRM_PCI_BUFFER_RO,
 					      0 );
    } else {
       pATIDRIServer->numBuffers = drmAddBufs( pATI->drmFD,
@@ -1316,7 +1377,7 @@ Bool ATIDRIScreenInit( ScreenPtr pScreen )
    /* Check the mach64 DRM version */
    version = drmGetVersion( pATI->drmFD );
    if ( version ) {
-      if ( version->version_major != 1 ||
+      if ( version->version_major != 2 ||
 	   version->version_minor < 0 ) {
 	 /* Incompatible DRM version */
 	 xf86DrvMsg( pScreen->myNum, X_ERROR,
@@ -1350,48 +1411,22 @@ Bool ATIDRIScreenInit( ScreenPtr pScreen )
    pATIDRIServer->IsPCI = (pATI->BusType == ATI_BUS_PCI || pATI->OptionIsPCI) ? TRUE : FALSE;
 
    if ( pATI->BusType != ATI_BUS_PCI && pATI->OptionIsPCI ) {
-       outm( AGP_BASE, 0 );
-       outm( AGP_CNTL, 0 );
        xf86DrvMsg(pScreen->myNum, X_CONFIG, "[dri] Forcing PCI mode\n");
    }
 
-   /* Check buffer size option for PCI, since it won't be done in ATIDRIAgpInit */
-   if ( pATIDRIServer->IsPCI) {
-      pATIDRIServer->bufferSize = ATI_DEFAULT_BUFFER_SIZE;
-      if (pATI->OptionBufferSize) {
-      	 if (pATI->OptionBufferSize < 1) {
-	    xf86DrvMsg( pScreen->myNum, X_ERROR, "[pci] Illegal DMA buffers size: %d MB\n",
-			pATI->OptionBufferSize );
-	    ATIDRICloseScreen( pScreen );
-	    return FALSE;
-	 }
-	 if (pATI->OptionBufferSize > 2) {
-	    xf86DrvMsg( pScreen->myNum, X_WARNING, "[pci] Illegal DMA buffers size: %d MB\n",
-			pATI->OptionBufferSize );
-	    xf86DrvMsg( pScreen->myNum, X_WARNING, "[pci] Clamping DMA buffers size to 2 MB\n");
-	    pATIDRIServer->bufferSize = 2;
-	 } else {
-	    pATIDRIServer->bufferSize = pATI->OptionBufferSize;
-	    xf86DrvMsg( pScreen->myNum, X_CONFIG, "[pci] Using %d MB DMA buffer size\n",
-			pATIDRIServer->bufferSize );
-	 }
-      } else {
-	 xf86DrvMsg( pScreen->myNum, X_DEFAULT, "[pci] Using %d MB DMA buffer size\n",
-		     pATIDRIServer->bufferSize );
-      }
-   }
-
    /* Initialize AGP */
    if ( !pATIDRIServer->IsPCI && !ATIDRIAgpInit( pScreen ) ) {
       pATIDRIServer->IsPCI = TRUE;
-      if ( pATI->BusType != ATI_BUS_PCI ) {
-	 outm( AGP_BASE, 0 );
-	 outm( AGP_CNTL, 0 );
-      }
       xf86DrvMsg( pScreen->myNum, X_WARNING, "[agp] AGP failed to initialize -- falling back to PCI mode.\n" );
       xf86DrvMsg( pScreen->myNum, X_WARNING, "[agp] Make sure you have the agpgart kernel module loaded.\n" );
    }
 
+   /* Initialize PCI */
+   if ( pATIDRIServer->IsPCI && !ATIDRIPciInit( pScreen ) ) {
+      ATIDRICloseScreen( pScreen );
+      return FALSE;
+   }
+
    if ( !ATIDRIMapInit( pScreen ) ) {
       ATIDRICloseScreen( pScreen );
       return FALSE;
@@ -1567,6 +1602,10 @@ void ATIDRICloseScreen( ScreenPtr pScreen )
       drmUnmap( pATIDRIServer->bufferMap, pATIDRIServer->bufferMapSize );
       pATIDRIServer->bufferMap = NULL;
    }
+   if ( pATIDRIServer->ringMap ) {
+      drmUnmap( pATIDRIServer->ringMap, pATIDRIServer->ringMapSize );
+      pATIDRIServer->ringMap = NULL;
+   }
    if ( pATIDRIServer->agpHandle ) {
       drmAgpUnbind( pATI->drmFD, pATIDRIServer->agpHandle );
       drmAgpFree( pATI->drmFD, pATIDRIServer->agpHandle );
@@ -1574,6 +1613,12 @@ void ATIDRICloseScreen( ScreenPtr pScreen )
       drmAgpRelease( pATI->drmFD );
    }
 
+   /* De-allocate all PCI resources */
+   if ( pATIDRIServer->IsPCI && pATIDRIServer->ringHandle ) {
+      drmRmMap( pATI->drmFD, pATIDRIServer->ringHandle );
+      pATIDRIServer->ringHandle = 0;
+   }
+
    /* De-allocate all DRI resources */
    DRICloseScreen( pScreen );
 
diff --git a/src/mach64_common.h b/src/mach64_common.h
index 1fb765a..f1f765a 100644
--- a/src/mach64_common.h
+++ b/src/mach64_common.h
@@ -111,7 +111,7 @@ typedef struct {
 } drmMach64Vertex;
 
 typedef struct {
-   int idx;
+   void *buf;
    int pitch;
    int offset;
    int format;