Commit:     190644e180794208bc638179f4d5940fe419bf9c
Parent:     d941cf5e373c356723fa648b9f0302a11c9b1770
Author:     Jesper Juhl <[EMAIL PROTECTED]>
AuthorDate: Sat Jul 21 17:39:11 2007 +0200
Committer:  Dave Airlie <[EMAIL PROTECTED]>
CommitDate: Fri Jul 27 10:44:32 2007 +1000

    Fix "use after free" / "double free" bug in ati_create_gatt_pages / 
    Coverity spotted a "use after free" bug in
    The same one that was in
    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 <[EMAIL PROTECTED]>
    Signed-off-by: Dave Airlie <[EMAIL PROTECTED]>
 drivers/char/agp/ati-agp.c |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/char/agp/ati-agp.c b/drivers/char/agp/ati-agp.c
index 780e59e..da7513d 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)
        for (i = 0; i < nr_tables; i++) {
                entry = kzalloc(sizeof(struct ati_page_map), GFP_KERNEL);
+               tables[i] = entry;
                if (entry == NULL) {
-                       while (i > 0) {
-                               kfree(tables[i-1]);
-                               i--;
-                       }
-                       kfree(tables);
                        retval = -ENOMEM;
-               tables[i] = entry;
                retval = ati_create_page_map(entry);
                if (retval != 0)
-       ati_generic_private.num_tables = nr_tables;
+       ati_generic_private.num_tables = i;
        ati_generic_private.gatt_pages = tables;
        if (retval != 0)
To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at

Reply via email to