exports: Add a reference count to export_item_t structs and ensure they are correctly used

Summary:
This diff fixes a bug in the NFS daemon where the auth cache would use an export item after it was free'd by the
auth params refresh thread. This usually manifests as a crash in production, when exports files are updated by chef.

Since each auth cache entry holds a pointer to an export_item_t it makes sense that it should first get a reference to it.

Freein'g the export_item_t struct happens only in `exp_item_unref()`, once the reference count has dropped to 0.

This diff also fixes a use-after-free bug in the auth-cache, in the insertion path.

In _cache_item(), if we find an entry in the dict, we update that entry with a timestamp & ref the export item associated with it.
However, if the item already existed and we called old_cache_insert() with the same key, we gave the dict permission to free the old entry.
We then end up using that entry.

The fix is to use dict_set_static_bin() instead of dict_set_bin() which informs the dict that the pointer we are giving it belongs to us.

This is a port of D5780476, D5785038 to 3.8

Change-Id: I5cdcdc1cc6abad26c7077d66a14f263da07678ac
Reviewed-on: https://review.gluster.org/18248
Reviewed-by: Shreyas Siravara <sshreyas@fb.com>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Smoke: Gluster Build System <jenkins@build.gluster.org>

3 files changed, 77 insertions, 10 deletions

diff --git a/xlators/nfs/server/src/auth-cache.c b/xlators/nfs/server/src/auth-cache.c
index 428821a5506..9524bdfd48d 100644
--- a/xlators/nfs/server/src/auth-cache.c
+++ b/xlators/nfs/server/src/auth-cache.c
@@ -98,8 +98,12 @@ static int32_t
 old_cache_insert (struct auth_cache *cache, char *hashkey,
                   struct auth_cache_entry *entry)
 {
-        return dict_set_bin (cache->cache_dict, hashkey,
-                             entry, sizeof (*entry));
+        /* We must use dict_set_static_bin() to ensure that
+         * the ownership of "entry" remains with us, and not
+         * the dict library.
+         */
+        return dict_set_static_bin (cache->cache_dict, hashkey,
+                                    entry, sizeof (*entry));
 }
 
 static void
@@ -181,8 +185,14 @@ auth_cache_entry_init ()
 void
 auth_cache_purge (struct auth_cache *cache)
 {
+        if (!cache) {
+                return;
+        }
+
         pthread_mutex_lock (&cache->lock);
-        (methods->reset) (cache);
+        {
+                (methods->reset) (cache);
+        }
         pthread_mutex_unlock (&cache->lock);
 }
 
@@ -219,7 +229,11 @@ _cache_lookup (struct auth_cache *cache, char *key,
         }
 
         if (time (NULL) - lookup_res->timestamp > cache->ttl_sec) {
+                // Destroy the auth cache entry
+                exp_item_unref (lookup_res->item);
+                lookup_res->item = NULL;
                 GF_FREE (lookup_res);
+
                 entry_data->data = NULL;
                 dict_del (cache->cache_dict, key);  // Remove from the cache
                 ret = ENTRY_EXPIRED;
@@ -314,9 +328,16 @@ __cache_item (struct auth_cache *cache, const char *path, struct nfs3_fh *fh,
                 GF_CHECK_ALLOC (entry, ret, out);
         }
 
-        // Populate the entry
+        // Update the timestamp
         entry->timestamp = time (NULL);
-        entry->item = export_item;
+
+        // Update entry->item if it is NULL or
+        // pointing to a different export_item
+        if (entry->item != export_item) {
+                exp_item_unref (entry->item);
+                entry->item = exp_item_ref (export_item);
+        }
+
         // Access is only allowed if  the status is set to
         // AUTH_CACHE_HOST_AUTH_OK
         entry->access_allowed = (status == AUTH_CACHE_HOST_AUTH_OK);
diff --git a/xlators/nfs/server/src/exports.c b/xlators/nfs/server/src/exports.c
index 83aec254040..b21b4fbeb70 100644
--- a/xlators/nfs/server/src/exports.c
+++ b/xlators/nfs/server/src/exports.c
@@ -240,9 +240,10 @@ _export_item_init ()
         struct export_item *item = GF_CALLOC (1, sizeof (*item),
                                               gf_common_mt_nfs_exports);
 
-        if (!item)
-                gf_msg (GF_EXP, GF_LOG_CRITICAL, ENOMEM, NFS_MSG_NO_MEMORY,
-                        "Failed to allocate export item!");
+        if (item) {
+                pthread_mutex_init (&item->lock, NULL);
+                exp_item_ref (item);
+        }
 
         return item;
 }
@@ -260,6 +261,7 @@ _export_item_deinit (struct export_item *item)
         if (!item)
                 return;
 
+        pthread_mutex_destroy (&item->lock);
         _export_options_deinit (item->opts);
         GF_FREE (item->name);
         GF_FREE (item);
@@ -355,7 +357,7 @@ static int
 __exp_dict_free_walk (dict_t *dict, char *key, data_t *val, void *tmp)
 {
         if (val) {
-                _export_item_deinit ((struct export_item *)val->data);
+                exp_item_unref ((export_item_t *)val->data);
                 val->data = NULL;
                 dict_del (dict, key);
         }
@@ -381,6 +383,42 @@ out:
         return;
 }
 
+export_item_t *
+exp_item_ref (export_item_t *item)
+{
+        if (!item) {
+                return NULL;
+        }
+
+        pthread_mutex_lock (&item->lock);
+        {
+                item->refcount++;
+        }
+        pthread_mutex_unlock (&item->lock);
+        return item;
+}
+
+void
+exp_item_unref (export_item_t *item)
+{
+        uint32_t refcount = 0;
+
+        if (!item) {
+                return;
+        }
+
+        pthread_mutex_lock (&item->lock);
+        {
+                item->refcount--;
+                refcount = item->refcount;
+        }
+        pthread_mutex_unlock (&item->lock);
+
+        if (refcount == 0) {
+                _export_item_deinit (item);
+        }
+}
+
 /**
  * exp_file_dir_from_uuid -- Using a uuid as the key, retrieve an exports
  *                           directory from the file.
@@ -768,7 +806,7 @@ __exp_line_ng_host_str_parse (char *str, struct export_item **exp_item)
         ret = __exp_line_opt_parse (optstr, &exp_opts);
         if (ret != 0) {
                 /* Bubble up the error to the caller */
-                _export_item_deinit (item);
+                exp_item_unref (item);
                 goto out;
         }
 
diff --git a/xlators/nfs/server/src/exports.h b/xlators/nfs/server/src/exports.h
index a4e15d3f7ef..4421253469d 100644
--- a/xlators/nfs/server/src/exports.h
+++ b/xlators/nfs/server/src/exports.h
@@ -57,6 +57,8 @@ typedef struct export_options export_options_t;
 struct export_item {
         char             *name;  /* Name of the export item */
         export_options_t *opts;  /* NFS Options */
+        uint32_t refcount;
+        pthread_mutex_t lock;
 };
 typedef struct export_item export_item_t;
 
@@ -94,4 +96,10 @@ struct export_dir *
 exp_file_dir_from_uuid (const struct exports_file *file,
                         const uuid_t export_uuid);
 
+export_item_t *
+exp_item_ref (export_item_t *item);
+
+void
+exp_item_unref (export_item_t *item);
+
 #endif  /* _EXPORTS_H_ */