Fix drm_remove_magic potential memory leak / corruption. Move drm
authorThomas Hellstrom <thomas@tungstengraphics.com>
Tue, 6 Jun 2006 17:46:17 +0000 (17:46 +0000)
committerThomas Hellstrom <thomas@tungstengraphics.com>
Tue, 6 Jun 2006 17:46:17 +0000 (17:46 +0000)
    authentication token hashing to new generic hash table implementation.

linux-core/drmP.h
linux-core/drm_auth.c
linux-core/drm_drv.c
linux-core/drm_fops.c

index ec72ebc..6c56b7d 100644 (file)
@@ -86,6 +86,7 @@
 #define __OS_HAS_MTRR (defined(CONFIG_MTRR))
 
 #include "drm_os_linux.h"
+#include "drm_hashtab.h"
 
 /* If you want the memory alloc debug functionality, change define below */
 /* #define DEBUG_MEMORY */
 #define DRM_DEBUG_CODE 2         /**< Include debugging code if > 1, then
                                     also include looping detection. */
 
-#define DRM_HASH_SIZE        16 /**< Size of key hash table. Must be power of 2. */
+#define DRM_MAGIC_HASH_ORDER  4 /**< Size of key hash table. Must be power of 2. */
 #define DRM_KERNEL_CONTEXT    0         /**< Change drm_resctx if changed */
 #define DRM_RESERVED_CONTEXTS 1         /**< Change drm_resctx if changed */
 #define DRM_LOOPING_LIMIT     5000000
@@ -293,9 +294,9 @@ typedef struct drm_devstate {
 } drm_devstate_t;
 
 typedef struct drm_magic_entry {
-       drm_magic_t magic;
+        drm_hash_item_t hash_item;
+        struct list_head head;
        struct drm_file *priv;
-       struct drm_magic_entry *next;
 } drm_magic_entry_t;
 
 typedef struct drm_magic_head {
@@ -670,7 +671,8 @@ typedef struct drm_device {
        /*@{ */
        drm_file_t *file_first;         /**< file list head */
        drm_file_t *file_last;          /**< file list tail */
-       drm_magic_head_t magiclist[DRM_HASH_SIZE]; /**< magic hash table */
+        drm_open_hash_t magiclist;
+        struct list_head magicfree;
        /*@} */
 
        /** \name Memory management */
index 591c33c..2857c94 100644 (file)
 #include "drmP.h"
 
 /**
- * Generate a hash key from a magic.
- *
- * \param magic magic.
- * \return hash key.
- *
- * The key is the modulus of the hash table size, #DRM_HASH_SIZE, which must be
- * a power of 2.
- */
-static int drm_hash_magic(drm_magic_t magic)
-{
-       return magic & (DRM_HASH_SIZE - 1);
-}
-
-/**
  * Find the file with the given magic number.
  *
  * \param dev DRM device.
@@ -63,15 +49,13 @@ static drm_file_t *drm_find_file(drm_device_t * dev, drm_magic_t magic)
 {
        drm_file_t *retval = NULL;
        drm_magic_entry_t *pt;
-       int hash = drm_hash_magic(magic);
+        drm_hash_item_t *hash;
 
-       down(&dev->struct_sem);
-       for (pt = dev->magiclist[hash].head; pt; pt = pt->next) {
-               if (pt->magic == magic) {
-                       retval = pt->priv;
-                       break;
-               }
-       }
+       down(&dev->struct_sem);        
+        if (!drm_ht_find_item(&dev->magiclist, (unsigned long) magic, &hash)) {
+                pt = list_entry(hash, drm_magic_entry_t, hash_item);
+                retval = pt->priv;
+        }
        up(&dev->struct_sem);
        return retval;
 }
@@ -90,28 +74,19 @@ static drm_file_t *drm_find_file(drm_device_t * dev, drm_magic_t magic)
 static int drm_add_magic(drm_device_t *dev, drm_file_t *priv,
                         drm_magic_t magic)
 {
-       int hash;
        drm_magic_entry_t *entry;
 
        DRM_DEBUG("%d\n", magic);
 
-       hash = drm_hash_magic(magic);
        entry = drm_alloc(sizeof(*entry), DRM_MEM_MAGIC);
        if (!entry)
                return -ENOMEM;
        memset(entry, 0, sizeof(*entry));
-       entry->magic = magic;
        entry->priv = priv;
-       entry->next = NULL;
-
-       down(&dev->struct_sem);
-       if (dev->magiclist[hash].tail) {
-               dev->magiclist[hash].tail->next = entry;
-               dev->magiclist[hash].tail = entry;
-       } else {
-               dev->magiclist[hash].head = entry;
-               dev->magiclist[hash].tail = entry;
-       }
+        entry->hash_item.key = (unsigned long) magic;
+        down(&dev->struct_sem);
+        drm_ht_insert_item(&dev->magiclist, &entry->hash_item);
+        list_add_tail(&entry->head, &dev->magicfree);
        up(&dev->struct_sem);
 
        return 0;
@@ -128,34 +103,24 @@ static int drm_add_magic(drm_device_t *dev, drm_file_t *priv,
  */
 static int drm_remove_magic(drm_device_t * dev, drm_magic_t magic)
 {
-       drm_magic_entry_t *prev = NULL;
        drm_magic_entry_t *pt;
-       int hash;
+        drm_hash_item_t *hash;
 
        DRM_DEBUG("%d\n", magic);
-       hash = drm_hash_magic(magic);
 
        down(&dev->struct_sem);
-       for (pt = dev->magiclist[hash].head; pt; prev = pt, pt = pt->next) {
-               if (pt->magic == magic) {
-                       if (dev->magiclist[hash].head == pt) {
-                               dev->magiclist[hash].head = pt->next;
-                       }
-                       if (dev->magiclist[hash].tail == pt) {
-                               dev->magiclist[hash].tail = prev;
-                       }
-                       if (prev) {
-                               prev->next = pt->next;
-                       }
-                       up(&dev->struct_sem);
-                       return 0;
-               }
-       }
-       up(&dev->struct_sem);
+        if (drm_ht_find_item(&dev->magiclist, (unsigned long) magic, &hash)) {
+                up(&dev->struct_sem);
+                return -EINVAL;
+        }
+        pt = list_entry(hash, drm_magic_entry_t, hash_item);
+        drm_ht_remove_item(&dev->magiclist, hash);
+        list_del(&pt->head);
+        up(&dev->struct_sem);
 
        drm_free(pt, sizeof(*pt), DRM_MEM_MAGIC);
 
-       return -EINVAL;
+       return 0;
 }
 
 /**
index 127e21e..07f9952 100644 (file)
@@ -163,14 +163,12 @@ int drm_lastclose(drm_device_t * dev)
                dev->unique_len = 0;
        }
 
-       /* Clear pid list */
-       for (i = 0; i < DRM_HASH_SIZE; i++) {
-               for (pt = dev->magiclist[i].head; pt; pt = next) {
-                       next = pt->next;
-                       drm_free(pt, sizeof(*pt), DRM_MEM_MAGIC);
-               }
-               dev->magiclist[i].head = dev->magiclist[i].tail = NULL;
-       }
+        list_for_each_entry_safe(pt, next, &dev->magicfree, head) {
+                list_del(&pt->head);
+                drm_ht_remove_item(&dev->magiclist, &pt->hash_item);
+                drm_free(pt, sizeof(*pt), DRM_MEM_MAGIC);
+        }
+
 
        /* Clear AGP information */
        if (drm_core_has_AGP(dev) && dev->agp) {
index a196213..632108d 100644 (file)
@@ -72,10 +72,8 @@ static int drm_setup(drm_device_t * dev)
        for (i = 0; i < DRM_ARRAY_SIZE(dev->counts); i++)
                atomic_set(&dev->counts[i], 0);
 
-       for (i = 0; i < DRM_HASH_SIZE; i++) {
-               dev->magiclist[i].head = NULL;
-               dev->magiclist[i].tail = NULL;
-       }
+        drm_ht_create(&dev->magiclist, DRM_MAGIC_HASH_ORDER);
+        INIT_LIST_HEAD(&dev->magicfree);
 
        dev->ctxlist = drm_alloc(sizeof(*dev->ctxlist), DRM_MEM_CTXLIST);
        if (dev->ctxlist == NULL)