From 7972a497548bcdfc7f3b6f24a1fd5e4b0e008fff Mon Sep 17 00:00:00 2001
From: Ryan Sullivan <rysulliv@redhat.com>
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 <fwestpha@redhat.com>
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 <pablo@netfilter.org>
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] <TASK>
[ 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 <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fwestpha@redhat.com>
Signed-off-by: Ryan Sullivan <rysulliv@redhat.com>
---
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 <net/netfilter/nf_tables.h>
#include <net/net_namespace.h>
#include <net/sock.h>
+#include <linux/livepatch.h>
+
+#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