* [PATCH 0/2] ip_frag: TAILQ and hash fixes @ 2026-04-08 16:16 Stephen Hemminger 2026-04-08 16:16 ` [PATCH 1/2] ip_frag: fix unsafe TAILQ usage Stephen Hemminger 2026-04-08 16:16 ` [PATCH 2/2] ip_frag: randomize hash seed Stephen Hemminger 0 siblings, 2 replies; 5+ messages in thread From: Stephen Hemminger @ 2026-04-08 16:16 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger The ip_frag library had a couple of issues found during review. The use of TAILQ made assumptions about TAILQ_REMOVE, and the hash function usage could be abused by attacker. Stephen Hemminger (2): ip_frag: fix unsafe TAILQ usage ip_frag: randomize hash seed lib/ip_frag/ip_frag_common.h | 1 + lib/ip_frag/ip_frag_internal.c | 18 ++++++++---------- lib/ip_frag/ip_reassembly.h | 1 + lib/ip_frag/rte_ip_frag_common.c | 21 +++++++++++---------- 4 files changed, 21 insertions(+), 20 deletions(-) -- 2.53.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] ip_frag: fix unsafe TAILQ usage 2026-04-08 16:16 [PATCH 0/2] ip_frag: TAILQ and hash fixes Stephen Hemminger @ 2026-04-08 16:16 ` Stephen Hemminger 2026-04-13 12:24 ` Konstantin Ananyev 2026-04-08 16:16 ` [PATCH 2/2] ip_frag: randomize hash seed Stephen Hemminger 1 sibling, 1 reply; 5+ messages in thread From: Stephen Hemminger @ 2026-04-08 16:16 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, stable, Konstantin Ananyev, Allain Legacy The frag table next pointer was being access after TAILQ_REMOVE(). This is not safe since it depends on TAILQ_REMOVE() not changing next pointer. Fix by using RTE_TAILQ_FOREACH_SAFE(). Fixes: 95908f52393d ("ip_frag: free mbufs on reassembly table destroy") Cc: stable@dpdk.org Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- lib/ip_frag/ip_frag_common.h | 1 + lib/ip_frag/rte_ip_frag_common.c | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/ip_frag/ip_frag_common.h b/lib/ip_frag/ip_frag_common.h index 51fc9d47fb..34be5bb6ab 100644 --- a/lib/ip_frag/ip_frag_common.h +++ b/lib/ip_frag/ip_frag_common.h @@ -8,6 +8,7 @@ #include <sys/queue.h> #include <rte_common.h> +#include <rte_tailq.h> #if defined(RTE_ARCH_ARM64) #include <rte_cmp_arm64.h> diff --git a/lib/ip_frag/rte_ip_frag_common.c b/lib/ip_frag/rte_ip_frag_common.c index ee9aa93027..79ac45289b 100644 --- a/lib/ip_frag/rte_ip_frag_common.c +++ b/lib/ip_frag/rte_ip_frag_common.c @@ -135,18 +135,18 @@ rte_ip_frag_table_del_expired_entries(struct rte_ip_frag_tbl *tbl, struct rte_ip_frag_death_row *dr, uint64_t tms) { uint64_t max_cycles; - struct ip_frag_pkt *fp; + struct ip_frag_pkt *fp, *tmp; max_cycles = tbl->max_cycles; - TAILQ_FOREACH(fp, &tbl->lru, lru) - if (max_cycles + fp->start < tms) { - /* check that death row has enough space */ - if (RTE_IP_FRAG_DEATH_ROW_MBUF_LEN - dr->cnt >= - fp->last_idx) - ip_frag_tbl_del(tbl, dr, fp); - else - return; - } else + RTE_TAILQ_FOREACH_SAFE(fp, &tbl->lru, lru, tmp) { + if (max_cycles + fp->start >= tms) + return; + + /* check that death row has enough space */ + if (RTE_IP_FRAG_DEATH_ROW_MBUF_LEN - dr->cnt < fp->last_idx) return; + + ip_frag_tbl_del(tbl, dr, fp); + } } -- 2.53.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH 1/2] ip_frag: fix unsafe TAILQ usage 2026-04-08 16:16 ` [PATCH 1/2] ip_frag: fix unsafe TAILQ usage Stephen Hemminger @ 2026-04-13 12:24 ` Konstantin Ananyev 0 siblings, 0 replies; 5+ messages in thread From: Konstantin Ananyev @ 2026-04-13 12:24 UTC (permalink / raw) To: Stephen Hemminger, dev@dpdk.org; +Cc: stable@dpdk.org, Allain Legacy > > The frag table next pointer was being access after TAILQ_REMOVE(). > This is not safe since it depends on TAILQ_REMOVE() not changing > next pointer. Fix by using RTE_TAILQ_FOREACH_SAFE(). > > Fixes: 95908f52393d ("ip_frag: free mbufs on reassembly table destroy") > Cc: stable@dpdk.org > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > --- > lib/ip_frag/ip_frag_common.h | 1 + > lib/ip_frag/rte_ip_frag_common.c | 20 ++++++++++---------- > 2 files changed, 11 insertions(+), 10 deletions(-) > > diff --git a/lib/ip_frag/ip_frag_common.h b/lib/ip_frag/ip_frag_common.h > index 51fc9d47fb..34be5bb6ab 100644 > --- a/lib/ip_frag/ip_frag_common.h > +++ b/lib/ip_frag/ip_frag_common.h > @@ -8,6 +8,7 @@ > #include <sys/queue.h> > > #include <rte_common.h> > +#include <rte_tailq.h> > > #if defined(RTE_ARCH_ARM64) > #include <rte_cmp_arm64.h> > diff --git a/lib/ip_frag/rte_ip_frag_common.c > b/lib/ip_frag/rte_ip_frag_common.c > index ee9aa93027..79ac45289b 100644 > --- a/lib/ip_frag/rte_ip_frag_common.c > +++ b/lib/ip_frag/rte_ip_frag_common.c > @@ -135,18 +135,18 @@ rte_ip_frag_table_del_expired_entries(struct > rte_ip_frag_tbl *tbl, > struct rte_ip_frag_death_row *dr, uint64_t tms) > { > uint64_t max_cycles; > - struct ip_frag_pkt *fp; > + struct ip_frag_pkt *fp, *tmp; > > max_cycles = tbl->max_cycles; > > - TAILQ_FOREACH(fp, &tbl->lru, lru) > - if (max_cycles + fp->start < tms) { > - /* check that death row has enough space */ > - if (RTE_IP_FRAG_DEATH_ROW_MBUF_LEN - dr->cnt >= > - fp->last_idx) > - ip_frag_tbl_del(tbl, dr, fp); > - else > - return; > - } else > + RTE_TAILQ_FOREACH_SAFE(fp, &tbl->lru, lru, tmp) { > + if (max_cycles + fp->start >= tms) > + return; > + > + /* check that death row has enough space */ > + if (RTE_IP_FRAG_DEATH_ROW_MBUF_LEN - dr->cnt < fp- > >last_idx) > return; > + > + ip_frag_tbl_del(tbl, dr, fp); > + } > } > -- Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com> > 2.53.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] ip_frag: randomize hash seed 2026-04-08 16:16 [PATCH 0/2] ip_frag: TAILQ and hash fixes Stephen Hemminger 2026-04-08 16:16 ` [PATCH 1/2] ip_frag: fix unsafe TAILQ usage Stephen Hemminger @ 2026-04-08 16:16 ` Stephen Hemminger 2026-04-13 12:24 ` Konstantin Ananyev 1 sibling, 1 reply; 5+ messages in thread From: Stephen Hemminger @ 2026-04-08 16:16 UTC (permalink / raw) To: dev Cc: Stephen Hemminger, stable, Konstantin Ananyev, Anatoly Burakov, Thomas Monjalon Both ipv4_frag_hash() and ipv6_frag_hash() use CRC32 (x86/ARM) or jhash with a fixed, publicly known prime seed (0xeaad8405). An attacker who can send crafted IP fragments can precompute hash collisions, causing all fragments to land in the same bucket. After bucket_entries concurrent flows collide, new flows are dropped. Fix by using a random hash seed added at table creation time. Fixes: 416707812c03 ("ip_frag: refactor reassembly code into a proper library") Cc: stable@dpdk.org Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- lib/ip_frag/ip_frag_internal.c | 18 ++++++++---------- lib/ip_frag/ip_reassembly.h | 1 + lib/ip_frag/rte_ip_frag_common.c | 1 + 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/ip_frag/ip_frag_internal.c b/lib/ip_frag/ip_frag_internal.c index 7cbef647df..382f42d0e1 100644 --- a/lib/ip_frag/ip_frag_internal.c +++ b/lib/ip_frag/ip_frag_internal.c @@ -9,8 +9,6 @@ #include "ip_frag_common.h" -#define PRIME_VALUE 0xeaad8405 - #define IP_FRAG_TBL_POS(tbl, sig) \ ((tbl)->pkt + ((sig) & (tbl)->entry_mask)) @@ -38,7 +36,7 @@ ip_frag_tbl_reuse(struct rte_ip_frag_tbl *tbl, struct rte_ip_frag_death_row *dr, static inline void -ipv4_frag_hash(const struct ip_frag_key *key, uint32_t *v1, uint32_t *v2) +ipv4_frag_hash(const struct ip_frag_key *key, uint32_t *v1, uint32_t *v2, uint32_t seed) { uint32_t v; const uint32_t *p; @@ -46,12 +44,12 @@ ipv4_frag_hash(const struct ip_frag_key *key, uint32_t *v1, uint32_t *v2) p = (const uint32_t *)&key->src_dst; #if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64) - v = rte_hash_crc_4byte(p[0], PRIME_VALUE); + v = rte_hash_crc_4byte(p[0], seed); v = rte_hash_crc_4byte(p[1], v); v = rte_hash_crc_4byte(key->id, v); #else - v = rte_jhash_3words(p[0], p[1], key->id, PRIME_VALUE); + v = rte_jhash_3words(p[0], p[1], key->id, seed); #endif /* RTE_ARCH_X86 */ *v1 = v; @@ -59,7 +57,7 @@ ipv4_frag_hash(const struct ip_frag_key *key, uint32_t *v1, uint32_t *v2) } static inline void -ipv6_frag_hash(const struct ip_frag_key *key, uint32_t *v1, uint32_t *v2) +ipv6_frag_hash(const struct ip_frag_key *key, uint32_t *v1, uint32_t *v2, uint32_t seed) { uint32_t v; const uint32_t *p; @@ -67,7 +65,7 @@ ipv6_frag_hash(const struct ip_frag_key *key, uint32_t *v1, uint32_t *v2) p = (const uint32_t *) &key->src_dst; #if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64) - v = rte_hash_crc_4byte(p[0], PRIME_VALUE); + v = rte_hash_crc_4byte(p[0], seed); v = rte_hash_crc_4byte(p[1], v); v = rte_hash_crc_4byte(p[2], v); v = rte_hash_crc_4byte(p[3], v); @@ -78,7 +76,7 @@ ipv6_frag_hash(const struct ip_frag_key *key, uint32_t *v1, uint32_t *v2) v = rte_hash_crc_4byte(key->id, v); #else - v = rte_jhash_3words(p[0], p[1], p[2], PRIME_VALUE); + v = rte_jhash_3words(p[0], p[1], p[2], seed); v = rte_jhash_3words(p[3], p[4], p[5], v); v = rte_jhash_3words(p[6], p[7], key->id, v); #endif /* RTE_ARCH_X86 */ @@ -301,9 +299,9 @@ ip_frag_lookup(struct rte_ip_frag_tbl *tbl, /* different hashing methods for IPv4 and IPv6 */ if (key->key_len == IPV4_KEYLEN) - ipv4_frag_hash(key, &sig1, &sig2); + ipv4_frag_hash(key, &sig1, &sig2, tbl->seed); else - ipv6_frag_hash(key, &sig1, &sig2); + ipv6_frag_hash(key, &sig1, &sig2, tbl->seed); p1 = IP_FRAG_TBL_POS(tbl, sig1); p2 = IP_FRAG_TBL_POS(tbl, sig2); diff --git a/lib/ip_frag/ip_reassembly.h b/lib/ip_frag/ip_reassembly.h index 54afed5417..9e1666ef67 100644 --- a/lib/ip_frag/ip_reassembly.h +++ b/lib/ip_frag/ip_reassembly.h @@ -79,6 +79,7 @@ struct rte_ip_frag_tbl { uint32_t bucket_entries; /* hash associativity. */ uint32_t nb_entries; /* total size of the table. */ uint32_t nb_buckets; /* num of associativity lines. */ + uint32_t seed; /* hash function init value */ struct ip_frag_pkt *last; /* last used entry. */ struct ip_pkt_list lru; /* LRU list for table entries. */ struct ip_frag_tbl_stat stat; /* statistics counters. */ diff --git a/lib/ip_frag/rte_ip_frag_common.c b/lib/ip_frag/rte_ip_frag_common.c index 79ac45289b..00bf9476de 100644 --- a/lib/ip_frag/rte_ip_frag_common.c +++ b/lib/ip_frag/rte_ip_frag_common.c @@ -79,6 +79,7 @@ rte_ip_frag_table_create(uint32_t bucket_num, uint32_t bucket_entries, tbl->nb_buckets = bucket_num; tbl->bucket_entries = bucket_entries; tbl->entry_mask = (tbl->nb_entries - 1) & ~(tbl->bucket_entries - 1); + tbl->seed = rte_rand(); TAILQ_INIT(&(tbl->lru)); return tbl; -- 2.53.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH 2/2] ip_frag: randomize hash seed 2026-04-08 16:16 ` [PATCH 2/2] ip_frag: randomize hash seed Stephen Hemminger @ 2026-04-13 12:24 ` Konstantin Ananyev 0 siblings, 0 replies; 5+ messages in thread From: Konstantin Ananyev @ 2026-04-13 12:24 UTC (permalink / raw) To: Stephen Hemminger, dev@dpdk.org Cc: stable@dpdk.org, Anatoly Burakov, Thomas Monjalon > Both ipv4_frag_hash() and ipv6_frag_hash() use CRC32 (x86/ARM) > or jhash with a fixed, publicly known prime seed (0xeaad8405). > An attacker who can send crafted IP fragments can precompute hash > collisions, causing all fragments to land in the same bucket. > After bucket_entries concurrent flows collide, new flows are dropped. > > Fix by using a random hash seed added at table creation time. > > Fixes: 416707812c03 ("ip_frag: refactor reassembly code into a proper library") > Cc: stable@dpdk.org > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > --- > lib/ip_frag/ip_frag_internal.c | 18 ++++++++---------- > lib/ip_frag/ip_reassembly.h | 1 + > lib/ip_frag/rte_ip_frag_common.c | 1 + > 3 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/lib/ip_frag/ip_frag_internal.c b/lib/ip_frag/ip_frag_internal.c > index 7cbef647df..382f42d0e1 100644 > --- a/lib/ip_frag/ip_frag_internal.c > +++ b/lib/ip_frag/ip_frag_internal.c > @@ -9,8 +9,6 @@ > > #include "ip_frag_common.h" > > -#define PRIME_VALUE 0xeaad8405 > - > #define IP_FRAG_TBL_POS(tbl, sig) \ > ((tbl)->pkt + ((sig) & (tbl)->entry_mask)) > > @@ -38,7 +36,7 @@ ip_frag_tbl_reuse(struct rte_ip_frag_tbl *tbl, struct > rte_ip_frag_death_row *dr, > > > static inline void > -ipv4_frag_hash(const struct ip_frag_key *key, uint32_t *v1, uint32_t *v2) > +ipv4_frag_hash(const struct ip_frag_key *key, uint32_t *v1, uint32_t *v2, > uint32_t seed) > { > uint32_t v; > const uint32_t *p; > @@ -46,12 +44,12 @@ ipv4_frag_hash(const struct ip_frag_key *key, uint32_t > *v1, uint32_t *v2) > p = (const uint32_t *)&key->src_dst; > > #if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64) > - v = rte_hash_crc_4byte(p[0], PRIME_VALUE); > + v = rte_hash_crc_4byte(p[0], seed); > v = rte_hash_crc_4byte(p[1], v); > v = rte_hash_crc_4byte(key->id, v); > #else > > - v = rte_jhash_3words(p[0], p[1], key->id, PRIME_VALUE); > + v = rte_jhash_3words(p[0], p[1], key->id, seed); > #endif /* RTE_ARCH_X86 */ > > *v1 = v; > @@ -59,7 +57,7 @@ ipv4_frag_hash(const struct ip_frag_key *key, uint32_t *v1, > uint32_t *v2) > } > > static inline void > -ipv6_frag_hash(const struct ip_frag_key *key, uint32_t *v1, uint32_t *v2) > +ipv6_frag_hash(const struct ip_frag_key *key, uint32_t *v1, uint32_t *v2, > uint32_t seed) > { > uint32_t v; > const uint32_t *p; > @@ -67,7 +65,7 @@ ipv6_frag_hash(const struct ip_frag_key *key, uint32_t *v1, > uint32_t *v2) > p = (const uint32_t *) &key->src_dst; > > #if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64) > - v = rte_hash_crc_4byte(p[0], PRIME_VALUE); > + v = rte_hash_crc_4byte(p[0], seed); > v = rte_hash_crc_4byte(p[1], v); > v = rte_hash_crc_4byte(p[2], v); > v = rte_hash_crc_4byte(p[3], v); > @@ -78,7 +76,7 @@ ipv6_frag_hash(const struct ip_frag_key *key, uint32_t *v1, > uint32_t *v2) > v = rte_hash_crc_4byte(key->id, v); > #else > > - v = rte_jhash_3words(p[0], p[1], p[2], PRIME_VALUE); > + v = rte_jhash_3words(p[0], p[1], p[2], seed); > v = rte_jhash_3words(p[3], p[4], p[5], v); > v = rte_jhash_3words(p[6], p[7], key->id, v); > #endif /* RTE_ARCH_X86 */ > @@ -301,9 +299,9 @@ ip_frag_lookup(struct rte_ip_frag_tbl *tbl, > > /* different hashing methods for IPv4 and IPv6 */ > if (key->key_len == IPV4_KEYLEN) > - ipv4_frag_hash(key, &sig1, &sig2); > + ipv4_frag_hash(key, &sig1, &sig2, tbl->seed); > else > - ipv6_frag_hash(key, &sig1, &sig2); > + ipv6_frag_hash(key, &sig1, &sig2, tbl->seed); > > p1 = IP_FRAG_TBL_POS(tbl, sig1); > p2 = IP_FRAG_TBL_POS(tbl, sig2); > diff --git a/lib/ip_frag/ip_reassembly.h b/lib/ip_frag/ip_reassembly.h > index 54afed5417..9e1666ef67 100644 > --- a/lib/ip_frag/ip_reassembly.h > +++ b/lib/ip_frag/ip_reassembly.h > @@ -79,6 +79,7 @@ struct rte_ip_frag_tbl { > uint32_t bucket_entries; /* hash associativity. */ > uint32_t nb_entries; /* total size of the table. */ > uint32_t nb_buckets; /* num of associativity lines. */ > + uint32_t seed; /* hash function init value */ > struct ip_frag_pkt *last; /* last used entry. */ > struct ip_pkt_list lru; /* LRU list for table entries. */ > struct ip_frag_tbl_stat stat; /* statistics counters. */ > diff --git a/lib/ip_frag/rte_ip_frag_common.c > b/lib/ip_frag/rte_ip_frag_common.c > index 79ac45289b..00bf9476de 100644 > --- a/lib/ip_frag/rte_ip_frag_common.c > +++ b/lib/ip_frag/rte_ip_frag_common.c > @@ -79,6 +79,7 @@ rte_ip_frag_table_create(uint32_t bucket_num, uint32_t > bucket_entries, > tbl->nb_buckets = bucket_num; > tbl->bucket_entries = bucket_entries; > tbl->entry_mask = (tbl->nb_entries - 1) & ~(tbl->bucket_entries - 1); > + tbl->seed = rte_rand(); > > TAILQ_INIT(&(tbl->lru)); > return tbl; > -- Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com> > 2.53.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-04-13 12:24 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-08 16:16 [PATCH 0/2] ip_frag: TAILQ and hash fixes Stephen Hemminger 2026-04-08 16:16 ` [PATCH 1/2] ip_frag: fix unsafe TAILQ usage Stephen Hemminger 2026-04-13 12:24 ` Konstantin Ananyev 2026-04-08 16:16 ` [PATCH 2/2] ip_frag: randomize hash seed Stephen Hemminger 2026-04-13 12:24 ` Konstantin Ananyev
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox