nfs: refcount each auth_cache_entry and related data_t

This makes sure that all the auth_cache_entry structures are only free'd
when there is no reference to it anymore. When it is free'd, the
associated data_t from the auth_cache->cache_dict gets unref'd too.

Upon calling auth_cache_purge(), the auth_cache->cache_dict will free
each auth_cache_entry in a secure way.

Change-Id: If097cc11838e43599040f5414f82b30fc0fd40c6
BUG: 1226717
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Reviewed-on: http://review.gluster.org/11023
Reviewed-by: Xavier Hernandez <xhernandez@datalab.es>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Tested-by: NetBSD Build System <jenkins@build.gluster.org>

1 files changed, 86 insertions, 9 deletions

diff --git a/xlators/nfs/server/src/auth-cache.c b/xlators/nfs/server/src/auth-cache.c
index ebaf72594b0..2d9af4e8c79 100644
--- a/xlators/nfs/server/src/auth-cache.c
+++ b/xlators/nfs/server/src/auth-cache.c
@@ -11,6 +11,7 @@
    cases as published by the Free Software Foundation.
 */
 
+#include "refcount.h"
 #include "auth-cache.h"
 #include "nfs3.h"
 #include "exports.h"
@@ -23,7 +24,10 @@ enum auth_cache_lookup_results {
 };
 
 struct auth_cache_entry {
-        time_t timestamp;
+        GF_REF_DECL;                    /* refcounting */
+        data_t             *data;       /* data_unref() on refcount == 0 */
+
+        time_t              timestamp;
         struct export_item *item;
 };
 
@@ -82,6 +86,29 @@ out:
         return cache;
 }
 
+/* auth_cache_entry_free -- called by refcounting subsystem on refcount == 0
+ *
+ * @to_free: auth_cache_entry that has refcount == 0 and needs to get free'd
+ */
+void
+auth_cache_entry_free (void *to_free)
+{
+        struct auth_cache_entry *entry      = to_free;
+        data_t                  *entry_data = NULL;
+
+        GF_VALIDATE_OR_GOTO (GF_NFS, entry, out);
+        GF_VALIDATE_OR_GOTO (GF_NFS, entry->data, out);
+
+        entry_data = entry->data;
+        /* set data_t->data to NULL, otherwise data_unref() tries to free it */
+        entry_data->data = NULL;
+        data_unref (entry_data);
+
+        GF_FREE (entry);
+out:
+        return;
+}
+
 /**
  * auth_cache_entry_init -- Initialize an auth cache entry
  *
@@ -97,6 +124,8 @@ auth_cache_entry_init ()
         if (!entry)
                 gf_msg (GF_NFS, GF_LOG_WARNING, ENOMEM, NFS_MSG_NO_MEMORY,
                         "failed to allocate entry");
+        else
+                GF_REF_INIT (entry, auth_cache_entry_free);
 
         return entry;
 }
@@ -116,17 +145,34 @@ auth_cache_add (struct auth_cache *cache, char *hashkey,
         GF_VALIDATE_OR_GOTO (GF_NFS, cache, out);
         GF_VALIDATE_OR_GOTO (GF_NFS, cache->cache_dict, out);
 
+        ret = GF_REF_GET (entry);
+        if (ret == 0) {
+                /* entry does not have any references */
+                ret = -1;
+                goto out;
+        }
+
         entry_data = bin_to_data (entry, sizeof (*entry));
         if (!entry_data) {
+                ret = -1;
+                GF_REF_PUT (entry);
                 goto out;
         }
 
+        /* we'll take an extra ref on the data_t, it gets unref'd when the
+         * auth_cache_entry is released */
+        entry->data = data_ref (entry_data);
+
         LOCK (&cache->lock);
         {
                 ret = dict_set (cache->cache_dict, hashkey, entry_data);
         }
         UNLOCK (&cache->lock);
 
+        if (ret) {
+                /* adding to dict failed */
+                GF_REF_PUT (entry);
+        }
 out:
         return ret;
 }
@@ -167,6 +213,7 @@ auth_cache_get (struct auth_cache *cache, char *hashkey,
 
         GF_VALIDATE_OR_GOTO (GF_NFS, cache, out);
         GF_VALIDATE_OR_GOTO (GF_NFS, cache->cache_dict, out);
+        GF_VALIDATE_OR_GOTO (GF_NFS, hashkey, out);
 
         LOCK (&cache->lock);
         {
@@ -174,8 +221,13 @@ auth_cache_get (struct auth_cache *cache, char *hashkey,
                 if (!entry_data)
                         goto unlock;
 
-                /* TODO: refcount++ on lookup_res */
                 lookup_res = (struct auth_cache_entry *)(entry_data->data);
+                if (GF_REF_GET (lookup_res) == 0) {
+                        /* entry has been free'd */
+                        ret = ENTRY_EXPIRED;
+                        goto unlock;
+                }
+
                 if (_auth_cache_expired (cache, lookup_res)) {
                         ret = ENTRY_EXPIRED;
 
@@ -239,7 +291,7 @@ auth_cache_lookup (struct auth_cache *cache, struct nfs3_fh *fh,
         case ENTRY_FOUND:
                 *timestamp = lookup_res->timestamp;
                 *can_write = lookup_res->item->opts->rw;
-                /* TODO: refcount-- lookup_res */
+                GF_REF_PUT (lookup_res);
                 break;
 
         case ENTRY_NOT_FOUND:
@@ -259,6 +311,28 @@ out:
         return ret;
 }
 
+/* auth_cache_entry_purge -- free up the auth_cache_entry
+ *
+ * This gets called through dict_foreach() by auth_cache_purge(). Each
+ * auth_cache_entry has a refcount which needs to be decremented. Once the
+ * auth_cache_entry reaches refcount == 0, auth_cache_entry_free() will call
+ * data_unref() to free the associated data_t.
+ *
+ * @d: dict that gets purged by auth_cache_purge()
+ * @k: hashkey of the current entry
+ * @v: data_t of the current entry
+ */
+int
+auth_cache_entry_purge (dict_t *d, char *k, data_t *v, void *_unused)
+{
+        struct auth_cache_entry *entry = (struct auth_cache_entry *) v->data;
+
+        if (entry)
+                GF_REF_PUT (entry);
+
+        return 0;
+}
+
 /**
  * auth_cache_purge -- Purge the dict in the cache and create a new empty one.
  *
@@ -277,11 +351,13 @@ auth_cache_purge (struct auth_cache *cache)
         LOCK (&cache->lock);
         {
                 old_cache_dict = cache->cache_dict;
-                (void) __sync_lock_test_and_set (&cache->cache_dict,
-                                                 new_cache_dict);
-                dict_unref (old_cache_dict);
+                cache->cache_dict = new_cache_dict;
         }
         UNLOCK (&cache->lock);
+
+        /* walk all entries and refcount-- with GF_REF_PUT() */
+        dict_foreach (old_cache_dict, auth_cache_entry_purge, NULL);
+        dict_unref (old_cache_dict);
 out:
         return;
 }
@@ -399,12 +475,13 @@ cache_nfs_fh (struct auth_cache *cache, struct nfs3_fh *fh,
         entry->item = export_item;
 
         ret = auth_cache_add (cache, hashkey, entry);
-        if (ret) {
-                GF_FREE (entry);
+        GF_REF_PUT (entry);
+        if (ret)
                 goto out;
-        }
+
         gf_msg_trace (GF_NFS, 0, "Caching file-handle (%s)", host_addr);
         ret = 0;
+
 out:
         GF_FREE (hashkey);