Fix potential oops and memory leaks when allocations fail in
authorLeif Delgass <ldelgass@users.sourceforge.net>
Fri, 25 Apr 2003 19:42:47 +0000 (19:42 +0000)
committerLeif Delgass <ldelgass@users.sourceforge.net>
Fri, 25 Apr 2003 19:42:47 +0000 (19:42 +0000)
    addbufs_agp/pci. Add support for buffer private structs with PCI DMA
    buffers. Also some debug format string fixes.

linux-core/drm_bufs.c
linux-core/drm_dma.c
linux/drm_bufs.h
linux/drm_dma.h

index 97997dc..0fb4376 100644 (file)
@@ -129,7 +129,7 @@ int DRM(addmap)( struct inode *inode, struct file *filp,
 
        case _DRM_SHM:
                map->handle = vmalloc_32(map->size);
-               DRM_DEBUG( "%ld %d %p\n",
+               DRM_DEBUG( "%lu %d %p\n",
                           map->size, DRM(order)( map->size ), map->handle );
                if ( !map->handle ) {
                        DRM(free)( map, sizeof(*map), DRM_MEM_MAPS );
@@ -270,9 +270,11 @@ static void DRM(cleanup_buf_error)(drm_buf_entry_t *entry)
 
        if (entry->seg_count) {
                for (i = 0; i < entry->seg_count; i++) {
-                       DRM(free_pages)(entry->seglist[i],
-                                       entry->page_order,
-                                       DRM_MEM_DMA);
+                       if (entry->seglist[i]) {
+                               DRM(free_pages)(entry->seglist[i],
+                                               entry->page_order,
+                                               DRM_MEM_DMA);
+                       }
                }
                DRM(free)(entry->seglist,
                          entry->seg_count *
@@ -282,9 +284,9 @@ static void DRM(cleanup_buf_error)(drm_buf_entry_t *entry)
                entry->seg_count = 0;
        }
 
-       if(entry->buf_count) {
-               for(i = 0; i < entry->buf_count; i++) {
-                       if(entry->buflist[i].dev_private) {
+       if (entry->buf_count) {
+               for (i = 0; i < entry->buf_count; i++) {
+                       if (entry->buflist[i].dev_private) {
                                DRM(free)(entry->buflist[i].dev_private,
                                          entry->buflist[i].dev_priv_size,
                                          DRM_MEM_BUFS);
@@ -346,7 +348,7 @@ int DRM(addbufs_agp)( struct inode *inode, struct file *filp,
        DRM_DEBUG( "count:      %d\n",  count );
        DRM_DEBUG( "order:      %d\n",  order );
        DRM_DEBUG( "size:       %d\n",  size );
-       DRM_DEBUG( "agp_offset: %ld\n", agp_offset );
+       DRM_DEBUG( "agp_offset: %lu\n", agp_offset );
        DRM_DEBUG( "alignment:  %d\n",  alignment );
        DRM_DEBUG( "page_order: %d\n",  page_order );
        DRM_DEBUG( "total:      %d\n",  total );
@@ -413,6 +415,9 @@ int DRM(addbufs_agp)( struct inode *inode, struct file *filp,
                        /* Set count correctly so we free the proper amount. */
                        entry->buf_count = count;
                        DRM(cleanup_buf_error)(entry);
+                       up( &dev->struct_sem );
+                       atomic_dec( &dev->buf_alloc );
+                       return -ENOMEM;
                }
                memset( buf->dev_private, 0, buf->dev_priv_size );
 
@@ -560,12 +565,13 @@ int DRM(addbufs_pci)( struct inode *inode, struct file *filp,
        }
        memset( entry->seglist, 0, count * sizeof(*entry->seglist) );
 
-       temp_pagelist = DRM(realloc)( dma->pagelist,
-                                     dma->page_count * sizeof(*dma->pagelist),
-                                     (dma->page_count + (count << page_order))
-                                     * sizeof(*dma->pagelist),
-                                     DRM_MEM_PAGES );
-       if(!temp_pagelist) {
+       /* Keep the original pagelist until we know all the allocations
+        * have succeeded
+        */
+       temp_pagelist = DRM(alloc)( (dma->page_count + (count << page_order))
+                                   * sizeof(*dma->pagelist),
+                                   DRM_MEM_PAGES );
+       if (!temp_pagelist) {
                DRM(free)( entry->buflist,
                           count * sizeof(*entry->buflist),
                           DRM_MEM_BUFS );
@@ -576,8 +582,9 @@ int DRM(addbufs_pci)( struct inode *inode, struct file *filp,
                atomic_dec( &dev->buf_alloc );
                return -ENOMEM;
        }
-
-       dma->pagelist = temp_pagelist;
+       memcpy(temp_pagelist,
+              dma->pagelist,
+              dma->page_count * sizeof(*dma->pagelist));
        DRM_DEBUG( "pagelist: %d entries\n",
                   dma->page_count + (count << page_order) );
 
@@ -588,13 +595,25 @@ int DRM(addbufs_pci)( struct inode *inode, struct file *filp,
 
        while ( entry->buf_count < count ) {
                page = DRM(alloc_pages)( page_order, DRM_MEM_DMA );
-               if ( !page ) break;
+               if ( !page ) {
+                       /* Set count correctly so we free the proper amount. */
+                       entry->buf_count = count;
+                       entry->seg_count = count;
+                       DRM(cleanup_buf_error)(entry);
+                       DRM(free)( temp_pagelist,
+                                  (dma->page_count + (count << page_order))
+                                  * sizeof(*dma->pagelist),
+                                  DRM_MEM_PAGES );
+                       up( &dev->struct_sem );
+                       atomic_dec( &dev->buf_alloc );
+                       return -ENOMEM;
+               }
                entry->seglist[entry->seg_count++] = page;
                for ( i = 0 ; i < (1 << page_order) ; i++ ) {
                        DRM_DEBUG( "page %d @ 0x%08lx\n",
                                   dma->page_count + page_count,
                                   page + PAGE_SIZE * i );
-                       dma->pagelist[dma->page_count + page_count++]
+                       temp_pagelist[dma->page_count + page_count++]
                                = page + PAGE_SIZE * i;
                }
                for ( offset = 0 ;
@@ -612,6 +631,25 @@ int DRM(addbufs_pci)( struct inode *inode, struct file *filp,
                        buf->pending = 0;
                        init_waitqueue_head( &buf->dma_wait );
                        buf->filp    = 0;
+
+                       buf->dev_priv_size = sizeof(DRIVER_BUF_PRIV_T);
+                       buf->dev_private = DRM(alloc)( sizeof(DRIVER_BUF_PRIV_T),
+                                                      DRM_MEM_BUFS );
+                       if(!buf->dev_private) {
+                               /* Set count correctly so we free the proper amount. */
+                               entry->buf_count = count;
+                               entry->seg_count = count;
+                               DRM(cleanup_buf_error)(entry);
+                               DRM(free)( temp_pagelist,
+                                          (dma->page_count + (count << page_order))
+                                          * sizeof(*dma->pagelist),
+                                          DRM_MEM_PAGES );
+                               up( &dev->struct_sem );
+                               atomic_dec( &dev->buf_alloc );
+                               return -ENOMEM;
+                       }
+                       memset( buf->dev_private, 0, buf->dev_priv_size );
+
                        DRM_DEBUG( "buffer %d @ %p\n",
                                   entry->buf_count, buf->address );
                }
@@ -623,9 +661,13 @@ int DRM(addbufs_pci)( struct inode *inode, struct file *filp,
                                     (dma->buf_count + entry->buf_count)
                                     * sizeof(*dma->buflist),
                                     DRM_MEM_BUFS );
-       if(!temp_buflist) {
+       if (!temp_buflist) {
                /* Free the entry because it isn't valid */
                DRM(cleanup_buf_error)(entry);
+               DRM(free)( temp_pagelist,
+                          (dma->page_count + (count << page_order))
+                          * sizeof(*dma->pagelist),
+                          DRM_MEM_PAGES );
                up( &dev->struct_sem );
                atomic_dec( &dev->buf_alloc );
                return -ENOMEM;
@@ -636,6 +678,14 @@ int DRM(addbufs_pci)( struct inode *inode, struct file *filp,
                dma->buflist[i + dma->buf_count] = &entry->buflist[i];
        }
 
+       /* No allocations failed, so now we can replace the orginal pagelist
+        * with the new one.
+        */
+       DRM(free)(dma->pagelist,
+                 dma->page_count * sizeof(*dma->pagelist),
+                 DRM_MEM_PAGES);
+       dma->pagelist = temp_pagelist;
+
        dma->buf_count += entry->buf_count;
        dma->seg_count += entry->seg_count;
        dma->page_count += entry->seg_count << page_order;
@@ -704,7 +754,7 @@ int DRM(addbufs_sg)( struct inode *inode, struct file *filp,
        DRM_DEBUG( "count:      %d\n",  count );
        DRM_DEBUG( "order:      %d\n",  order );
        DRM_DEBUG( "size:       %d\n",  size );
-       DRM_DEBUG( "agp_offset: %ld\n", agp_offset );
+       DRM_DEBUG( "agp_offset: %lu\n", agp_offset );
        DRM_DEBUG( "alignment:  %d\n",  alignment );
        DRM_DEBUG( "page_order: %d\n",  page_order );
        DRM_DEBUG( "total:      %d\n",  total );
index 640e245..7c1785b 100644 (file)
@@ -84,22 +84,24 @@ void DRM(dma_takedown)(drm_device_t *dev)
                                  dma->bufs[i].buf_count,
                                  dma->bufs[i].seg_count);
                        for (j = 0; j < dma->bufs[i].seg_count; j++) {
-                               DRM(free_pages)(dma->bufs[i].seglist[j],
-                                               dma->bufs[i].page_order,
-                                               DRM_MEM_DMA);
+                               if (dma->bufs[i].seglist[j]) {
+                                       DRM(free_pages)(dma->bufs[i].seglist[j],
+                                                       dma->bufs[i].page_order,
+                                                       DRM_MEM_DMA);
+                               }
                        }
                        DRM(free)(dma->bufs[i].seglist,
                                  dma->bufs[i].seg_count
                                  * sizeof(*dma->bufs[0].seglist),
                                  DRM_MEM_SEGS);
                }
-               if(dma->bufs[i].buf_count) {
-                       for(j = 0; j < dma->bufs[i].buf_count; j++) {
-                          if(dma->bufs[i].buflist[j].dev_private) {
-                             DRM(free)(dma->bufs[i].buflist[j].dev_private,
-                                       dma->bufs[i].buflist[j].dev_priv_size,
-                                       DRM_MEM_BUFS);
-                          }
+               if (dma->bufs[i].buf_count) {
+                       for (j = 0; j < dma->bufs[i].buf_count; j++) {
+                               if (dma->bufs[i].buflist[j].dev_private) {
+                                       DRM(free)(dma->bufs[i].buflist[j].dev_private,
+                                                 dma->bufs[i].buflist[j].dev_priv_size,
+                                                 DRM_MEM_BUFS);
+                               }
                        }
                        DRM(free)(dma->bufs[i].buflist,
                                  dma->bufs[i].buf_count *
index 97997dc..0fb4376 100644 (file)
@@ -129,7 +129,7 @@ int DRM(addmap)( struct inode *inode, struct file *filp,
 
        case _DRM_SHM:
                map->handle = vmalloc_32(map->size);
-               DRM_DEBUG( "%ld %d %p\n",
+               DRM_DEBUG( "%lu %d %p\n",
                           map->size, DRM(order)( map->size ), map->handle );
                if ( !map->handle ) {
                        DRM(free)( map, sizeof(*map), DRM_MEM_MAPS );
@@ -270,9 +270,11 @@ static void DRM(cleanup_buf_error)(drm_buf_entry_t *entry)
 
        if (entry->seg_count) {
                for (i = 0; i < entry->seg_count; i++) {
-                       DRM(free_pages)(entry->seglist[i],
-                                       entry->page_order,
-                                       DRM_MEM_DMA);
+                       if (entry->seglist[i]) {
+                               DRM(free_pages)(entry->seglist[i],
+                                               entry->page_order,
+                                               DRM_MEM_DMA);
+                       }
                }
                DRM(free)(entry->seglist,
                          entry->seg_count *
@@ -282,9 +284,9 @@ static void DRM(cleanup_buf_error)(drm_buf_entry_t *entry)
                entry->seg_count = 0;
        }
 
-       if(entry->buf_count) {
-               for(i = 0; i < entry->buf_count; i++) {
-                       if(entry->buflist[i].dev_private) {
+       if (entry->buf_count) {
+               for (i = 0; i < entry->buf_count; i++) {
+                       if (entry->buflist[i].dev_private) {
                                DRM(free)(entry->buflist[i].dev_private,
                                          entry->buflist[i].dev_priv_size,
                                          DRM_MEM_BUFS);
@@ -346,7 +348,7 @@ int DRM(addbufs_agp)( struct inode *inode, struct file *filp,
        DRM_DEBUG( "count:      %d\n",  count );
        DRM_DEBUG( "order:      %d\n",  order );
        DRM_DEBUG( "size:       %d\n",  size );
-       DRM_DEBUG( "agp_offset: %ld\n", agp_offset );
+       DRM_DEBUG( "agp_offset: %lu\n", agp_offset );
        DRM_DEBUG( "alignment:  %d\n",  alignment );
        DRM_DEBUG( "page_order: %d\n",  page_order );
        DRM_DEBUG( "total:      %d\n",  total );
@@ -413,6 +415,9 @@ int DRM(addbufs_agp)( struct inode *inode, struct file *filp,
                        /* Set count correctly so we free the proper amount. */
                        entry->buf_count = count;
                        DRM(cleanup_buf_error)(entry);
+                       up( &dev->struct_sem );
+                       atomic_dec( &dev->buf_alloc );
+                       return -ENOMEM;
                }
                memset( buf->dev_private, 0, buf->dev_priv_size );
 
@@ -560,12 +565,13 @@ int DRM(addbufs_pci)( struct inode *inode, struct file *filp,
        }
        memset( entry->seglist, 0, count * sizeof(*entry->seglist) );
 
-       temp_pagelist = DRM(realloc)( dma->pagelist,
-                                     dma->page_count * sizeof(*dma->pagelist),
-                                     (dma->page_count + (count << page_order))
-                                     * sizeof(*dma->pagelist),
-                                     DRM_MEM_PAGES );
-       if(!temp_pagelist) {
+       /* Keep the original pagelist until we know all the allocations
+        * have succeeded
+        */
+       temp_pagelist = DRM(alloc)( (dma->page_count + (count << page_order))
+                                   * sizeof(*dma->pagelist),
+                                   DRM_MEM_PAGES );
+       if (!temp_pagelist) {
                DRM(free)( entry->buflist,
                           count * sizeof(*entry->buflist),
                           DRM_MEM_BUFS );
@@ -576,8 +582,9 @@ int DRM(addbufs_pci)( struct inode *inode, struct file *filp,
                atomic_dec( &dev->buf_alloc );
                return -ENOMEM;
        }
-
-       dma->pagelist = temp_pagelist;
+       memcpy(temp_pagelist,
+              dma->pagelist,
+              dma->page_count * sizeof(*dma->pagelist));
        DRM_DEBUG( "pagelist: %d entries\n",
                   dma->page_count + (count << page_order) );
 
@@ -588,13 +595,25 @@ int DRM(addbufs_pci)( struct inode *inode, struct file *filp,
 
        while ( entry->buf_count < count ) {
                page = DRM(alloc_pages)( page_order, DRM_MEM_DMA );
-               if ( !page ) break;
+               if ( !page ) {
+                       /* Set count correctly so we free the proper amount. */
+                       entry->buf_count = count;
+                       entry->seg_count = count;
+                       DRM(cleanup_buf_error)(entry);
+                       DRM(free)( temp_pagelist,
+                                  (dma->page_count + (count << page_order))
+                                  * sizeof(*dma->pagelist),
+                                  DRM_MEM_PAGES );
+                       up( &dev->struct_sem );
+                       atomic_dec( &dev->buf_alloc );
+                       return -ENOMEM;
+               }
                entry->seglist[entry->seg_count++] = page;
                for ( i = 0 ; i < (1 << page_order) ; i++ ) {
                        DRM_DEBUG( "page %d @ 0x%08lx\n",
                                   dma->page_count + page_count,
                                   page + PAGE_SIZE * i );
-                       dma->pagelist[dma->page_count + page_count++]
+                       temp_pagelist[dma->page_count + page_count++]
                                = page + PAGE_SIZE * i;
                }
                for ( offset = 0 ;
@@ -612,6 +631,25 @@ int DRM(addbufs_pci)( struct inode *inode, struct file *filp,
                        buf->pending = 0;
                        init_waitqueue_head( &buf->dma_wait );
                        buf->filp    = 0;
+
+                       buf->dev_priv_size = sizeof(DRIVER_BUF_PRIV_T);
+                       buf->dev_private = DRM(alloc)( sizeof(DRIVER_BUF_PRIV_T),
+                                                      DRM_MEM_BUFS );
+                       if(!buf->dev_private) {
+                               /* Set count correctly so we free the proper amount. */
+                               entry->buf_count = count;
+                               entry->seg_count = count;
+                               DRM(cleanup_buf_error)(entry);
+                               DRM(free)( temp_pagelist,
+                                          (dma->page_count + (count << page_order))
+                                          * sizeof(*dma->pagelist),
+                                          DRM_MEM_PAGES );
+                               up( &dev->struct_sem );
+                               atomic_dec( &dev->buf_alloc );
+                               return -ENOMEM;
+                       }
+                       memset( buf->dev_private, 0, buf->dev_priv_size );
+
                        DRM_DEBUG( "buffer %d @ %p\n",
                                   entry->buf_count, buf->address );
                }
@@ -623,9 +661,13 @@ int DRM(addbufs_pci)( struct inode *inode, struct file *filp,
                                     (dma->buf_count + entry->buf_count)
                                     * sizeof(*dma->buflist),
                                     DRM_MEM_BUFS );
-       if(!temp_buflist) {
+       if (!temp_buflist) {
                /* Free the entry because it isn't valid */
                DRM(cleanup_buf_error)(entry);
+               DRM(free)( temp_pagelist,
+                          (dma->page_count + (count << page_order))
+                          * sizeof(*dma->pagelist),
+                          DRM_MEM_PAGES );
                up( &dev->struct_sem );
                atomic_dec( &dev->buf_alloc );
                return -ENOMEM;
@@ -636,6 +678,14 @@ int DRM(addbufs_pci)( struct inode *inode, struct file *filp,
                dma->buflist[i + dma->buf_count] = &entry->buflist[i];
        }
 
+       /* No allocations failed, so now we can replace the orginal pagelist
+        * with the new one.
+        */
+       DRM(free)(dma->pagelist,
+                 dma->page_count * sizeof(*dma->pagelist),
+                 DRM_MEM_PAGES);
+       dma->pagelist = temp_pagelist;
+
        dma->buf_count += entry->buf_count;
        dma->seg_count += entry->seg_count;
        dma->page_count += entry->seg_count << page_order;
@@ -704,7 +754,7 @@ int DRM(addbufs_sg)( struct inode *inode, struct file *filp,
        DRM_DEBUG( "count:      %d\n",  count );
        DRM_DEBUG( "order:      %d\n",  order );
        DRM_DEBUG( "size:       %d\n",  size );
-       DRM_DEBUG( "agp_offset: %ld\n", agp_offset );
+       DRM_DEBUG( "agp_offset: %lu\n", agp_offset );
        DRM_DEBUG( "alignment:  %d\n",  alignment );
        DRM_DEBUG( "page_order: %d\n",  page_order );
        DRM_DEBUG( "total:      %d\n",  total );
index 640e245..7c1785b 100644 (file)
@@ -84,22 +84,24 @@ void DRM(dma_takedown)(drm_device_t *dev)
                                  dma->bufs[i].buf_count,
                                  dma->bufs[i].seg_count);
                        for (j = 0; j < dma->bufs[i].seg_count; j++) {
-                               DRM(free_pages)(dma->bufs[i].seglist[j],
-                                               dma->bufs[i].page_order,
-                                               DRM_MEM_DMA);
+                               if (dma->bufs[i].seglist[j]) {
+                                       DRM(free_pages)(dma->bufs[i].seglist[j],
+                                                       dma->bufs[i].page_order,
+                                                       DRM_MEM_DMA);
+                               }
                        }
                        DRM(free)(dma->bufs[i].seglist,
                                  dma->bufs[i].seg_count
                                  * sizeof(*dma->bufs[0].seglist),
                                  DRM_MEM_SEGS);
                }
-               if(dma->bufs[i].buf_count) {
-                       for(j = 0; j < dma->bufs[i].buf_count; j++) {
-                          if(dma->bufs[i].buflist[j].dev_private) {
-                             DRM(free)(dma->bufs[i].buflist[j].dev_private,
-                                       dma->bufs[i].buflist[j].dev_priv_size,
-                                       DRM_MEM_BUFS);
-                          }
+               if (dma->bufs[i].buf_count) {
+                       for (j = 0; j < dma->bufs[i].buf_count; j++) {
+                               if (dma->bufs[i].buflist[j].dev_private) {
+                                       DRM(free)(dma->bufs[i].buflist[j].dev_private,
+                                                 dma->bufs[i].buflist[j].dev_priv_size,
+                                                 DRM_MEM_BUFS);
+                               }
                        }
                        DRM(free)(dma->bufs[i].buflist,
                                  dma->bufs[i].buf_count *