From a82474ff773b3f5940a1939fcc0db1bca7b60fed Mon Sep 17 00:00:00 2001 From: Sasha Levin Date: Thu, 11 May 2023 21:49:30 -0400 Subject: [PATCH] Fixes for 4.14 Signed-off-by: Sasha Levin --- ...les-bogus-ebusy-when-deleting-set-af.patch | 225 +++++++++ ...les-deactivate-anonymous-set-from-pr.patch | 126 +++++ ...les-split-set-destruction-in-deactiv.patch | 270 +++++++++++ ...les-unbind-set-in-rule-from-commit-p.patch | 431 ++++++++++++++++++ ...les-use-after-free-in-failing-rule-w.patch | 126 +++++ ...ter-nft_hash-fix-nft_hash_deactivate.patch | 39 ++ queue-4.14/series | 6 + 7 files changed, 1223 insertions(+) create mode 100644 queue-4.14/netfilter-nf_tables-bogus-ebusy-when-deleting-set-af.patch create mode 100644 queue-4.14/netfilter-nf_tables-deactivate-anonymous-set-from-pr.patch create mode 100644 queue-4.14/netfilter-nf_tables-split-set-destruction-in-deactiv.patch create mode 100644 queue-4.14/netfilter-nf_tables-unbind-set-in-rule-from-commit-p.patch create mode 100644 queue-4.14/netfilter-nf_tables-use-after-free-in-failing-rule-w.patch create mode 100644 queue-4.14/netfilter-nft_hash-fix-nft_hash_deactivate.patch diff --git a/queue-4.14/netfilter-nf_tables-bogus-ebusy-when-deleting-set-af.patch b/queue-4.14/netfilter-nf_tables-bogus-ebusy-when-deleting-set-af.patch new file mode 100644 index 00000000000..78820ddd21a --- /dev/null +++ b/queue-4.14/netfilter-nf_tables-bogus-ebusy-when-deleting-set-af.patch @@ -0,0 +1,225 @@ +From cdef8e239ef7cee1c1a07f4d277502109b65c1d0 Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Thu, 11 May 2023 17:41:42 +0200 +Subject: netfilter: nf_tables: bogus EBUSY when deleting set after flush +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +From: Pablo Neira Ayuso + +[ backport for 4.14 of 273fe3f1006ea5ebc63d6729e43e8e45e32b256a ] + +Set deletion after flush coming in the same batch results in EBUSY. Add +set use counter to track the number of references to this set from +rules. We cannot rely on the list of bindings for this since such list +is still populated from the preparation phase. + +Reported-by: Václav Zindulka +Signed-off-by: Pablo Neira Ayuso +Signed-off-by: Sasha Levin +--- + include/net/netfilter/nf_tables.h | 6 ++++++ + net/netfilter/nf_tables_api.c | 28 +++++++++++++++++++++++++++- + net/netfilter/nft_dynset.c | 13 +++++++++---- + net/netfilter/nft_lookup.c | 13 +++++++++---- + net/netfilter/nft_objref.c | 13 +++++++++---- + 5 files changed, 60 insertions(+), 13 deletions(-) + +diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h +index ca82f32d10cd4..fe56b2f825b4e 100644 +--- a/include/net/netfilter/nf_tables.h ++++ b/include/net/netfilter/nf_tables.h +@@ -383,6 +383,7 @@ void nft_unregister_set(struct nft_set_type *type); + * @dtype: data type (verdict or numeric type defined by userspace) + * @objtype: object type (see NFT_OBJECT_* definitions) + * @size: maximum set size ++ * @use: number of rules references to this set + * @nelems: number of elements + * @ndeact: number of deactivated elements queued for removal + * @timeout: default timeout value in jiffies +@@ -405,6 +406,7 @@ struct nft_set { + u32 dtype; + u32 objtype; + u32 size; ++ u32 use; + atomic_t nelems; + u32 ndeact; + u64 timeout; +@@ -459,6 +461,10 @@ struct nft_set_binding { + u32 flags; + }; + ++enum nft_trans_phase; ++void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set, ++ struct nft_set_binding *binding, ++ enum nft_trans_phase phase); + int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set, + struct nft_set_binding *binding); + void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set, +diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c +index e9e3e7680a14c..2f5b5d563e4d1 100644 +--- a/net/netfilter/nf_tables_api.c ++++ b/net/netfilter/nf_tables_api.c +@@ -3309,6 +3309,9 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk, + + static void nft_set_destroy(struct nft_set *set) + { ++ if (WARN_ON(set->use > 0)) ++ return; ++ + set->ops->destroy(set); + module_put(set->ops->type->owner); + kfree(set->name); +@@ -3339,7 +3342,7 @@ static int nf_tables_delset(struct net *net, struct sock *nlsk, + if (IS_ERR(set)) + return PTR_ERR(set); + +- if (!list_empty(&set->bindings) || ++ if (set->use || + (nlh->nlmsg_flags & NLM_F_NONREC && atomic_read(&set->nelems) > 0)) + return -EBUSY; + +@@ -3367,6 +3370,9 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set, + struct nft_set_binding *i; + struct nft_set_iter iter; + ++ if (set->use == UINT_MAX) ++ return -EOVERFLOW; ++ + if (!list_empty(&set->bindings) && set->flags & NFT_SET_ANONYMOUS) + return -EBUSY; + +@@ -3394,6 +3400,7 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set, + binding->chain = ctx->chain; + list_add_tail_rcu(&binding->list, &set->bindings); + nft_set_trans_bind(ctx, set); ++ set->use++; + + return 0; + } +@@ -3413,6 +3420,25 @@ void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set, + } + EXPORT_SYMBOL_GPL(nf_tables_unbind_set); + ++void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set, ++ struct nft_set_binding *binding, ++ enum nft_trans_phase phase) ++{ ++ switch (phase) { ++ case NFT_TRANS_PREPARE: ++ set->use--; ++ return; ++ case NFT_TRANS_ABORT: ++ case NFT_TRANS_RELEASE: ++ set->use--; ++ /* fall through */ ++ default: ++ nf_tables_unbind_set(ctx, set, binding, ++ phase == NFT_TRANS_COMMIT); ++ } ++} ++EXPORT_SYMBOL_GPL(nf_tables_deactivate_set); ++ + void nf_tables_destroy_set(const struct nft_ctx *ctx, struct nft_set *set) + { + if (list_empty(&set->bindings) && set->flags & NFT_SET_ANONYMOUS) +diff --git a/net/netfilter/nft_dynset.c b/net/netfilter/nft_dynset.c +index 7e0a203437408..a20f1668328dc 100644 +--- a/net/netfilter/nft_dynset.c ++++ b/net/netfilter/nft_dynset.c +@@ -229,11 +229,15 @@ static void nft_dynset_deactivate(const struct nft_ctx *ctx, + { + struct nft_dynset *priv = nft_expr_priv(expr); + +- if (phase == NFT_TRANS_PREPARE) +- return; ++ nf_tables_deactivate_set(ctx, priv->set, &priv->binding, phase); ++} ++ ++static void nft_dynset_activate(const struct nft_ctx *ctx, ++ const struct nft_expr *expr) ++{ ++ struct nft_dynset *priv = nft_expr_priv(expr); + +- nf_tables_unbind_set(ctx, priv->set, &priv->binding, +- phase == NFT_TRANS_COMMIT); ++ priv->set->use++; + } + + static void nft_dynset_destroy(const struct nft_ctx *ctx, +@@ -281,6 +285,7 @@ static const struct nft_expr_ops nft_dynset_ops = { + .eval = nft_dynset_eval, + .init = nft_dynset_init, + .destroy = nft_dynset_destroy, ++ .activate = nft_dynset_activate, + .deactivate = nft_dynset_deactivate, + .dump = nft_dynset_dump, + }; +diff --git a/net/netfilter/nft_lookup.c b/net/netfilter/nft_lookup.c +index 7dd35245222c5..453f84c571662 100644 +--- a/net/netfilter/nft_lookup.c ++++ b/net/netfilter/nft_lookup.c +@@ -124,11 +124,15 @@ static void nft_lookup_deactivate(const struct nft_ctx *ctx, + { + struct nft_lookup *priv = nft_expr_priv(expr); + +- if (phase == NFT_TRANS_PREPARE) +- return; ++ nf_tables_deactivate_set(ctx, priv->set, &priv->binding, phase); ++} ++ ++static void nft_lookup_activate(const struct nft_ctx *ctx, ++ const struct nft_expr *expr) ++{ ++ struct nft_lookup *priv = nft_expr_priv(expr); + +- nf_tables_unbind_set(ctx, priv->set, &priv->binding, +- phase == NFT_TRANS_COMMIT); ++ priv->set->use++; + } + + static void nft_lookup_destroy(const struct nft_ctx *ctx, +@@ -164,6 +168,7 @@ static const struct nft_expr_ops nft_lookup_ops = { + .size = NFT_EXPR_SIZE(sizeof(struct nft_lookup)), + .eval = nft_lookup_eval, + .init = nft_lookup_init, ++ .activate = nft_lookup_activate, + .deactivate = nft_lookup_deactivate, + .destroy = nft_lookup_destroy, + .dump = nft_lookup_dump, +diff --git a/net/netfilter/nft_objref.c b/net/netfilter/nft_objref.c +index f72aeff93efad..7e628f4f02b93 100644 +--- a/net/netfilter/nft_objref.c ++++ b/net/netfilter/nft_objref.c +@@ -160,11 +160,15 @@ static void nft_objref_map_deactivate(const struct nft_ctx *ctx, + { + struct nft_objref_map *priv = nft_expr_priv(expr); + +- if (phase == NFT_TRANS_PREPARE) +- return; ++ nf_tables_deactivate_set(ctx, priv->set, &priv->binding, phase); ++} ++ ++static void nft_objref_map_activate(const struct nft_ctx *ctx, ++ const struct nft_expr *expr) ++{ ++ struct nft_objref_map *priv = nft_expr_priv(expr); + +- nf_tables_unbind_set(ctx, priv->set, &priv->binding, +- phase == NFT_TRANS_COMMIT); ++ priv->set->use++; + } + + static void nft_objref_map_destroy(const struct nft_ctx *ctx, +@@ -181,6 +185,7 @@ static const struct nft_expr_ops nft_objref_map_ops = { + .size = NFT_EXPR_SIZE(sizeof(struct nft_objref_map)), + .eval = nft_objref_map_eval, + .init = nft_objref_map_init, ++ .activate = nft_objref_map_activate, + .deactivate = nft_objref_map_deactivate, + .destroy = nft_objref_map_destroy, + .dump = nft_objref_map_dump, +-- +2.39.2 + diff --git a/queue-4.14/netfilter-nf_tables-deactivate-anonymous-set-from-pr.patch b/queue-4.14/netfilter-nf_tables-deactivate-anonymous-set-from-pr.patch new file mode 100644 index 00000000000..45ae503ff03 --- /dev/null +++ b/queue-4.14/netfilter-nf_tables-deactivate-anonymous-set-from-pr.patch @@ -0,0 +1,126 @@ +From 4d302451b3e5d5dffe9ce989cac7ac2cffa63ce5 Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Thu, 11 May 2023 17:41:43 +0200 +Subject: netfilter: nf_tables: deactivate anonymous set from preparation phase + +From: Pablo Neira Ayuso + +[ backport for 4.14 of c1592a89942e9678f7d9c8030efa777c0d57edab ] + +Toggle deleted anonymous sets as inactive in the next generation, so +users cannot perform any update on it. Clear the generation bitmask +in case the transaction is aborted. + +The following KASAN splat shows a set element deletion for a bound +anonymous set that has been already removed in the same transaction. + +[ 64.921510] ================================================================== +[ 64.923123] BUG: KASAN: wild-memory-access in nf_tables_commit+0xa24/0x1490 [nf_tables] +[ 64.924745] Write of size 8 at addr dead000000000122 by task test/890 +[ 64.927903] CPU: 3 PID: 890 Comm: test Not tainted 6.3.0+ #253 +[ 64.931120] Call Trace: +[ 64.932699] +[ 64.934292] dump_stack_lvl+0x33/0x50 +[ 64.935908] ? nf_tables_commit+0xa24/0x1490 [nf_tables] +[ 64.937551] kasan_report+0xda/0x120 +[ 64.939186] ? nf_tables_commit+0xa24/0x1490 [nf_tables] +[ 64.940814] nf_tables_commit+0xa24/0x1490 [nf_tables] +[ 64.942452] ? __kasan_slab_alloc+0x2d/0x60 +[ 64.944070] ? nf_tables_setelem_notify+0x190/0x190 [nf_tables] +[ 64.945710] ? kasan_set_track+0x21/0x30 +[ 64.947323] nfnetlink_rcv_batch+0x709/0xd90 [nfnetlink] +[ 64.948898] ? nfnetlink_rcv_msg+0x480/0x480 [nfnetlink] + +Signed-off-by: Pablo Neira Ayuso +Signed-off-by: Sasha Levin +--- + include/net/netfilter/nf_tables.h | 1 + + net/netfilter/nf_tables_api.c | 12 ++++++++++++ + net/netfilter/nft_dynset.c | 2 +- + net/netfilter/nft_lookup.c | 2 +- + net/netfilter/nft_objref.c | 2 +- + 5 files changed, 16 insertions(+), 3 deletions(-) + +diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h +index fe56b2f825b4e..2db486e9724c6 100644 +--- a/include/net/netfilter/nf_tables.h ++++ b/include/net/netfilter/nf_tables.h +@@ -462,6 +462,7 @@ struct nft_set_binding { + }; + + enum nft_trans_phase; ++void nf_tables_activate_set(const struct nft_ctx *ctx, struct nft_set *set); + void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set, + struct nft_set_binding *binding, + enum nft_trans_phase phase); +diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c +index 2f5b5d563e4d1..c683a45b8ae53 100644 +--- a/net/netfilter/nf_tables_api.c ++++ b/net/netfilter/nf_tables_api.c +@@ -3420,12 +3420,24 @@ void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set, + } + EXPORT_SYMBOL_GPL(nf_tables_unbind_set); + ++void nf_tables_activate_set(const struct nft_ctx *ctx, struct nft_set *set) ++{ ++ if (set->flags & NFT_SET_ANONYMOUS) ++ nft_clear(ctx->net, set); ++ ++ set->use++; ++} ++EXPORT_SYMBOL_GPL(nf_tables_activate_set); ++ + void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set, + struct nft_set_binding *binding, + enum nft_trans_phase phase) + { + switch (phase) { + case NFT_TRANS_PREPARE: ++ if (set->flags & NFT_SET_ANONYMOUS) ++ nft_deactivate_next(ctx->net, set); ++ + set->use--; + return; + case NFT_TRANS_ABORT: +diff --git a/net/netfilter/nft_dynset.c b/net/netfilter/nft_dynset.c +index a20f1668328dc..74e8fdaa34321 100644 +--- a/net/netfilter/nft_dynset.c ++++ b/net/netfilter/nft_dynset.c +@@ -237,7 +237,7 @@ static void nft_dynset_activate(const struct nft_ctx *ctx, + { + struct nft_dynset *priv = nft_expr_priv(expr); + +- priv->set->use++; ++ nf_tables_activate_set(ctx, priv->set); + } + + static void nft_dynset_destroy(const struct nft_ctx *ctx, +diff --git a/net/netfilter/nft_lookup.c b/net/netfilter/nft_lookup.c +index 453f84c571662..4fcbe51e88c76 100644 +--- a/net/netfilter/nft_lookup.c ++++ b/net/netfilter/nft_lookup.c +@@ -132,7 +132,7 @@ static void nft_lookup_activate(const struct nft_ctx *ctx, + { + struct nft_lookup *priv = nft_expr_priv(expr); + +- priv->set->use++; ++ nf_tables_activate_set(ctx, priv->set); + } + + static void nft_lookup_destroy(const struct nft_ctx *ctx, +diff --git a/net/netfilter/nft_objref.c b/net/netfilter/nft_objref.c +index 7e628f4f02b93..49a067a67e723 100644 +--- a/net/netfilter/nft_objref.c ++++ b/net/netfilter/nft_objref.c +@@ -168,7 +168,7 @@ static void nft_objref_map_activate(const struct nft_ctx *ctx, + { + struct nft_objref_map *priv = nft_expr_priv(expr); + +- priv->set->use++; ++ nf_tables_activate_set(ctx, priv->set); + } + + static void nft_objref_map_destroy(const struct nft_ctx *ctx, +-- +2.39.2 + diff --git a/queue-4.14/netfilter-nf_tables-split-set-destruction-in-deactiv.patch b/queue-4.14/netfilter-nf_tables-split-set-destruction-in-deactiv.patch new file mode 100644 index 00000000000..33c87f520e3 --- /dev/null +++ b/queue-4.14/netfilter-nf_tables-split-set-destruction-in-deactiv.patch @@ -0,0 +1,270 @@ +From 7cb9fc6fa844732d7d0ec681fa5d8465baff2e18 Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Thu, 11 May 2023 17:41:38 +0200 +Subject: netfilter: nf_tables: split set destruction in deactivate and destroy + phase + +From: Florian Westphal + +[ backport for 4.14 of cd5125d8f51882279f50506bb9c7e5e89dc9bef3 ] + +Splits unbind_set into destroy_set and unbinding operation. + +Unbinding removes set from lists (so new transaction would not +find it anymore) but keeps memory allocated (so packet path continues +to work). + +Rebind function is added to allow unrolling in case transaction +that wants to remove set is aborted. + +Destroy function is added to free the memory, but this could occur +outside of transaction in the future. + +Signed-off-by: Florian Westphal +Signed-off-by: Pablo Neira Ayuso +Signed-off-by: Sasha Levin +--- + include/net/netfilter/nf_tables.h | 7 +++++- + net/netfilter/nf_tables_api.c | 36 +++++++++++++++++++++---------- + net/netfilter/nft_dynset.c | 21 +++++++++++++++++- + net/netfilter/nft_lookup.c | 20 ++++++++++++++++- + net/netfilter/nft_objref.c | 20 ++++++++++++++++- + 5 files changed, 89 insertions(+), 15 deletions(-) + +diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h +index 3107895115c25..59da90bb840d9 100644 +--- a/include/net/netfilter/nf_tables.h ++++ b/include/net/netfilter/nf_tables.h +@@ -463,6 +463,9 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set, + struct nft_set_binding *binding); + void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set, + struct nft_set_binding *binding); ++void nf_tables_rebind_set(const struct nft_ctx *ctx, struct nft_set *set, ++ struct nft_set_binding *binding); ++void nf_tables_destroy_set(const struct nft_ctx *ctx, struct nft_set *set); + + /** + * enum nft_set_extensions - set extension type IDs +@@ -716,7 +719,9 @@ struct nft_expr_type { + * @eval: Expression evaluation function + * @size: full expression size, including private data size + * @init: initialization function +- * @destroy: destruction function ++ * @activate: activate expression in the next generation ++ * @deactivate: deactivate expression in next generation ++ * @destroy: destruction function, called after synchronize_rcu + * @dump: function to dump parameters + * @type: expression type + * @validate: validate expression, called during loop detection +diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c +index b24c83cf64b97..39416c568d181 100644 +--- a/net/netfilter/nf_tables_api.c ++++ b/net/netfilter/nf_tables_api.c +@@ -327,7 +327,7 @@ static int nft_delrule_by_chain(struct nft_ctx *ctx) + return 0; + } + +-static int nft_trans_set_add(struct nft_ctx *ctx, int msg_type, ++static int nft_trans_set_add(const struct nft_ctx *ctx, int msg_type, + struct nft_set *set) + { + struct nft_trans *trans; +@@ -347,7 +347,7 @@ static int nft_trans_set_add(struct nft_ctx *ctx, int msg_type, + return 0; + } + +-static int nft_delset(struct nft_ctx *ctx, struct nft_set *set) ++static int nft_delset(const struct nft_ctx *ctx, struct nft_set *set) + { + int err; + +@@ -3311,13 +3311,6 @@ static void nft_set_destroy(struct nft_set *set) + kvfree(set); + } + +-static void nf_tables_set_destroy(const struct nft_ctx *ctx, struct nft_set *set) +-{ +- list_del_rcu(&set->list); +- nf_tables_set_notify(ctx, set, NFT_MSG_DELSET, GFP_ATOMIC); +- nft_set_destroy(set); +-} +- + static int nf_tables_delset(struct net *net, struct sock *nlsk, + struct sk_buff *skb, const struct nlmsghdr *nlh, + const struct nlattr * const nla[], +@@ -3400,17 +3393,38 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set, + } + EXPORT_SYMBOL_GPL(nf_tables_bind_set); + +-void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set, ++void nf_tables_rebind_set(const struct nft_ctx *ctx, struct nft_set *set, + struct nft_set_binding *binding) ++{ ++ if (list_empty(&set->bindings) && set->flags & NFT_SET_ANONYMOUS && ++ nft_is_active(ctx->net, set)) ++ list_add_tail_rcu(&set->list, &ctx->table->sets); ++ ++ list_add_tail_rcu(&binding->list, &set->bindings); ++} ++EXPORT_SYMBOL_GPL(nf_tables_rebind_set); ++ ++void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set, ++ struct nft_set_binding *binding) + { + list_del_rcu(&binding->list); + + if (list_empty(&set->bindings) && set->flags & NFT_SET_ANONYMOUS && + nft_is_active(ctx->net, set)) +- nf_tables_set_destroy(ctx, set); ++ list_del_rcu(&set->list); + } + EXPORT_SYMBOL_GPL(nf_tables_unbind_set); + ++void nf_tables_destroy_set(const struct nft_ctx *ctx, struct nft_set *set) ++{ ++ if (list_empty(&set->bindings) && set->flags & NFT_SET_ANONYMOUS && ++ nft_is_active(ctx->net, set)) { ++ nf_tables_set_notify(ctx, set, NFT_MSG_DELSET, GFP_ATOMIC); ++ nft_set_destroy(set); ++ } ++} ++EXPORT_SYMBOL_GPL(nf_tables_destroy_set); ++ + const struct nft_set_ext_type nft_set_ext_types[] = { + [NFT_SET_EXT_KEY] = { + .align = __alignof__(u32), +diff --git a/net/netfilter/nft_dynset.c b/net/netfilter/nft_dynset.c +index f8688f9bf46ca..800ca627a4577 100644 +--- a/net/netfilter/nft_dynset.c ++++ b/net/netfilter/nft_dynset.c +@@ -223,14 +223,31 @@ static int nft_dynset_init(const struct nft_ctx *ctx, + return err; + } + ++static void nft_dynset_activate(const struct nft_ctx *ctx, ++ const struct nft_expr *expr) ++{ ++ struct nft_dynset *priv = nft_expr_priv(expr); ++ ++ nf_tables_rebind_set(ctx, priv->set, &priv->binding); ++} ++ ++static void nft_dynset_deactivate(const struct nft_ctx *ctx, ++ const struct nft_expr *expr) ++{ ++ struct nft_dynset *priv = nft_expr_priv(expr); ++ ++ nf_tables_unbind_set(ctx, priv->set, &priv->binding); ++} ++ + static void nft_dynset_destroy(const struct nft_ctx *ctx, + const struct nft_expr *expr) + { + struct nft_dynset *priv = nft_expr_priv(expr); + +- nf_tables_unbind_set(ctx, priv->set, &priv->binding); + if (priv->expr != NULL) + nft_expr_destroy(ctx, priv->expr); ++ ++ nf_tables_destroy_set(ctx, priv->set); + } + + static int nft_dynset_dump(struct sk_buff *skb, const struct nft_expr *expr) +@@ -267,6 +284,8 @@ static const struct nft_expr_ops nft_dynset_ops = { + .eval = nft_dynset_eval, + .init = nft_dynset_init, + .destroy = nft_dynset_destroy, ++ .activate = nft_dynset_activate, ++ .deactivate = nft_dynset_deactivate, + .dump = nft_dynset_dump, + }; + +diff --git a/net/netfilter/nft_lookup.c b/net/netfilter/nft_lookup.c +index 44015a151ad69..c34667f4f48a5 100644 +--- a/net/netfilter/nft_lookup.c ++++ b/net/netfilter/nft_lookup.c +@@ -118,12 +118,28 @@ static int nft_lookup_init(const struct nft_ctx *ctx, + return 0; + } + ++static void nft_lookup_activate(const struct nft_ctx *ctx, ++ const struct nft_expr *expr) ++{ ++ struct nft_lookup *priv = nft_expr_priv(expr); ++ ++ nf_tables_rebind_set(ctx, priv->set, &priv->binding); ++} ++ ++static void nft_lookup_deactivate(const struct nft_ctx *ctx, ++ const struct nft_expr *expr) ++{ ++ struct nft_lookup *priv = nft_expr_priv(expr); ++ ++ nf_tables_unbind_set(ctx, priv->set, &priv->binding); ++} ++ + static void nft_lookup_destroy(const struct nft_ctx *ctx, + const struct nft_expr *expr) + { + struct nft_lookup *priv = nft_expr_priv(expr); + +- nf_tables_unbind_set(ctx, priv->set, &priv->binding); ++ nf_tables_destroy_set(ctx, priv->set); + } + + static int nft_lookup_dump(struct sk_buff *skb, const struct nft_expr *expr) +@@ -151,6 +167,8 @@ static const struct nft_expr_ops nft_lookup_ops = { + .size = NFT_EXPR_SIZE(sizeof(struct nft_lookup)), + .eval = nft_lookup_eval, + .init = nft_lookup_init, ++ .activate = nft_lookup_activate, ++ .deactivate = nft_lookup_deactivate, + .destroy = nft_lookup_destroy, + .dump = nft_lookup_dump, + }; +diff --git a/net/netfilter/nft_objref.c b/net/netfilter/nft_objref.c +index 7bcdc48f3d737..d289fa5e4eb96 100644 +--- a/net/netfilter/nft_objref.c ++++ b/net/netfilter/nft_objref.c +@@ -154,12 +154,28 @@ static int nft_objref_map_dump(struct sk_buff *skb, const struct nft_expr *expr) + return -1; + } + ++static void nft_objref_map_activate(const struct nft_ctx *ctx, ++ const struct nft_expr *expr) ++{ ++ struct nft_objref_map *priv = nft_expr_priv(expr); ++ ++ nf_tables_rebind_set(ctx, priv->set, &priv->binding); ++} ++ ++static void nft_objref_map_deactivate(const struct nft_ctx *ctx, ++ const struct nft_expr *expr) ++{ ++ struct nft_objref_map *priv = nft_expr_priv(expr); ++ ++ nf_tables_unbind_set(ctx, priv->set, &priv->binding); ++} ++ + static void nft_objref_map_destroy(const struct nft_ctx *ctx, + const struct nft_expr *expr) + { + struct nft_objref_map *priv = nft_expr_priv(expr); + +- nf_tables_unbind_set(ctx, priv->set, &priv->binding); ++ nf_tables_destroy_set(ctx, priv->set); + } + + static struct nft_expr_type nft_objref_type; +@@ -168,6 +184,8 @@ static const struct nft_expr_ops nft_objref_map_ops = { + .size = NFT_EXPR_SIZE(sizeof(struct nft_objref_map)), + .eval = nft_objref_map_eval, + .init = nft_objref_map_init, ++ .activate = nft_objref_map_activate, ++ .deactivate = nft_objref_map_deactivate, + .destroy = nft_objref_map_destroy, + .dump = nft_objref_map_dump, + }; +-- +2.39.2 + diff --git a/queue-4.14/netfilter-nf_tables-unbind-set-in-rule-from-commit-p.patch b/queue-4.14/netfilter-nf_tables-unbind-set-in-rule-from-commit-p.patch new file mode 100644 index 00000000000..84c9a8e4c7f --- /dev/null +++ b/queue-4.14/netfilter-nf_tables-unbind-set-in-rule-from-commit-p.patch @@ -0,0 +1,431 @@ +From 481535a26e2e2d44a73f709d7a859a192183b3d0 Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Thu, 11 May 2023 17:41:39 +0200 +Subject: netfilter: nf_tables: unbind set in rule from commit path + +From: Pablo Neira Ayuso + +[ backport for 4.14 of f6ac8585897684374a19863fff21186a05805286 ] + +Anonymous sets that are bound to rules from the same transaction trigger +a kernel splat from the abort path due to double set list removal and +double free. + +This patch updates the logic to search for the transaction that is +responsible for creating the set and disable the set list removal and +release, given the rule is now responsible for this. Lookup is reverse +since the transaction that adds the set is likely to be at the tail of +the list. + +Moreover, this patch adds the unbind step to deliver the event from the +commit path. This should not be done from the worker thread, since we +have no guarantees of in-order delivery to the listener. + +This patch removes the assumption that both activate and deactivate +callbacks need to be provided. + +Fixes: cd5125d8f518 ("netfilter: nf_tables: split set destruction in deactivate and destroy phase") +Reported-by: Mikhail Morfikov +Signed-off-by: Pablo Neira Ayuso +Signed-off-by: Sasha Levin +--- + include/net/netfilter/nf_tables.h | 17 +++++-- + net/netfilter/nf_tables_api.c | 85 +++++++++++++++---------------- + net/netfilter/nft_dynset.c | 18 +++---- + net/netfilter/nft_immediate.c | 6 ++- + net/netfilter/nft_lookup.c | 18 +++---- + net/netfilter/nft_objref.c | 18 +++---- + 6 files changed, 80 insertions(+), 82 deletions(-) + +diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h +index 59da90bb840d9..cc6ba7e593e74 100644 +--- a/include/net/netfilter/nf_tables.h ++++ b/include/net/netfilter/nf_tables.h +@@ -462,9 +462,7 @@ struct nft_set_binding { + int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set, + struct nft_set_binding *binding); + void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set, +- struct nft_set_binding *binding); +-void nf_tables_rebind_set(const struct nft_ctx *ctx, struct nft_set *set, +- struct nft_set_binding *binding); ++ struct nft_set_binding *binding, bool commit); + void nf_tables_destroy_set(const struct nft_ctx *ctx, struct nft_set *set); + + /** +@@ -713,6 +711,13 @@ struct nft_expr_type { + + #define NFT_EXPR_STATEFUL 0x1 + ++enum nft_trans_phase { ++ NFT_TRANS_PREPARE, ++ NFT_TRANS_ABORT, ++ NFT_TRANS_COMMIT, ++ NFT_TRANS_RELEASE ++}; ++ + /** + * struct nft_expr_ops - nf_tables expression operations + * +@@ -742,7 +747,8 @@ struct nft_expr_ops { + void (*activate)(const struct nft_ctx *ctx, + const struct nft_expr *expr); + void (*deactivate)(const struct nft_ctx *ctx, +- const struct nft_expr *expr); ++ const struct nft_expr *expr, ++ enum nft_trans_phase phase); + void (*destroy)(const struct nft_ctx *ctx, + const struct nft_expr *expr); + int (*dump)(struct sk_buff *skb, +@@ -1290,12 +1296,15 @@ struct nft_trans_rule { + struct nft_trans_set { + struct nft_set *set; + u32 set_id; ++ bool bound; + }; + + #define nft_trans_set(trans) \ + (((struct nft_trans_set *)trans->data)->set) + #define nft_trans_set_id(trans) \ + (((struct nft_trans_set *)trans->data)->set_id) ++#define nft_trans_set_bound(trans) \ ++ (((struct nft_trans_set *)trans->data)->bound) + + struct nft_trans_chain { + bool update; +diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c +index 39416c568d181..5541ba7cc4a01 100644 +--- a/net/netfilter/nf_tables_api.c ++++ b/net/netfilter/nf_tables_api.c +@@ -140,6 +140,23 @@ static void nft_trans_destroy(struct nft_trans *trans) + kfree(trans); + } + ++static void nft_set_trans_bind(const struct nft_ctx *ctx, struct nft_set *set) ++{ ++ struct net *net = ctx->net; ++ struct nft_trans *trans; ++ ++ if (!(set->flags & NFT_SET_ANONYMOUS)) ++ return; ++ ++ list_for_each_entry_reverse(trans, &net->nft.commit_list, list) { ++ if (trans->msg_type == NFT_MSG_NEWSET && ++ nft_trans_set(trans) == set) { ++ nft_trans_set_bound(trans) = true; ++ break; ++ } ++ } ++} ++ + static int nf_tables_register_hooks(struct net *net, + const struct nft_table *table, + struct nft_chain *chain, +@@ -221,18 +238,6 @@ static int nft_delchain(struct nft_ctx *ctx) + return err; + } + +-/* either expr ops provide both activate/deactivate, or neither */ +-static bool nft_expr_check_ops(const struct nft_expr_ops *ops) +-{ +- if (!ops) +- return true; +- +- if (WARN_ON_ONCE((!ops->activate ^ !ops->deactivate))) +- return false; +- +- return true; +-} +- + static void nft_rule_expr_activate(const struct nft_ctx *ctx, + struct nft_rule *rule) + { +@@ -248,14 +253,15 @@ static void nft_rule_expr_activate(const struct nft_ctx *ctx, + } + + static void nft_rule_expr_deactivate(const struct nft_ctx *ctx, +- struct nft_rule *rule) ++ struct nft_rule *rule, ++ enum nft_trans_phase phase) + { + struct nft_expr *expr; + + expr = nft_expr_first(rule); + while (expr != nft_expr_last(rule) && expr->ops) { + if (expr->ops->deactivate) +- expr->ops->deactivate(ctx, expr); ++ expr->ops->deactivate(ctx, expr, phase); + + expr = nft_expr_next(expr); + } +@@ -306,7 +312,7 @@ static int nft_delrule(struct nft_ctx *ctx, struct nft_rule *rule) + nft_trans_destroy(trans); + return err; + } +- nft_rule_expr_deactivate(ctx, rule); ++ nft_rule_expr_deactivate(ctx, rule, NFT_TRANS_PREPARE); + + return 0; + } +@@ -1737,9 +1743,6 @@ static int nf_tables_delchain(struct net *net, struct sock *nlsk, + */ + int nft_register_expr(struct nft_expr_type *type) + { +- if (!nft_expr_check_ops(type->ops)) +- return -EINVAL; +- + nfnl_lock(NFNL_SUBSYS_NFTABLES); + if (type->family == NFPROTO_UNSPEC) + list_add_tail_rcu(&type->list, &nf_tables_expressions); +@@ -1889,10 +1892,6 @@ static int nf_tables_expr_parse(const struct nft_ctx *ctx, + err = PTR_ERR(ops); + goto err1; + } +- if (!nft_expr_check_ops(ops)) { +- err = -EINVAL; +- goto err1; +- } + } else + ops = type->ops; + +@@ -2297,7 +2296,7 @@ static void nf_tables_rule_destroy(const struct nft_ctx *ctx, + static void nf_tables_rule_release(const struct nft_ctx *ctx, + struct nft_rule *rule) + { +- nft_rule_expr_deactivate(ctx, rule); ++ nft_rule_expr_deactivate(ctx, rule, NFT_TRANS_RELEASE); + nf_tables_rule_destroy(ctx, rule); + } + +@@ -3389,39 +3388,30 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set, + bind: + binding->chain = ctx->chain; + list_add_tail_rcu(&binding->list, &set->bindings); ++ nft_set_trans_bind(ctx, set); ++ + return 0; + } + EXPORT_SYMBOL_GPL(nf_tables_bind_set); + +-void nf_tables_rebind_set(const struct nft_ctx *ctx, struct nft_set *set, +- struct nft_set_binding *binding) +-{ +- if (list_empty(&set->bindings) && set->flags & NFT_SET_ANONYMOUS && +- nft_is_active(ctx->net, set)) +- list_add_tail_rcu(&set->list, &ctx->table->sets); +- +- list_add_tail_rcu(&binding->list, &set->bindings); +-} +-EXPORT_SYMBOL_GPL(nf_tables_rebind_set); +- + void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set, +- struct nft_set_binding *binding) ++ struct nft_set_binding *binding, bool event) + { + list_del_rcu(&binding->list); + +- if (list_empty(&set->bindings) && set->flags & NFT_SET_ANONYMOUS && +- nft_is_active(ctx->net, set)) ++ if (list_empty(&set->bindings) && set->flags & NFT_SET_ANONYMOUS) { + list_del_rcu(&set->list); ++ if (event) ++ nf_tables_set_notify(ctx, set, NFT_MSG_DELSET, ++ GFP_KERNEL); ++ } + } + EXPORT_SYMBOL_GPL(nf_tables_unbind_set); + + void nf_tables_destroy_set(const struct nft_ctx *ctx, struct nft_set *set) + { +- if (list_empty(&set->bindings) && set->flags & NFT_SET_ANONYMOUS && +- nft_is_active(ctx->net, set)) { +- nf_tables_set_notify(ctx, set, NFT_MSG_DELSET, GFP_ATOMIC); ++ if (list_empty(&set->bindings) && set->flags & NFT_SET_ANONYMOUS) + nft_set_destroy(set); +- } + } + EXPORT_SYMBOL_GPL(nf_tables_destroy_set); + +@@ -5197,6 +5187,9 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb) + nf_tables_rule_notify(&trans->ctx, + nft_trans_rule(trans), + NFT_MSG_DELRULE); ++ nft_rule_expr_deactivate(&trans->ctx, ++ nft_trans_rule(trans), ++ NFT_TRANS_COMMIT); + break; + case NFT_MSG_NEWSET: + nft_clear(net, nft_trans_set(trans)); +@@ -5274,7 +5267,8 @@ static void nf_tables_abort_release(struct nft_trans *trans) + nf_tables_rule_destroy(&trans->ctx, nft_trans_rule(trans)); + break; + case NFT_MSG_NEWSET: +- nft_set_destroy(nft_trans_set(trans)); ++ if (!nft_trans_set_bound(trans)) ++ nft_set_destroy(nft_trans_set(trans)); + break; + case NFT_MSG_NEWSETELEM: + nft_set_elem_destroy(nft_trans_elem_set(trans), +@@ -5334,7 +5328,9 @@ static int nf_tables_abort(struct net *net, struct sk_buff *skb) + case NFT_MSG_NEWRULE: + trans->ctx.chain->use--; + list_del_rcu(&nft_trans_rule(trans)->list); +- nft_rule_expr_deactivate(&trans->ctx, nft_trans_rule(trans)); ++ nft_rule_expr_deactivate(&trans->ctx, ++ nft_trans_rule(trans), ++ NFT_TRANS_ABORT); + break; + case NFT_MSG_DELRULE: + trans->ctx.chain->use++; +@@ -5344,7 +5340,8 @@ static int nf_tables_abort(struct net *net, struct sk_buff *skb) + break; + case NFT_MSG_NEWSET: + trans->ctx.table->use--; +- list_del_rcu(&nft_trans_set(trans)->list); ++ if (!nft_trans_set_bound(trans)) ++ list_del_rcu(&nft_trans_set(trans)->list); + break; + case NFT_MSG_DELSET: + trans->ctx.table->use++; +diff --git a/net/netfilter/nft_dynset.c b/net/netfilter/nft_dynset.c +index 800ca627a4577..7e0a203437408 100644 +--- a/net/netfilter/nft_dynset.c ++++ b/net/netfilter/nft_dynset.c +@@ -223,20 +223,17 @@ static int nft_dynset_init(const struct nft_ctx *ctx, + return err; + } + +-static void nft_dynset_activate(const struct nft_ctx *ctx, +- const struct nft_expr *expr) +-{ +- struct nft_dynset *priv = nft_expr_priv(expr); +- +- nf_tables_rebind_set(ctx, priv->set, &priv->binding); +-} +- + static void nft_dynset_deactivate(const struct nft_ctx *ctx, +- const struct nft_expr *expr) ++ const struct nft_expr *expr, ++ enum nft_trans_phase phase) + { + struct nft_dynset *priv = nft_expr_priv(expr); + +- nf_tables_unbind_set(ctx, priv->set, &priv->binding); ++ if (phase == NFT_TRANS_PREPARE) ++ return; ++ ++ nf_tables_unbind_set(ctx, priv->set, &priv->binding, ++ phase == NFT_TRANS_COMMIT); + } + + static void nft_dynset_destroy(const struct nft_ctx *ctx, +@@ -284,7 +281,6 @@ static const struct nft_expr_ops nft_dynset_ops = { + .eval = nft_dynset_eval, + .init = nft_dynset_init, + .destroy = nft_dynset_destroy, +- .activate = nft_dynset_activate, + .deactivate = nft_dynset_deactivate, + .dump = nft_dynset_dump, + }; +diff --git a/net/netfilter/nft_immediate.c b/net/netfilter/nft_immediate.c +index aa87ff8beae82..86fd35018b4a6 100644 +--- a/net/netfilter/nft_immediate.c ++++ b/net/netfilter/nft_immediate.c +@@ -78,10 +78,14 @@ static void nft_immediate_activate(const struct nft_ctx *ctx, + } + + static void nft_immediate_deactivate(const struct nft_ctx *ctx, +- const struct nft_expr *expr) ++ const struct nft_expr *expr, ++ enum nft_trans_phase phase) + { + const struct nft_immediate_expr *priv = nft_expr_priv(expr); + ++ if (phase == NFT_TRANS_COMMIT) ++ return; ++ + return nft_data_release(&priv->data, nft_dreg_to_type(priv->dreg)); + } + +diff --git a/net/netfilter/nft_lookup.c b/net/netfilter/nft_lookup.c +index c34667f4f48a5..7dd35245222c5 100644 +--- a/net/netfilter/nft_lookup.c ++++ b/net/netfilter/nft_lookup.c +@@ -118,20 +118,17 @@ static int nft_lookup_init(const struct nft_ctx *ctx, + return 0; + } + +-static void nft_lookup_activate(const struct nft_ctx *ctx, +- const struct nft_expr *expr) +-{ +- struct nft_lookup *priv = nft_expr_priv(expr); +- +- nf_tables_rebind_set(ctx, priv->set, &priv->binding); +-} +- + static void nft_lookup_deactivate(const struct nft_ctx *ctx, +- const struct nft_expr *expr) ++ const struct nft_expr *expr, ++ enum nft_trans_phase phase) + { + struct nft_lookup *priv = nft_expr_priv(expr); + +- nf_tables_unbind_set(ctx, priv->set, &priv->binding); ++ if (phase == NFT_TRANS_PREPARE) ++ return; ++ ++ nf_tables_unbind_set(ctx, priv->set, &priv->binding, ++ phase == NFT_TRANS_COMMIT); + } + + static void nft_lookup_destroy(const struct nft_ctx *ctx, +@@ -167,7 +164,6 @@ static const struct nft_expr_ops nft_lookup_ops = { + .size = NFT_EXPR_SIZE(sizeof(struct nft_lookup)), + .eval = nft_lookup_eval, + .init = nft_lookup_init, +- .activate = nft_lookup_activate, + .deactivate = nft_lookup_deactivate, + .destroy = nft_lookup_destroy, + .dump = nft_lookup_dump, +diff --git a/net/netfilter/nft_objref.c b/net/netfilter/nft_objref.c +index d289fa5e4eb96..f72aeff93efad 100644 +--- a/net/netfilter/nft_objref.c ++++ b/net/netfilter/nft_objref.c +@@ -154,20 +154,17 @@ static int nft_objref_map_dump(struct sk_buff *skb, const struct nft_expr *expr) + return -1; + } + +-static void nft_objref_map_activate(const struct nft_ctx *ctx, +- const struct nft_expr *expr) +-{ +- struct nft_objref_map *priv = nft_expr_priv(expr); +- +- nf_tables_rebind_set(ctx, priv->set, &priv->binding); +-} +- + static void nft_objref_map_deactivate(const struct nft_ctx *ctx, +- const struct nft_expr *expr) ++ const struct nft_expr *expr, ++ enum nft_trans_phase phase) + { + struct nft_objref_map *priv = nft_expr_priv(expr); + +- nf_tables_unbind_set(ctx, priv->set, &priv->binding); ++ if (phase == NFT_TRANS_PREPARE) ++ return; ++ ++ nf_tables_unbind_set(ctx, priv->set, &priv->binding, ++ phase == NFT_TRANS_COMMIT); + } + + static void nft_objref_map_destroy(const struct nft_ctx *ctx, +@@ -184,7 +181,6 @@ static const struct nft_expr_ops nft_objref_map_ops = { + .size = NFT_EXPR_SIZE(sizeof(struct nft_objref_map)), + .eval = nft_objref_map_eval, + .init = nft_objref_map_init, +- .activate = nft_objref_map_activate, + .deactivate = nft_objref_map_deactivate, + .destroy = nft_objref_map_destroy, + .dump = nft_objref_map_dump, +-- +2.39.2 + diff --git a/queue-4.14/netfilter-nf_tables-use-after-free-in-failing-rule-w.patch b/queue-4.14/netfilter-nf_tables-use-after-free-in-failing-rule-w.patch new file mode 100644 index 00000000000..12941fcc54e --- /dev/null +++ b/queue-4.14/netfilter-nf_tables-use-after-free-in-failing-rule-w.patch @@ -0,0 +1,126 @@ +From 469bb74639a86d157a7153e43514047022f9a01b Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Thu, 11 May 2023 17:41:41 +0200 +Subject: netfilter: nf_tables: use-after-free in failing rule with bound set + +From: Pablo Neira Ayuso + +[ backport for 4.14 of 6a0a8d10a3661a036b55af695542a714c429ab7c ] + +If a rule that has already a bound anonymous set fails to be added, the +preparation phase releases the rule and the bound set. However, the +transaction object from the abort path still has a reference to the set +object that is stale, leading to a use-after-free when checking for the +set->bound field. Add a new field to the transaction that specifies if +the set is bound, so the abort path can skip releasing it since the rule +command owns it and it takes care of releasing it. After this update, +the set->bound field is removed. + +[ 24.649883] Unable to handle kernel paging request at virtual address 0000000000040434 +[ 24.657858] Mem abort info: +[ 24.660686] ESR = 0x96000004 +[ 24.663769] Exception class = DABT (current EL), IL = 32 bits +[ 24.669725] SET = 0, FnV = 0 +[ 24.672804] EA = 0, S1PTW = 0 +[ 24.675975] Data abort info: +[ 24.678880] ISV = 0, ISS = 0x00000004 +[ 24.682743] CM = 0, WnR = 0 +[ 24.685723] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000428952000 +[ 24.692207] [0000000000040434] pgd=0000000000000000 +[ 24.697119] Internal error: Oops: 96000004 [#1] SMP +[...] +[ 24.889414] Call trace: +[ 24.891870] __nf_tables_abort+0x3f0/0x7a0 +[ 24.895984] nf_tables_abort+0x20/0x40 +[ 24.899750] nfnetlink_rcv_batch+0x17c/0x588 +[ 24.904037] nfnetlink_rcv+0x13c/0x190 +[ 24.907803] netlink_unicast+0x18c/0x208 +[ 24.911742] netlink_sendmsg+0x1b0/0x350 +[ 24.915682] sock_sendmsg+0x4c/0x68 +[ 24.919185] ___sys_sendmsg+0x288/0x2c8 +[ 24.923037] __sys_sendmsg+0x7c/0xd0 +[ 24.926628] __arm64_sys_sendmsg+0x2c/0x38 +[ 24.930744] el0_svc_common.constprop.0+0x94/0x158 +[ 24.935556] el0_svc_handler+0x34/0x90 +[ 24.939322] el0_svc+0x8/0xc +[ 24.942216] Code: 37280300 f9404023 91014262 aa1703e0 (f9401863) +[ 24.948336] ---[ end trace cebbb9dcbed3b56f ]--- + +Fixes: f6ac85858976 ("netfilter: nf_tables: unbind set in rule from commit path") +Signed-off-by: Pablo Neira Ayuso +Signed-off-by: Sasha Levin +--- + include/net/netfilter/nf_tables.h | 3 +++ + net/netfilter/nf_tables_api.c | 22 +++++++++++++++++----- + 2 files changed, 20 insertions(+), 5 deletions(-) + +diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h +index cc6ba7e593e74..ca82f32d10cd4 100644 +--- a/include/net/netfilter/nf_tables.h ++++ b/include/net/netfilter/nf_tables.h +@@ -1335,12 +1335,15 @@ struct nft_trans_table { + struct nft_trans_elem { + struct nft_set *set; + struct nft_set_elem elem; ++ bool bound; + }; + + #define nft_trans_elem_set(trans) \ + (((struct nft_trans_elem *)trans->data)->set) + #define nft_trans_elem(trans) \ + (((struct nft_trans_elem *)trans->data)->elem) ++#define nft_trans_elem_set_bound(trans) \ ++ (((struct nft_trans_elem *)trans->data)->bound) + + struct nft_trans_obj { + struct nft_object *obj; +diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c +index 5541ba7cc4a01..e9e3e7680a14c 100644 +--- a/net/netfilter/nf_tables_api.c ++++ b/net/netfilter/nf_tables_api.c +@@ -149,9 +149,14 @@ static void nft_set_trans_bind(const struct nft_ctx *ctx, struct nft_set *set) + return; + + list_for_each_entry_reverse(trans, &net->nft.commit_list, list) { +- if (trans->msg_type == NFT_MSG_NEWSET && +- nft_trans_set(trans) == set) { +- nft_trans_set_bound(trans) = true; ++ switch (trans->msg_type) { ++ case NFT_MSG_NEWSET: ++ if (nft_trans_set(trans) == set) ++ nft_trans_set_bound(trans) = true; ++ break; ++ case NFT_MSG_NEWSETELEM: ++ if (nft_trans_elem_set(trans) == set) ++ nft_trans_elem_set_bound(trans) = true; + break; + } + } +@@ -5340,8 +5345,11 @@ static int nf_tables_abort(struct net *net, struct sk_buff *skb) + break; + case NFT_MSG_NEWSET: + trans->ctx.table->use--; +- if (!nft_trans_set_bound(trans)) +- list_del_rcu(&nft_trans_set(trans)->list); ++ if (nft_trans_set_bound(trans)) { ++ nft_trans_destroy(trans); ++ break; ++ } ++ list_del_rcu(&nft_trans_set(trans)->list); + break; + case NFT_MSG_DELSET: + trans->ctx.table->use++; +@@ -5349,6 +5357,10 @@ static int nf_tables_abort(struct net *net, struct sk_buff *skb) + nft_trans_destroy(trans); + break; + case NFT_MSG_NEWSETELEM: ++ if (nft_trans_elem_set_bound(trans)) { ++ nft_trans_destroy(trans); ++ break; ++ } + te = (struct nft_trans_elem *)trans->data; + + te->set->ops->remove(net, te->set, &te->elem); +-- +2.39.2 + diff --git a/queue-4.14/netfilter-nft_hash-fix-nft_hash_deactivate.patch b/queue-4.14/netfilter-nft_hash-fix-nft_hash_deactivate.patch new file mode 100644 index 00000000000..aa2b8203355 --- /dev/null +++ b/queue-4.14/netfilter-nft_hash-fix-nft_hash_deactivate.patch @@ -0,0 +1,39 @@ +From 54677226b53aa6edeed6f2d88a7c2d8388807397 Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Thu, 11 May 2023 17:41:40 +0200 +Subject: netfilter: nft_hash: fix nft_hash_deactivate +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +From: Pablo Neira Ayuso + +[ backport for 4.14 of 7f4dae2d7f03d2aaf3b7d8343d4509c8d9d7ca9b ] + +Jindřich Makovička says: + The logical OR looks fishy to me. Shouldn't be && there instead? + +Link: https://bugzilla.netfilter.org/show_bug.cgi?id=1199 +Signed-off-by: Florian Westphal +Signed-off-by: Pablo Neira Ayuso +Signed-off-by: Sasha Levin +--- + net/netfilter/nft_set_hash.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c +index a684234bd229c..eb7db31dd1733 100644 +--- a/net/netfilter/nft_set_hash.c ++++ b/net/netfilter/nft_set_hash.c +@@ -520,7 +520,7 @@ static void *nft_hash_deactivate(const struct net *net, + hash = nft_jhash(set, priv, &this->ext); + hlist_for_each_entry(he, &priv->table[hash], node) { + if (!memcmp(nft_set_ext_key(&this->ext), &elem->key.val, +- set->klen) || ++ set->klen) && + nft_set_elem_active(&he->ext, genmask)) { + nft_set_elem_change_active(net, set, &he->ext); + return he; +-- +2.39.2 + diff --git a/queue-4.14/series b/queue-4.14/series index 00e05a07085..e189c22a43c 100644 --- a/queue-4.14/series +++ b/queue-4.14/series @@ -80,3 +80,9 @@ dm-integrity-call-kmem_cache_destroy-in-dm_integrity_init-error-path.patch dm-flakey-fix-a-crash-with-invalid-table-line.patch dm-ioctl-fix-nested-locking-in-table_clear-to-remove-deadlock-concern.patch perf-auxtrace-fix-address-filter-entire-kernel-size.patch +netfilter-nf_tables-split-set-destruction-in-deactiv.patch +netfilter-nf_tables-unbind-set-in-rule-from-commit-p.patch +netfilter-nft_hash-fix-nft_hash_deactivate.patch +netfilter-nf_tables-use-after-free-in-failing-rule-w.patch +netfilter-nf_tables-bogus-ebusy-when-deleting-set-af.patch +netfilter-nf_tables-deactivate-anonymous-set-from-pr.patch -- 2.47.3