| Message ID | 20240830090626.18537-2-massimiliano.pellizzer@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | CVE-2024-27397 | expand |
On 30-08-2024 11:05, Massimiliano Pellizzer wrote: > From: Pablo Neira Ayuso <pablo@netfilter.org> > > commit 7395dfacfff65e9938ac0889dafa1ab01e987d15 upstream > > Add a timestamp field at the beginning of the transaction, store it > in the nftables per-netns area. > > Update set backend .insert, .deactivate and sync gc path to use the > timestamp, this avoids that an element expires while control plane > transaction is still unfinished. > > .lookup and .update, which are used from packet path, still use the > current time to check if the element has expired. And .get path and dump > also since this runs lockless under rcu read size lock. Then, there is > async gc which also needs to check the current time since it runs > asynchronously from a workqueue. > > [ NB: rbtree GC updates has been excluded because GC is asynchronous. ] > > Fixes: c3e1b005ed1c ("netfilter: nf_tables: add set element timeout support") > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > (backported from commit eaf1a29ea5d7dba8e84e9e9f3b3f47d0cd540bfe linux-5.4.y) > [mpellizzer: solved a merge conflict due to a variable declaration which > does not affect the patch] > CVE--2024-27397 CVE-2024-27397 Nacking this as I expect to break quite some automation to have this mistake. > Signed-off-by: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com> > --- > include/net/netfilter/nf_tables.h | 21 +++++++++++++++++++-- > net/netfilter/nf_tables_api.c | 1 + > net/netfilter/nft_set_hash.c | 8 +++++++- > net/netfilter/nft_set_rbtree.c | 6 ++++-- > 4 files changed, 31 insertions(+), 5 deletions(-) > > diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h > index 74ee59746f6b..a7672b8a909e 100644 > --- a/include/net/netfilter/nf_tables.h > +++ b/include/net/netfilter/nf_tables.h > @@ -13,6 +13,7 @@ > #include <net/netfilter/nf_flow_table.h> > #include <net/netlink.h> > #include <net/flow_offload.h> > +#include <net/netns/generic.h> > > struct module; > > @@ -656,10 +657,16 @@ static inline struct nft_expr *nft_set_ext_expr(const struct nft_set_ext *ext) > return nft_set_ext(ext, NFT_SET_EXT_EXPR); > } > > -static inline bool nft_set_elem_expired(const struct nft_set_ext *ext) > +static inline bool __nft_set_elem_expired(const struct nft_set_ext *ext, > + u64 tstamp) > { > return nft_set_ext_exists(ext, NFT_SET_EXT_EXPIRATION) && > - time_is_before_eq_jiffies64(*nft_set_ext_expiration(ext)); > + time_after_eq64(tstamp, *nft_set_ext_expiration(ext)); > +} > + > +static inline bool nft_set_elem_expired(const struct nft_set_ext *ext) > +{ > + return __nft_set_elem_expired(ext, get_jiffies_64()); > } > > static inline struct nft_set_ext *nft_set_elem_ext(const struct nft_set *set, > @@ -1488,9 +1495,19 @@ struct nftables_pernet { > struct list_head module_list; > struct list_head notify_list; > struct mutex commit_mutex; > + u64 tstamp; > unsigned int base_seq; > u8 validate_state; > unsigned int gc_seq; > }; > > +extern unsigned int nf_tables_net_id; > + > +static inline u64 nft_net_tstamp(const struct net *net) > +{ > + struct nftables_pernet *nft_net = net_generic(net, nf_tables_net_id); > + > + return nft_net->tstamp; > +} > + > #endif /* _NET_NF_TABLES_H */ > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > index 10e7d8e9df04..14f730a371bd 100644 > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c > @@ -7810,6 +7810,7 @@ static bool nf_tables_valid_genid(struct net *net, u32 genid) > bool genid_ok; > > mutex_lock(&nft_net->commit_mutex); > + nft_net->tstamp = get_jiffies_64(); > > genid_ok = genid == 0 || nft_net->base_seq == genid; > if (!genid_ok) > diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c > index 7f49c6c37c94..92ab00d17337 100644 > --- a/net/netfilter/nft_set_hash.c > +++ b/net/netfilter/nft_set_hash.c > @@ -38,6 +38,7 @@ struct nft_rhash_cmp_arg { > const struct nft_set *set; > const u32 *key; > u8 genmask; > + u64 tstamp; > }; > > static inline u32 nft_rhash_key(const void *data, u32 len, u32 seed) > @@ -64,7 +65,7 @@ static inline int nft_rhash_cmp(struct rhashtable_compare_arg *arg, > return 1; > if (nft_set_elem_is_dead(&he->ext)) > return 1; > - if (nft_set_elem_expired(&he->ext)) > + if (__nft_set_elem_expired(&he->ext, x->tstamp)) > return 1; > if (!nft_set_elem_active(&he->ext, x->genmask)) > return 1; > @@ -88,6 +89,7 @@ static bool nft_rhash_lookup(const struct net *net, const struct nft_set *set, > .genmask = nft_genmask_cur(net), > .set = set, > .key = key, > + .tstamp = get_jiffies_64(), > }; > > he = rhashtable_lookup(&priv->ht, &arg, nft_rhash_params); > @@ -106,6 +108,7 @@ static void *nft_rhash_get(const struct net *net, const struct nft_set *set, > .genmask = nft_genmask_cur(net), > .set = set, > .key = elem->key.val.data, > + .tstamp = get_jiffies_64(), > }; > > he = rhashtable_lookup(&priv->ht, &arg, nft_rhash_params); > @@ -129,6 +132,7 @@ static bool nft_rhash_update(struct nft_set *set, const u32 *key, > .genmask = NFT_GENMASK_ANY, > .set = set, > .key = key, > + .tstamp = get_jiffies_64(), > }; > > he = rhashtable_lookup(&priv->ht, &arg, nft_rhash_params); > @@ -172,6 +176,7 @@ static int nft_rhash_insert(const struct net *net, const struct nft_set *set, > .genmask = nft_genmask_next(net), > .set = set, > .key = elem->key.val.data, > + .tstamp = nft_net_tstamp(net), > }; > struct nft_rhash_elem *prev; > > @@ -214,6 +219,7 @@ static void *nft_rhash_deactivate(const struct net *net, > .genmask = nft_genmask_next(net), > .set = set, > .key = elem->key.val.data, > + .tstamp = nft_net_tstamp(net), > }; > > rcu_read_lock(); > diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c > index ec322d818de0..4594e1d88cf9 100644 > --- a/net/netfilter/nft_set_rbtree.c > +++ b/net/netfilter/nft_set_rbtree.c > @@ -316,6 +316,7 @@ static int __nft_rbtree_insert(const struct net *net, const struct nft_set *set, > struct nft_rbtree *priv = nft_set_priv(set); > u8 cur_genmask = nft_genmask_cur(net); > u8 genmask = nft_genmask_next(net); > + u64 tstamp = nft_net_tstamp(net); > int d; > > /* Descend the tree to search for an existing element greater than the > @@ -363,7 +364,7 @@ static int __nft_rbtree_insert(const struct net *net, const struct nft_set *set, > /* perform garbage collection to avoid bogus overlap reports > * but skip new elements in this transaction. > */ > - if (nft_set_elem_expired(&rbe->ext) && > + if (__nft_set_elem_expired(&rbe->ext, tstamp) && > nft_set_elem_active(&rbe->ext, cur_genmask)) { > const struct nft_rbtree_elem *removed_end; > > @@ -550,6 +551,7 @@ static void *nft_rbtree_deactivate(const struct net *net, > const struct rb_node *parent = priv->root.rb_node; > struct nft_rbtree_elem *rbe, *this = elem->priv; > u8 genmask = nft_genmask_next(net); > + u64 tstamp = nft_net_tstamp(net); > int d; > > while (parent != NULL) { > @@ -570,7 +572,7 @@ static void *nft_rbtree_deactivate(const struct net *net, > nft_rbtree_interval_end(this)) { > parent = parent->rb_right; > continue; > - } else if (nft_set_elem_expired(&rbe->ext)) { > + } else if (__nft_set_elem_expired(&rbe->ext, tstamp)) { > break; > } else if (!nft_set_elem_active(&rbe->ext, genmask)) { > parent = parent->rb_left;
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h index 74ee59746f6b..a7672b8a909e 100644 --- a/include/net/netfilter/nf_tables.h +++ b/include/net/netfilter/nf_tables.h @@ -13,6 +13,7 @@ #include <net/netfilter/nf_flow_table.h> #include <net/netlink.h> #include <net/flow_offload.h> +#include <net/netns/generic.h> struct module; @@ -656,10 +657,16 @@ static inline struct nft_expr *nft_set_ext_expr(const struct nft_set_ext *ext) return nft_set_ext(ext, NFT_SET_EXT_EXPR); } -static inline bool nft_set_elem_expired(const struct nft_set_ext *ext) +static inline bool __nft_set_elem_expired(const struct nft_set_ext *ext, + u64 tstamp) { return nft_set_ext_exists(ext, NFT_SET_EXT_EXPIRATION) && - time_is_before_eq_jiffies64(*nft_set_ext_expiration(ext)); + time_after_eq64(tstamp, *nft_set_ext_expiration(ext)); +} + +static inline bool nft_set_elem_expired(const struct nft_set_ext *ext) +{ + return __nft_set_elem_expired(ext, get_jiffies_64()); } static inline struct nft_set_ext *nft_set_elem_ext(const struct nft_set *set, @@ -1488,9 +1495,19 @@ struct nftables_pernet { struct list_head module_list; struct list_head notify_list; struct mutex commit_mutex; + u64 tstamp; unsigned int base_seq; u8 validate_state; unsigned int gc_seq; }; +extern unsigned int nf_tables_net_id; + +static inline u64 nft_net_tstamp(const struct net *net) +{ + struct nftables_pernet *nft_net = net_generic(net, nf_tables_net_id); + + return nft_net->tstamp; +} + #endif /* _NET_NF_TABLES_H */ diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 10e7d8e9df04..14f730a371bd 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -7810,6 +7810,7 @@ static bool nf_tables_valid_genid(struct net *net, u32 genid) bool genid_ok; mutex_lock(&nft_net->commit_mutex); + nft_net->tstamp = get_jiffies_64(); genid_ok = genid == 0 || nft_net->base_seq == genid; if (!genid_ok) diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c index 7f49c6c37c94..92ab00d17337 100644 --- a/net/netfilter/nft_set_hash.c +++ b/net/netfilter/nft_set_hash.c @@ -38,6 +38,7 @@ struct nft_rhash_cmp_arg { const struct nft_set *set; const u32 *key; u8 genmask; + u64 tstamp; }; static inline u32 nft_rhash_key(const void *data, u32 len, u32 seed) @@ -64,7 +65,7 @@ static inline int nft_rhash_cmp(struct rhashtable_compare_arg *arg, return 1; if (nft_set_elem_is_dead(&he->ext)) return 1; - if (nft_set_elem_expired(&he->ext)) + if (__nft_set_elem_expired(&he->ext, x->tstamp)) return 1; if (!nft_set_elem_active(&he->ext, x->genmask)) return 1; @@ -88,6 +89,7 @@ static bool nft_rhash_lookup(const struct net *net, const struct nft_set *set, .genmask = nft_genmask_cur(net), .set = set, .key = key, + .tstamp = get_jiffies_64(), }; he = rhashtable_lookup(&priv->ht, &arg, nft_rhash_params); @@ -106,6 +108,7 @@ static void *nft_rhash_get(const struct net *net, const struct nft_set *set, .genmask = nft_genmask_cur(net), .set = set, .key = elem->key.val.data, + .tstamp = get_jiffies_64(), }; he = rhashtable_lookup(&priv->ht, &arg, nft_rhash_params); @@ -129,6 +132,7 @@ static bool nft_rhash_update(struct nft_set *set, const u32 *key, .genmask = NFT_GENMASK_ANY, .set = set, .key = key, + .tstamp = get_jiffies_64(), }; he = rhashtable_lookup(&priv->ht, &arg, nft_rhash_params); @@ -172,6 +176,7 @@ static int nft_rhash_insert(const struct net *net, const struct nft_set *set, .genmask = nft_genmask_next(net), .set = set, .key = elem->key.val.data, + .tstamp = nft_net_tstamp(net), }; struct nft_rhash_elem *prev; @@ -214,6 +219,7 @@ static void *nft_rhash_deactivate(const struct net *net, .genmask = nft_genmask_next(net), .set = set, .key = elem->key.val.data, + .tstamp = nft_net_tstamp(net), }; rcu_read_lock(); diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c index ec322d818de0..4594e1d88cf9 100644 --- a/net/netfilter/nft_set_rbtree.c +++ b/net/netfilter/nft_set_rbtree.c @@ -316,6 +316,7 @@ static int __nft_rbtree_insert(const struct net *net, const struct nft_set *set, struct nft_rbtree *priv = nft_set_priv(set); u8 cur_genmask = nft_genmask_cur(net); u8 genmask = nft_genmask_next(net); + u64 tstamp = nft_net_tstamp(net); int d; /* Descend the tree to search for an existing element greater than the @@ -363,7 +364,7 @@ static int __nft_rbtree_insert(const struct net *net, const struct nft_set *set, /* perform garbage collection to avoid bogus overlap reports * but skip new elements in this transaction. */ - if (nft_set_elem_expired(&rbe->ext) && + if (__nft_set_elem_expired(&rbe->ext, tstamp) && nft_set_elem_active(&rbe->ext, cur_genmask)) { const struct nft_rbtree_elem *removed_end; @@ -550,6 +551,7 @@ static void *nft_rbtree_deactivate(const struct net *net, const struct rb_node *parent = priv->root.rb_node; struct nft_rbtree_elem *rbe, *this = elem->priv; u8 genmask = nft_genmask_next(net); + u64 tstamp = nft_net_tstamp(net); int d; while (parent != NULL) { @@ -570,7 +572,7 @@ static void *nft_rbtree_deactivate(const struct net *net, nft_rbtree_interval_end(this)) { parent = parent->rb_right; continue; - } else if (nft_set_elem_expired(&rbe->ext)) { + } else if (__nft_set_elem_expired(&rbe->ext, tstamp)) { break; } else if (!nft_set_elem_active(&rbe->ext, genmask)) { parent = parent->rb_left;