From cb32dde3be831096e98c66398159f7d7ddf3d672 Mon Sep 17 00:00:00 2001 From: Leif Delgass Date: Fri, 25 Apr 2003 19:42:47 +0000 Subject: Fix potential oops and memory leaks when allocations fail in addbufs_agp/pci. Add support for buffer private structs with PCI DMA buffers. Also some debug format string fixes. --- linux-core/drm_bufs.c | 90 +++++++++++++++++++++++++++++++++++++++------------ linux-core/drm_dma.c | 22 +++++++------ linux/drm_bufs.h | 90 +++++++++++++++++++++++++++++++++++++++------------ linux/drm_dma.h | 22 +++++++------ 4 files changed, 164 insertions(+), 60 deletions(-) diff --git a/linux-core/drm_bufs.c b/linux-core/drm_bufs.c index 97997dc1..0fb4376c 100644 --- a/linux-core/drm_bufs.c +++ b/linux-core/drm_bufs.c @@ -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 ); diff --git a/linux-core/drm_dma.c b/linux-core/drm_dma.c index 640e245d..7c1785b4 100644 --- a/linux-core/drm_dma.c +++ b/linux-core/drm_dma.c @@ -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 * diff --git a/linux/drm_bufs.h b/linux/drm_bufs.h index 97997dc1..0fb4376c 100644 --- a/linux/drm_bufs.h +++ b/linux/drm_bufs.h @@ -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 ); diff --git a/linux/drm_dma.h b/linux/drm_dma.h index 640e245d..7c1785b4 100644 --- a/linux/drm_dma.h +++ b/linux/drm_dma.h @@ -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 * -- cgit v1.2.3