Commit:     bdc3e603cda3433c2ccc2069d28f7f3cd319cfc6
Parent:     a2721e998ede079db10f65e4b42310f79dc8f135
Author:     Jesper Juhl <[EMAIL PROTECTED]>
AuthorDate: Mon Oct 15 10:24:05 2007 +1000
Committer:  Dave Airlie <[EMAIL PROTECTED](none)>
CommitDate: Mon Oct 15 10:32:15 2007 +1000

    fix use after free in amd create gatt pages
    Coverity spotted a "use after free" bug in
    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 <[EMAIL PROTECTED]>
    Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
    Signed-off-by: Dave Airlie <[EMAIL PROTECTED]>
 drivers/char/agp/amd-k7-agp.c |    9 ++-------
 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 f60bca7..1405a42 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)
        for (i = 0; i < nr_tables; i++) {
                entry = kzalloc(sizeof(struct amd_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 = amd_create_page_map(entry);
                if (retval != 0)
-       amd_irongate_private.num_tables = nr_tables;
+       amd_irongate_private.num_tables = i;
        amd_irongate_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