From 7972a497548bcdfc7f3b6f24a1fd5e4b0e008fff Mon Sep 17 00:00:00 2001 From: Ryan Sullivan Date: Thu, 15 Jun 2023 10:43:28 -0400 Subject: [KPATCH CVE-2023-32233] kpatch fixes for CVE-2023-32233 Kernels: 3.10.0-1160.81.1.el7 3.10.0-1160.83.1.el7 3.10.0-1160.88.1.el7 3.10.0-1160.90.1.el7 3.10.0-1160.92.1.el7 Kpatch-MR: https://gitlab.com/redhat/prdsc/rhel/src/kpatch/rhel-7/-/merge_requests/54 Approved-by: Yannick Cote (@ycote1) Approved-by: Joe Lawrence (@joe.lawrence) Changes since last build: [x86_64]: l2cap_core.o: changed function: l2cap_data_channel l2cap_core.o: changed function: l2cap_rx_state_recv nf_tables_api.o: changed function: __nf_tables_abort nf_tables_api.o: changed function: nf_tables_delsetelem nf_tables_api.o: changed function: nf_tables_newset nf_tables_api.o: changed function: nft_delrule nf_tables_api.o: changed function: nft_set_destroy nf_tables_api.o: changed function: nft_validate_register_store nf_tables_api.o: new function: nf_tables_activate_set nf_tables_api.o: new function: nf_tables_deactivate_set nf_tables_api.o: new function: nft_rule_expr_deactivate nft_dynset.o: new function: klp_is_nft_dynset nft_dynset.o: new function: nft_dynset_activate nft_dynset.o: new function: nft_dynset_deactivate nft_lookup.o: new function: klp_is_nft_lookup nft_lookup.o: new function: nft_lookup_activate nft_lookup.o: new function: nft_lookup_deactivate [ppc64le]: l2cap_core.o: changed function: l2cap_data_channel l2cap_core.o: changed function: l2cap_rx_state_recv nf_tables_api.o: changed function: __nf_tables_abort nf_tables_api.o: changed function: nf_tables_bind_check_setelem nf_tables_api.o: changed function: nf_tables_bind_set nf_tables_api.o: changed function: nf_tables_commit nf_tables_api.o: changed function: nf_tables_delrule nf_tables_api.o: changed function: nf_tables_delsetelem nf_tables_api.o: changed function: nf_tables_newset nf_tables_api.o: changed function: nf_tables_unbind_set nf_tables_api.o: changed function: nft_add_set_elem nf_tables_api.o: changed function: nft_delrule_by_chain nf_tables_api.o: changed function: nft_unregister_afinfo nf_tables_api.o: changed function: nft_validate_register_store nf_tables_api.o: new function: nf_tables_activate_set nf_tables_api.o: new function: nf_tables_deactivate_set nft_dynset.o: new function: klp_is_nft_dynset nft_dynset.o: new function: nft_dynset_activate nft_dynset.o: new function: nft_dynset_deactivate nft_lookup.o: new function: klp_is_nft_lookup nft_lookup.o: new function: nft_lookup_activate nft_lookup.o: new function: nft_lookup_deactivate --------------------------- Modifications: - Removes prototype definitions of nf_tables_activate_set() and nf_tables_deactivate_set() from nf_tables.h and moves them into the affected files above when they are called - Adds optimization attribute "-fno-optimize-sibling-calls" to the nf_tables_deactivate_set() - Removes definitions/edits of the policy and removed fields from the nft_set struct instead using a shadow variable (ID = KLP_CVE_2023_32233), created in nf_tables_newset() and destroyed in nft_set_destroy() - Removes .activate and .deactivate from nft_(dynset/lookup)_ops and instead changes nft_rule_expr_(activate/deactivate) to directly call activate and deactivate functions from within them commit ffb7eb4b21c69f2b5e084c85b37eb033544e3fc9 Author: Florian Westphal Date: Tue May 16 13:34:35 2023 +0200 netfilter: nf_tables: deactivate anonymous set from preparation phase Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2196159 Upstream Status: commit c1592a89942e9 CVE: CVE-2023-32233 Conflicts: everywhere Tested: nftables v0.8 shell/py tests A rule like ip saddr { 1.2.3.4, 5.6.7.8 } accept consists of multiple expressions and a set (the part in { }). This set only has a auto-assigned name "__set%d" hidden from userspace view, but its still internally referenceable this way. Such "anonymous" sets are owned by the rule that use it. Rule deletion auto-removes the set as well. Unfortunately userspace can craft a transaction that first deletes the rule and then attempts an operation on the anon set, such as deleting it or deleting an element from the set. Upstream patch uses existing delete/activate callbacks to disable the set in the new generation (the "preview" of the future ruleset). This makes such attempt at (re)using the set fail because the set won't be visible anymore. In RHEL7 we cannot mark the set as inactive in next generation because neither sets nor set elements have such a generation bit mask. This backport adds minimal bits from 408070d6ee3490 "netfilter: nf_tables: add nft_set_is_anonymous() helper" bb7b40aecbf778 "netfilter: nf_tables: bogus EBUSY in chain deletions" cd5125d8f51882 "netfilter: nf_tables: split set destruction in deactivate and destroy phase" for the necessary infrastructure to mark anon sets as "to be removed" from transaction preparation phase. Introduce an explicit "removed" bit flag that is set once the nft_lookup or dynset expression referencing an anonymous set gets scheduled for removal in the transaction phase. This can then be detected in a subsequent DELSETEM attempt and an error can be returned. commit c1592a89942e9678f7d9c8030efa777c0d57edab Author: Pablo Neira Ayuso Date: Tue May 2 10:25:24 2023 +0200 netfilter: nf_tables: deactivate anonymous set from preparation phase 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: Florian Westphal Signed-off-by: Ryan Sullivan --- net/netfilter/nf_tables_api.c | 97 ++++++++++++++++++++++++++++++++++- net/netfilter/nft_dynset.c | 26 ++++++++++ net/netfilter/nft_lookup.c | 26 ++++++++++ 3 files changed, 147 insertions(+), 2 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 44738b987690..feff3b92a617 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -20,6 +20,9 @@ #include #include #include +#include + +#define KLP_CVE_2023_32233 0x2022101200000111 static LIST_HEAD(nf_tables_expressions); @@ -260,6 +263,50 @@ static inline void nft_rule_clear(struct net *net, struct nft_rule *rule) rule->genmask &= ~nft_genmask_next(net); } +extern void nft_dynset_deactivate(const struct nft_ctx *ctx, + const struct nft_expr *expr); +extern void nft_dynset_activate(const struct nft_ctx *ctx, + const struct nft_expr *expr); +extern int klp_is_nft_dynset(const struct nft_expr *expr); + +extern void nft_lookup_deactivate(const struct nft_ctx *ctx, + const struct nft_expr *expr); +extern void nft_lookup_activate(const struct nft_ctx *ctx, + const struct nft_expr *expr); +extern int klp_is_nft_lookup(const struct nft_expr *expr); + +static void nft_rule_expr_activate(const struct nft_ctx *ctx, + struct nft_rule *rule) +{ + struct nft_expr *expr; + + expr = nft_expr_first(rule); + while (expr != nft_expr_last(rule) && expr->ops) { + if (klp_is_nft_dynset(expr)) + nft_dynset_activate(ctx, expr); + else if (klp_is_nft_lookup(expr)) + nft_lookup_activate(ctx, expr); + + expr = nft_expr_next(expr); + } +} + +static void nft_rule_expr_deactivate(const struct nft_ctx *ctx, + struct nft_rule *rule) +{ + struct nft_expr *expr; + + expr = nft_expr_first(rule); + while (expr != nft_expr_last(rule) && expr->ops) { + if (klp_is_nft_dynset(expr)) + nft_dynset_deactivate(ctx, expr); + else if (klp_is_nft_lookup(expr)) + nft_lookup_deactivate(ctx, expr); + + expr = nft_expr_next(expr); + } +} + static int nf_tables_delrule_deactivate(struct nft_ctx *ctx, struct nft_rule *rule) { @@ -301,6 +348,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); return 0; } @@ -2736,6 +2784,7 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk, unsigned char *udata; u16 udlen; int err; + u16 *klp_removed; if (nla[NFTA_SET_TABLE] == NULL || nla[NFTA_SET_NAME] == NULL || @@ -2861,10 +2910,16 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk, if (set == NULL) goto err1; + klp_removed = klp_shadow_alloc(set, KLP_CVE_2023_32233, + sizeof(*klp_removed), GFP_KERNEL, + NULL, NULL); + if(!klp_removed) + goto err2; + nla_strlcpy(name, nla[NFTA_SET_NAME], sizeof(set->name)); err = nf_tables_set_alloc_name(&ctx, set, name); if (err < 0) - goto err2; + goto klp_err; udata = NULL; if (udlen) { @@ -2889,7 +2944,7 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk, err = ops->init(set, &desc, nla); if (err < 0) - goto err2; + goto klp_err; err = nft_trans_set_add(&ctx, NFT_MSG_NEWSET, set); if (err < 0) @@ -2901,6 +2956,8 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk, err3: ops->destroy(set); +klp_err: + klp_shadow_free(set, KLP_CVE_2023_32233, NULL); err2: kfree(set); err1: @@ -2910,6 +2967,7 @@ err1: static void nft_set_destroy(struct nft_set *set) { + klp_shadow_free(set, KLP_CVE_2023_32233, NULL); set->ops->destroy(set); module_put(set->ops->owner); kfree(set); @@ -3013,6 +3071,34 @@ void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set, nf_tables_set_destroy(ctx, set); } +static inline bool nft_set_is_anonymous(const struct nft_set *set) +{ + return set->flags & NFT_SET_ANONYMOUS; +} + +void nf_tables_activate_set(const struct nft_ctx *ctx, struct nft_set *set) +{ + u16 *klp_removed; + + if (nft_set_is_anonymous(set)) { + klp_removed = klp_shadow_get(set, KLP_CVE_2023_32233); + if(klp_removed) + *klp_removed = 0; + } +} + +__attribute__((optimize("-fno-optimize-sibling-calls"))) +void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set, + struct nft_set_binding *binding) +{ + u16 *klp_removed; + klp_removed = klp_shadow_get(set, KLP_CVE_2023_32233); + if (nft_set_is_anonymous(set)) { + if(klp_removed) + *klp_removed = 1; + } +} + const struct nft_set_ext_type nft_set_ext_types[] = { [NFT_SET_EXT_KEY] = { .align = __alignof__(u32), @@ -3705,6 +3791,7 @@ static int nf_tables_delsetelem(struct net *net, struct sock *nlsk, struct nft_set *set; struct nft_ctx ctx; int rem, err = 0; + u16 *klp_removed; err = nft_ctx_init_from_elemattr(&ctx, net, skb, nlh, nla); if (err < 0) @@ -3716,6 +3803,10 @@ static int nf_tables_delsetelem(struct net *net, struct sock *nlsk, if (!list_empty(&set->bindings) && set->flags & NFT_SET_CONSTANT) return -EBUSY; + klp_removed = klp_shadow_get(set, KLP_CVE_2023_32233); + if (klp_removed && *klp_removed) + return -ENOENT; + if (nla[NFTA_SET_ELEM_LIST_ELEMENTS] == NULL) { struct nft_set_dump_args args = { .iter = { @@ -4152,11 +4243,13 @@ static int __nf_tables_abort(struct net *net) break; case NFT_MSG_NEWRULE: trans->ctx.chain->use--; + nft_rule_expr_deactivate(&trans->ctx, nft_trans_rule(trans)); list_del_rcu(&nft_trans_rule(trans)->list); break; case NFT_MSG_DELRULE: trans->ctx.chain->use++; nft_rule_clear(trans->ctx.net, nft_trans_rule(trans)); + nft_rule_expr_activate(&trans->ctx, nft_trans_rule(trans)); nft_trans_destroy(trans); break; case NFT_MSG_NEWSET: diff --git a/net/netfilter/nft_dynset.c b/net/netfilter/nft_dynset.c index 0cf187230050..4d6d3af26a5b 100644 --- a/net/netfilter/nft_dynset.c +++ b/net/netfilter/nft_dynset.c @@ -204,6 +204,32 @@ err1: return err; } +void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set, + struct nft_set_binding *binding); + +void nft_dynset_deactivate(const struct nft_ctx *ctx, + const struct nft_expr *expr) +{ + struct nft_dynset *priv = nft_expr_priv(expr); + + nf_tables_deactivate_set(ctx, priv->set, &priv->binding); +} + +void nf_tables_activate_set(const struct nft_ctx *ctx, struct nft_set *set); + +void nft_dynset_activate(const struct nft_ctx *ctx, + const struct nft_expr *expr) +{ + struct nft_dynset *priv = nft_expr_priv(expr); + + nf_tables_activate_set(ctx, priv->set); +} + +int klp_is_nft_dynset(const struct nft_expr *expr) +{ + return expr->ops->type == &nft_dynset_type; +} + static void nft_dynset_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) { diff --git a/net/netfilter/nft_lookup.c b/net/netfilter/nft_lookup.c index c9ce7d60cd73..ab98a5cb7128 100644 --- a/net/netfilter/nft_lookup.c +++ b/net/netfilter/nft_lookup.c @@ -125,6 +125,32 @@ static int nft_lookup_init(const struct nft_ctx *ctx, return 0; } +void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set, + struct nft_set_binding *binding); + +void nft_lookup_deactivate(const struct nft_ctx *ctx, + const struct nft_expr *expr) +{ + struct nft_lookup *priv = nft_expr_priv(expr); + + nf_tables_deactivate_set(ctx, priv->set, &priv->binding); +} + +void nf_tables_activate_set(const struct nft_ctx *ctx, struct nft_set *set); + +void nft_lookup_activate(const struct nft_ctx *ctx, + const struct nft_expr *expr) +{ + struct nft_lookup *priv = nft_expr_priv(expr); + + nf_tables_activate_set(ctx, priv->set); +} + +int klp_is_nft_lookup(const struct nft_expr *expr) +{ + return expr->ops->type == &nft_lookup_type; +} + static void nft_lookup_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) { -- 2.40.1