When no key is supplied by the application, rte_thash_init_ctx()
generated the Toeplitz hash key with rte_rand().

That generator is not meant to be unpredictable and its internal state
can be recovered from a small number of outputs, so an attacker able to
observe other random values can derive the RSS key and craft flows that
all land on a single receive queue.

Use rte_random_bytes() for the hash key and for the LFSR state.

The hash key is generated once at context creation. It is now drawn
before the tailq lock is taken, since the system random generator can
block until the kernel pool is initialized and that should not be
done while holding a global lock.

The LFSR state is also set from alloc_lfsr(), which besides context
creation is reached from rte_thash_add_helper() and
rte_thash_adjust_tuple(). Those paths report a failure of the random
source as -ENOMEM, which is what they already return when alloc_lfsr()
fails.

No longer use rte_memcpy() just use memcpy().

Signed-off-by: Stephen Hemminger <[email protected]>
---
 lib/hash/rte_thash.c | 46 +++++++++++++++++++++++++++++++++++---------
 lib/hash/rte_thash.h |  4 +++-
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/lib/hash/rte_thash.c b/lib/hash/rte_thash.c
index fcd41248b9..7730ee053b 100644
--- a/lib/hash/rte_thash.c
+++ b/lib/hash/rte_thash.c
@@ -9,7 +9,6 @@
 #include <rte_thash.h>
 #include <rte_tailq.h>
 #include <rte_random.h>
-#include <rte_memcpy.h>
 #include <rte_errno.h>
 #include <rte_eal_memconfig.h>
 #include <rte_log.h>
@@ -176,7 +175,17 @@ alloc_lfsr(uint32_t poly_degree)
        lfsr->deg = poly_degree;
        lfsr->poly = thash_get_rand_poly(lfsr->deg);
        do {
-               lfsr->state = rte_rand() & ((1 << lfsr->deg) - 1);
+               uint32_t rnd;
+
+               if (rte_random_bytes(&rnd, sizeof(rnd)) != 0) {
+                       rte_free(lfsr);
+                       return NULL;
+               }
+
+               /* deg can be 32, so the mask must not be computed by
+                * shifting an int by the degree.
+                */
+               lfsr->state = rnd & RTE_GENMASK32(lfsr->deg - 1, 0);
        } while (lfsr->state == 0);
        /* init reverse order polynomial */
        lfsr->rev_poly = get_rev_poly(lfsr->poly, lfsr->deg);
@@ -215,13 +224,35 @@ rte_thash_init_ctx(const char *name, uint32_t key_len, 
uint32_t reta_sz,
        struct rte_thash_ctx *ctx;
        struct rte_tailq_entry *te;
        struct rte_thash_list *thash_list;
-       uint32_t i;
+       uint8_t *rand_key = NULL;
 
        if ((name == NULL) || (key_len == 0) || !RETA_SZ_IN_RANGE(reta_sz)) {
                rte_errno = EINVAL;
                return NULL;
        }
 
+       /* Draw the random key before taking the tailq lock, the system
+        * random generator can block until it is initialized.
+        */
+       if (key == NULL) {
+               rand_key = rte_zmalloc(NULL, key_len, 0);
+               if (rand_key == NULL) {
+                       HASH_LOG(ERR, "thash ctx %s key allocation failed",
+                               name);
+                       rte_errno = ENOMEM;
+                       return NULL;
+               }
+
+               if (rte_random_bytes(rand_key, key_len) != 0) {
+                       HASH_LOG(ERR,
+                               "Cannot generate hash key for thash context %s",
+                               name);
+                       rte_free(rand_key);
+                       rte_errno = EIO;
+                       return NULL;
+               }
+       }
+
        thash_list = RTE_TAILQ_CAST(rte_thash_tailq.head, rte_thash_list);
 
        rte_mcfg_tailq_write_lock();
@@ -262,12 +293,7 @@ rte_thash_init_ctx(const char *name, uint32_t key_len, 
uint32_t reta_sz,
        LIST_INIT(&ctx->head);
        ctx->flags = flags;
 
-       if (key)
-               rte_memcpy(ctx->hash_key, key, key_len);
-       else {
-               for (i = 0; i < key_len; i++)
-                       ctx->hash_key[i] = rte_rand();
-       }
+       memcpy(ctx->hash_key, key ? key : rand_key, key_len);
 
        if (rte_thash_gfni_supported()) {
                ctx->matrices = rte_zmalloc(NULL, key_len * sizeof(uint64_t),
@@ -286,6 +312,7 @@ rte_thash_init_ctx(const char *name, uint32_t key_len, 
uint32_t reta_sz,
        TAILQ_INSERT_TAIL(thash_list, te, next);
 
        rte_mcfg_tailq_write_unlock();
+       rte_free(rand_key);
 
        return ctx;
 
@@ -295,6 +322,7 @@ rte_thash_init_ctx(const char *name, uint32_t key_len, 
uint32_t reta_sz,
        rte_free(te);
 exit:
        rte_mcfg_tailq_write_unlock();
+       rte_free(rand_key);
        return NULL;
 }
 
diff --git a/lib/hash/rte_thash.h b/lib/hash/rte_thash.h
index 8bca430663..4387eb4a07 100644
--- a/lib/hash/rte_thash.h
+++ b/lib/hash/rte_thash.h
@@ -295,7 +295,9 @@ struct rte_thash_subtuple_helper;
  *  the reta entry.
  * @param key
  *  Pointer to the key used to init an internal key state.
- *  Could be NULL, in this case internal key will be inited with random.
+ *  Could be NULL, in this case the internal key is generated from the
+ *  random source of the operating system, and context creation fails
+ *  if that source is unavailable.
  * @param flags
  *  Supported flags are:
  *   RTE_THASH_IGNORE_PERIOD_OVERFLOW
-- 
2.53.0

Reply via email to