aboutsummaryrefslogtreecommitdiffstats
path: root/drivers
diff options
context:
space:
mode:
authorJesper Juhl <jesper.juhl@gmail.com>2007-10-14 20:24:05 -0400
committerDave Airlie <airlied@optimus.(none)>2007-10-14 20:32:15 -0400
commitbdc3e603cda3433c2ccc2069d28f7f3cd319cfc6 (patch)
treeec5fed177ef0a7f07852f9f430fdcbd5e6275105 /drivers
parenta2721e998ede079db10f65e4b42310f79dc8f135 (diff)
fix use after free in amd create gatt pages
Coverity spotted a "use after free" bug in drivers/char/agp/amd-k7-agp.c::amd_create_gatt_pages(). The problem is this: If "entry = kzalloc(sizeof(struct amd_page_map), GFP_KERNEL);" fails, then there's a loop in the function to free all entries allocated so far and break out of the allocation loop. That in itself is pretty sane, but then the (now freed) 'tables' is assigned to amd_irongate_private.gatt_pages and 'retval' is set to -ENOMEM which causes amd_free_gatt_pages(); to be called at the end of the function. The problem with this is that amd_free_gatt_pages() will then loop 'amd_irongate_private.num_tables' times and try to free each entry in tables[] - this is bad since tables has already been freed and furthermore it will call kfree(tables) at the end - a double free. This patch removes the freeing loop in amd_create_gatt_pages() and instead relies entirely on the call to amd_free_gatt_pages() to free everything we allocated in case of an error. It also sets amd_irongate_private.num_tables to the actual number of entries allocated instead of just using the value passed in from the caller - this ensures that amd_free_gatt_pages() will only attempt to free stuff that was actually allocated. Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Dave Airlie <airlied@redhat.com>
Diffstat (limited to 'drivers')
-rw-r--r--drivers/char/agp/amd-k7-agp.c9
1 files changed, 2 insertions, 7 deletions
diff --git a/drivers/char/agp/amd-k7-agp.c b/drivers/char/agp/amd-k7-agp.c
index f60bca70d1fb..1405a42585e1 100644
--- a/drivers/char/agp/amd-k7-agp.c
+++ b/drivers/char/agp/amd-k7-agp.c
@@ -100,21 +100,16 @@ static int amd_create_gatt_pages(int nr_tables)
100 100
101 for (i = 0; i < nr_tables; i++) { 101 for (i = 0; i < nr_tables; i++) {
102 entry = kzalloc(sizeof(struct amd_page_map), GFP_KERNEL); 102 entry = kzalloc(sizeof(struct amd_page_map), GFP_KERNEL);
103 tables[i] = entry;
103 if (entry == NULL) { 104 if (entry == NULL) {
104 while (i > 0) {
105 kfree(tables[i-1]);
106 i--;
107 }
108 kfree(tables);
109 retval = -ENOMEM; 105 retval = -ENOMEM;
110 break; 106 break;
111 } 107 }
112 tables[i] = entry;
113 retval = amd_create_page_map(entry); 108 retval = amd_create_page_map(entry);
114 if (retval != 0) 109 if (retval != 0)
115 break; 110 break;
116 } 111 }
117 amd_irongate_private.num_tables = nr_tables; 112 amd_irongate_private.num_tables = i;
118 amd_irongate_private.gatt_pages = tables; 113 amd_irongate_private.gatt_pages = tables;
119 114
120 if (retval != 0) 115 if (retval != 0)