aboutsummaryrefslogtreecommitdiffstats
path: root/drivers/char/agp
diff options
context:
space:
mode:
authorJesper Juhl <jesper.juhl@gmail.com>2007-07-21 11:39:11 -0400
committerDave Airlie <airlied@redhat.com>2007-07-26 20:44:32 -0400
commit190644e180794208bc638179f4d5940fe419bf9c (patch)
treedada81603601e8af69f10f7802ffc3cb4dc1b381 /drivers/char/agp
parentd941cf5e373c356723fa648b9f0302a11c9b1770 (diff)
Fix "use after free" / "double free" bug in ati_create_gatt_pages / ati_free_gatt_pages
Hi, Coverity spotted a "use after free" bug in drivers/char/agp/ati-agp.c::ati_create_gatt_pages(). The same one that was in drivers/char/agp/amd-k7-agp.c::amd_create_gatt_pages() The problem is this: If "entry = kzalloc(sizeof(struct ati_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 ati_generic_private.gatt_pages and 'retval' is set to -ENOMEM which causes ati_free_gatt_pages(); to be called at the end of the function. The problem with this is that ati_free_gatt_pages() will then loop 'ati_generic_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 ati_create_gatt_pages() and instead relies entirely on the call to ati_free_gatt_pages() to free everything we allocated in case of an error. It also sets ati_generic_private.num_tables to the actual number of entries allocated instead of just using the value passed in from the caller - this ensures that ati_free_gatt_pages() will only attempt to free stuff that was actually allocated. Note: I'm in no way intimate with this code and I have no way to actually test this patch (besides compile test it), so while I've tried to be careful in reading the code and make sure the patch does the right thing an ACK from someone who actually knows the code in-depth would be very much appreciated. Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com> Signed-off-by: Dave Airlie <airlied@linux.ie>
Diffstat (limited to 'drivers/char/agp')
-rw-r--r--drivers/char/agp/ati-agp.c9
1 files changed, 2 insertions, 7 deletions
diff --git a/drivers/char/agp/ati-agp.c b/drivers/char/agp/ati-agp.c
index 780e59e588ad..da7513d7b4e7 100644
--- a/drivers/char/agp/ati-agp.c
+++ b/drivers/char/agp/ati-agp.c
@@ -123,21 +123,16 @@ static int ati_create_gatt_pages(int nr_tables)
123 123
124 for (i = 0; i < nr_tables; i++) { 124 for (i = 0; i < nr_tables; i++) {
125 entry = kzalloc(sizeof(struct ati_page_map), GFP_KERNEL); 125 entry = kzalloc(sizeof(struct ati_page_map), GFP_KERNEL);
126 tables[i] = entry;
126 if (entry == NULL) { 127 if (entry == NULL) {
127 while (i > 0) {
128 kfree(tables[i-1]);
129 i--;
130 }
131 kfree(tables);
132 retval = -ENOMEM; 128 retval = -ENOMEM;
133 break; 129 break;
134 } 130 }
135 tables[i] = entry;
136 retval = ati_create_page_map(entry); 131 retval = ati_create_page_map(entry);
137 if (retval != 0) 132 if (retval != 0)
138 break; 133 break;
139 } 134 }
140 ati_generic_private.num_tables = nr_tables; 135 ati_generic_private.num_tables = i;
141 ati_generic_private.gatt_pages = tables; 136 ati_generic_private.gatt_pages = tables;
142 137
143 if (retval != 0) 138 if (retval != 0)