summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNiels de Vos <ndevos@redhat.com>2015-05-31 16:27:44 +0200
committerNiels de Vos <ndevos@redhat.com>2015-06-28 10:37:27 -0700
commit7b51bd636fc5e5e1ae48a4e7cba48d0d20878d15 (patch)
tree18dc62d891e1af39055627ba8637ca847130cb3d
parent67f7562b5cc9e42774d1dc569471f86f61eef040 (diff)
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>
-rw-r--r--xlators/nfs/server/src/auth-cache.c95
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);