hash: make tailq fully local

Since the data structures such as rings are shared in their entirety,
those TAILQ pointers are shared as well. Meaning that, after a
successful rte_ring creation, the tailq_next pointer of the last
ring in the TAILQ will be updated with a pointer to a ring which may
not be present in the address space of another process (i.e. a ring
that may be host-local or guest-local, and not shared over IVSHMEM).
Any successive ring create/lookup on the other side of IVSHMEM will
result in trying to dereference an invalid pointer.

This patchset fixes this problem by creating a default tailq entry
that may be used by any data structure that chooses to use TAILQs.
This default TAILQ entry will consist of a tailq_next/tailq_prev
pointers, and an opaque pointer to arbitrary data. All TAILQ
pointers from data structures themselves will be removed and
replaced by those generic TAILQ entries, thus fixing the problem
of potentially exposing local address space to shared structures.

Technically, only rte_ring structure require modification, because
IVSHMEM is only using memzones (which aren't in TAILQs) and rings,
but for consistency's sake other TAILQ-based data structures were
adapted as well.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

diff --git a/lib/librte_hash/rte_fbk_hash.c b/lib/librte_hash/rte_fbk_hash.c
index d1caabc..421e1cd 100644 (file)
--- a/lib/librte_hash/rte_fbk_hash.c
+++ b/lib/librte_hash/rte_fbk_hash.c
@@ -54,7 +54,7 @@
 
 #include "rte_fbk_hash.h"
 
-TAILQ_HEAD(rte_fbk_hash_list, rte_fbk_hash_table);
+TAILQ_HEAD(rte_fbk_hash_list, rte_tailq_entry);
 
 /**
  * Performs a lookup for an existing hash table, and returns a pointer to
@@ -69,24 +69,29 @@ TAILQ_HEAD(rte_fbk_hash_list, rte_fbk_hash_table);
 struct rte_fbk_hash_table *
 rte_fbk_hash_find_existing(const char *name)
 {
-       struct rte_fbk_hash_table *h;
+       struct rte_fbk_hash_table *h = NULL;
+       struct rte_tailq_entry *te;
        struct rte_fbk_hash_list *fbk_hash_list;
 
        /* check that we have an initialised tail queue */
        if ((fbk_hash_list =
-            RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_FBK_HASH, rte_fbk_hash_list)) == NULL) {
+                       RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_FBK_HASH,
+                                       rte_fbk_hash_list)) == NULL) {
                rte_errno = E_RTE_NO_TAILQ;
                return NULL;
        }
 
        rte_rwlock_read_lock(RTE_EAL_TAILQ_RWLOCK);
-       TAILQ_FOREACH(h, fbk_hash_list, next) {
+       TAILQ_FOREACH(te, fbk_hash_list, next) {
+               h = (struct rte_fbk_hash_table *) te->data;
                if (strncmp(name, h->name, RTE_FBK_HASH_NAMESIZE) == 0)
                        break;
        }
        rte_rwlock_read_unlock(RTE_EAL_TAILQ_RWLOCK);
-       if (h == NULL)
+       if (te == NULL) {
                rte_errno = ENOENT;
+               return NULL;
+       }
        return h;
 }
 
@@ -104,6 +109,7 @@ struct rte_fbk_hash_table *
 rte_fbk_hash_create(const struct rte_fbk_hash_params *params)
 {
        struct rte_fbk_hash_table *ht = NULL;
+       struct rte_tailq_entry *te;
        char hash_name[RTE_FBK_HASH_NAMESIZE];
        const uint32_t mem_size =
                        sizeof(*ht) + (sizeof(ht->t[0]) * params->entries);
@@ -112,7 +118,8 @@ rte_fbk_hash_create(const struct rte_fbk_hash_params *params)
 
        /* check that we have an initialised tail queue */
        if ((fbk_hash_list =
-            RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_FBK_HASH, rte_fbk_hash_list)) == NULL) {
+                       RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_FBK_HASH,
+                                       rte_fbk_hash_list)) == NULL) {
                rte_errno = E_RTE_NO_TAILQ;
                return NULL;
        }
@@ -134,20 +141,28 @@ rte_fbk_hash_create(const struct rte_fbk_hash_params *params)
        rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK);
 
        /* guarantee there's no existing */
-       TAILQ_FOREACH(ht, fbk_hash_list, next) {
+       TAILQ_FOREACH(te, fbk_hash_list, next) {
+               ht = (struct rte_fbk_hash_table *) te->data;
                if (strncmp(params->name, ht->name, RTE_FBK_HASH_NAMESIZE) == 0)
                        break;
        }
-       if (ht != NULL)
+       if (te != NULL)
                goto exit;
 
+       te = rte_zmalloc("FBK_HASH_TAILQ_ENTRY", sizeof(*te), 0);
+       if (te == NULL) {
+               RTE_LOG(ERR, HASH, "Failed to allocate tailq entry\n");
+               goto exit;
+       }
+
        /* Allocate memory for table. */
-       ht = (struct rte_fbk_hash_table *)rte_malloc_socket(hash_name, mem_size,
+       ht = (struct rte_fbk_hash_table *)rte_zmalloc_socket(hash_name, mem_size,
                        0, params->socket_id);
-       if (ht == NULL)
+       if (ht == NULL) {
+               RTE_LOG(ERR, HASH, "Failed to allocate fbk hash table\n");
+               rte_free(te);
                goto exit;
-
-       memset(ht, 0, mem_size);
+       }
 
        /* Set up hash table context. */
        snprintf(ht->name, sizeof(ht->name), "%s", params->name);
@@ -169,7 +184,9 @@ rte_fbk_hash_create(const struct rte_fbk_hash_params *params)
                ht->init_val = RTE_FBK_HASH_INIT_VAL_DEFAULT;
        }
 
-       TAILQ_INSERT_TAIL(fbk_hash_list, ht, next);
+       te->data = (void *) ht;
+
+       TAILQ_INSERT_TAIL(fbk_hash_list, te, next);
 
 exit:
        rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
@@ -186,10 +203,38 @@ exit:
 void
 rte_fbk_hash_free(struct rte_fbk_hash_table *ht)
 {
+       struct rte_tailq_entry *te;
+       struct rte_fbk_hash_list *fbk_hash_list;
+
        if (ht == NULL)
                return;
 
-       RTE_EAL_TAILQ_REMOVE(RTE_TAILQ_FBK_HASH, rte_fbk_hash_list, ht);
+       /* check that we have an initialised tail queue */
+       if ((fbk_hash_list =
+                       RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_FBK_HASH,
+                                       rte_fbk_hash_list)) == NULL) {
+               rte_errno = E_RTE_NO_TAILQ;
+               return;
+       }
+
+       rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK);
+
+       /* find out tailq entry */
+       TAILQ_FOREACH(te, fbk_hash_list, next) {
+               if (te->data == (void *) ht)
+                       break;
+       }
+
+       if (te == NULL) {
+               rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
+               return;
+       }
+
+       TAILQ_REMOVE(fbk_hash_list, te, next);
+
+       rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
+
        rte_free(ht);
+       rte_free(te);
 }
 

diff --git a/lib/librte_hash/rte_fbk_hash.h b/lib/librte_hash/rte_fbk_hash.h
index 4d1a316..3d229bf 100644 (file)
--- a/lib/librte_hash/rte_fbk_hash.h
+++ b/lib/librte_hash/rte_fbk_hash.h
@@ -103,11 +103,8 @@ union rte_fbk_hash_entry {
 };
 
 
-
 /** The four-byte key hash table structure. */
 struct rte_fbk_hash_table {
-       TAILQ_ENTRY(rte_fbk_hash_table) next;   /**< Linked list. */
-
        char name[RTE_FBK_HASH_NAMESIZE];       /**< Name of the hash. */
        uint32_t entries;               /**< Total number of entries. */
        uint32_t entries_per_bucket;    /**< Number of entries in a bucket. */

diff --git a/lib/librte_hash/rte_hash.c b/lib/librte_hash/rte_hash.c
index b7d882b..d02b6b4 100644 (file)
--- a/lib/librte_hash/rte_hash.c
+++ b/lib/librte_hash/rte_hash.c
@@ -60,7 +60,7 @@
 #include "rte_hash.h"
 
 
-TAILQ_HEAD(rte_hash_list, rte_hash);
+TAILQ_HEAD(rte_hash_list, rte_tailq_entry);
 
 /* Macro to enable/disable run-time checking of function parameters */
 #if defined(RTE_LIBRTE_HASH_DEBUG)
@@ -141,24 +141,29 @@ find_first(uint32_t sig, const uint32_t *sig_bucket, uint32_t num_sigs)
 struct rte_hash *
 rte_hash_find_existing(const char *name)
 {
-       struct rte_hash *h;
+       struct rte_hash *h = NULL;
+       struct rte_tailq_entry *te;
        struct rte_hash_list *hash_list;
 
        /* check that we have an initialised tail queue */
-       if ((hash_list = RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_HASH, rte_hash_list)) == NULL) {
+       if ((hash_list =
+                       RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_HASH, rte_hash_list)) == NULL) {
                rte_errno = E_RTE_NO_TAILQ;
                return NULL;
        }
 
        rte_rwlock_read_lock(RTE_EAL_TAILQ_RWLOCK);
-       TAILQ_FOREACH(h, hash_list, next) {
+       TAILQ_FOREACH(te, hash_list, next) {
+               h = (struct rte_hash *) te->data;
                if (strncmp(name, h->name, RTE_HASH_NAMESIZE) == 0)
                        break;
        }
        rte_rwlock_read_unlock(RTE_EAL_TAILQ_RWLOCK);
 
-       if (h == NULL)
+       if (te == NULL) {
                rte_errno = ENOENT;
+               return NULL;
+       }
        return h;
 }
 
@@ -166,6 +171,7 @@ struct rte_hash *
 rte_hash_create(const struct rte_hash_parameters *params)
 {
        struct rte_hash *h = NULL;
+       struct rte_tailq_entry *te;
        uint32_t num_buckets, sig_bucket_size, key_size,
                hash_tbl_size, sig_tbl_size, key_tbl_size, mem_size;
        char hash_name[RTE_HASH_NAMESIZE];
@@ -212,17 +218,25 @@ rte_hash_create(const struct rte_hash_parameters *params)
        rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK);
 
        /* guarantee there's no existing */
-       TAILQ_FOREACH(h, hash_list, next) {
+       TAILQ_FOREACH(te, hash_list, next) {
+               h = (struct rte_hash *) te->data;
                if (strncmp(params->name, h->name, RTE_HASH_NAMESIZE) == 0)
                        break;
        }
-       if (h != NULL)
+       if (te != NULL)
+               goto exit;
+
+       te = rte_zmalloc("HASH_TAILQ_ENTRY", sizeof(*te), 0);
+       if (te == NULL) {
+               RTE_LOG(ERR, HASH, "tailq entry allocation failed\n");
                goto exit;
+       }
 
        h = (struct rte_hash *)rte_zmalloc_socket(hash_name, mem_size,
                                           CACHE_LINE_SIZE, params->socket_id);
        if (h == NULL) {
                RTE_LOG(ERR, HASH, "memory allocation failed\n");
+               rte_free(te);
                goto exit;
        }
 
@@ -242,7 +256,9 @@ rte_hash_create(const struct rte_hash_parameters *params)
        h->hash_func = (params->hash_func == NULL) ?
                DEFAULT_HASH_FUNC : params->hash_func;
 
-       TAILQ_INSERT_TAIL(hash_list, h, next);
+       te->data = (void *) h;
+
+       TAILQ_INSERT_TAIL(hash_list, te, next);
 
 exit:
        rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
@@ -253,11 +269,38 @@ exit:
 void
 rte_hash_free(struct rte_hash *h)
 {
+       struct rte_tailq_entry *te;
+       struct rte_hash_list *hash_list;
+
        if (h == NULL)
                return;
 
-       RTE_EAL_TAILQ_REMOVE(RTE_TAILQ_HASH, rte_hash_list, h);
+       /* check that we have an initialised tail queue */
+       if ((hash_list =
+            RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_HASH, rte_hash_list)) == NULL) {
+               rte_errno = E_RTE_NO_TAILQ;
+               return;
+       }
+
+       rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK);
+
+       /* find out tailq entry */
+       TAILQ_FOREACH(te, hash_list, next) {
+               if (te->data == (void *) h)
+                       break;
+       }
+
+       if (te == NULL) {
+               rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
+               return;
+       }
+
+       TAILQ_REMOVE(hash_list, te, next);
+
+       rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
+
        rte_free(h);
+       rte_free(te);
 }
 
 static inline int32_t

diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h
index 5228e3a..2ecaf1a 100644 (file)
--- a/lib/librte_hash/rte_hash.h
+++ b/lib/librte_hash/rte_hash.h
@@ -86,8 +86,6 @@ struct rte_hash_parameters {
 
 /** A hash table structure. */
 struct rte_hash {
-       TAILQ_ENTRY(rte_hash) next;/**< Next in list. */
-
        char name[RTE_HASH_NAMESIZE];   /**< Name of the hash. */
        uint32_t entries;               /**< Total table entries. */
        uint32_t bucket_entries;        /**< Bucket entries. */