Skip to content

Commit 95b04bb

Browse files
gvrose8192gvrose8192
committed
netfilter: nf_tables: fix table flag updates
jira VUlN-597 subsystem-sync netfilter:nf_tables 4.18.0-553 commit-author Pablo Neira Ayuso <[email protected]> commit 179d9ba upstream-diff Again some cruft around an upstream commit that Red Hat did not take - using branch 8_10 as the source of truth for the commit. The dormant flag need to be updated from the preparation phase, otherwise, two consecutive requests to dorm a table in the same batch might try to remove the same hooks twice, resulting in the following warning: hook not found, pf 3 num 0 WARNING: CPU: 0 PID: 334 at net/netfilter/core.c:480 __nf_unregister_net_hook+0x1eb/0x610 net/netfilter/core.c:480 Modules linked in: CPU: 0 PID: 334 Comm: kworker/u4:5 Not tainted 5.12.0-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: netns cleanup_net RIP: 0010:__nf_unregister_net_hook+0x1eb/0x610 net/netfilter/core.c:480 This patch is a partial revert of 0ce7cf4 ("netfilter: nftables: update table flags from the commit phase") to restore the previous behaviour. However, there is still another problem: A batch containing a series of dorm-wakeup-dorm table and vice-versa also trigger the warning above since hook unregistration happens from the preparation phase, while hook registration occurs from the commit phase. To fix this problem, this patch adds two internal flags to annotate the original dormant flag status which are __NFT_TABLE_F_WAS_DORMANT and __NFT_TABLE_F_WAS_AWAKEN, to restore it from the abort path. The __NFT_TABLE_F_UPDATE bitmask allows to handle the dormant flag update with one single transaction. Reported-by: [email protected] Fixes: 0ce7cf4 ("netfilter: nftables: update table flags from the commit phase") Signed-off-by: Pablo Neira Ayuso <[email protected]> (cherry picked from commit 179d9ba) Signed-off-by: Greg Rose <[email protected]>
1 parent 878f6ba commit 95b04bb

File tree

2 files changed

+40
-25
lines changed

2 files changed

+40
-25
lines changed

‎include/net/netfilter/nf_tables.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1449,16 +1449,10 @@ struct nft_trans_chain {
14491449

14501450
struct nft_trans_table {
14511451
bool update;
1452-
u8 state;
1453-
u32 flags;
14541452
};
14551453

14561454
#define nft_trans_table_update(trans) \
14571455
(((struct nft_trans_table *)trans->data)->update)
1458-
#define nft_trans_table_state(trans) \
1459-
(((struct nft_trans_table *)trans->data)->state)
1460-
#define nft_trans_table_flags(trans) \
1461-
(((struct nft_trans_table *)trans->data)->flags)
14621456

14631457
struct nft_trans_elem {
14641458
struct nft_set *set;

‎net/netfilter/nf_tables_api.c

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,8 @@ static int nf_tables_fill_table_info(struct sk_buff *skb, struct net *net,
739739
nfmsg->res_id = htons(net->nft.base_seq & 0xffff);
740740

741741
if (nla_put_string(skb, NFTA_TABLE_NAME, table->name) ||
742-
nla_put_be32(skb, NFTA_TABLE_FLAGS, htonl(table->flags)) ||
742+
nla_put_be32(skb, NFTA_TABLE_FLAGS,
743+
htonl(table->flags & NFT_TABLE_F_MASK)) ||
743744
nla_put_be32(skb, NFTA_TABLE_USE, htonl(table->use)) ||
744745
nla_put_be64(skb, NFTA_TABLE_HANDLE, cpu_to_be64(table->handle),
745746
NFTA_TABLE_PAD))
@@ -942,20 +943,22 @@ static int nf_tables_table_enable(struct net *net, struct nft_table *table)
942943

943944
static void nf_tables_table_disable(struct net *net, struct nft_table *table)
944945
{
946+
table->flags &= ~NFT_TABLE_F_DORMANT;
945947
nft_table_disable(net, table, 0);
948+
table->flags |= NFT_TABLE_F_DORMANT;
946949
}
947950

948-
enum {
949-
NFT_TABLE_STATE_UNCHANGED = 0,
950-
NFT_TABLE_STATE_DORMANT,
951-
NFT_TABLE_STATE_WAKEUP
952-
};
951+
#define __NFT_TABLE_F_INTERNAL (NFT_TABLE_F_MASK + 1)
952+
#define __NFT_TABLE_F_WAS_DORMANT (__NFT_TABLE_F_INTERNAL << 0)
953+
#define __NFT_TABLE_F_WAS_AWAKEN (__NFT_TABLE_F_INTERNAL << 1)
954+
#define __NFT_TABLE_F_UPDATE (__NFT_TABLE_F_WAS_DORMANT | \
955+
__NFT_TABLE_F_WAS_AWAKEN)
953956

954957
static int nf_tables_updtable(struct nft_ctx *ctx)
955958
{
956959
struct nft_trans *trans;
957960
u32 flags;
958-
int ret = 0;
961+
int ret;
959962

960963
if (!ctx->nla[NFTA_TABLE_FLAGS])
961964
return 0;
@@ -974,21 +977,27 @@ static int nf_tables_updtable(struct nft_ctx *ctx)
974977

975978
if ((flags & NFT_TABLE_F_DORMANT) &&
976979
!(ctx->table->flags & NFT_TABLE_F_DORMANT)) {
977-
nft_trans_table_state(trans) = NFT_TABLE_STATE_DORMANT;
980+
ctx->table->flags |= NFT_TABLE_F_DORMANT;
981+
if (!(ctx->table->flags & __NFT_TABLE_F_UPDATE))
982+
ctx->table->flags |= __NFT_TABLE_F_WAS_AWAKEN;
978983
} else if (!(flags & NFT_TABLE_F_DORMANT) &&
979984
ctx->table->flags & NFT_TABLE_F_DORMANT) {
980-
ret = nf_tables_table_enable(ctx->net, ctx->table);
981-
if (ret >= 0)
982-
nft_trans_table_state(trans) = NFT_TABLE_STATE_WAKEUP;
985+
ctx->table->flags &= ~NFT_TABLE_F_DORMANT;
986+
if (!(ctx->table->flags & __NFT_TABLE_F_UPDATE)) {
987+
ret = nf_tables_table_enable(ctx->net, ctx->table);
988+
if (ret < 0)
989+
goto err_register_hooks;
990+
991+
ctx->table->flags |= __NFT_TABLE_F_WAS_DORMANT;
992+
}
983993
}
984-
if (ret < 0)
985-
goto err;
986994

987-
nft_trans_table_flags(trans) = flags;
988995
nft_trans_table_update(trans) = true;
989996
list_add_tail(&trans->list, &ctx->net->nft.commit_list);
997+
990998
return 0;
991-
err:
999+
1000+
err_register_hooks:
9921001
nft_trans_destroy(trans);
9931002
return ret;
9941003
}
@@ -8230,10 +8239,14 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
82308239
switch (trans->msg_type) {
82318240
case NFT_MSG_NEWTABLE:
82328241
if (nft_trans_table_update(trans)) {
8233-
if (nft_trans_table_state(trans) == NFT_TABLE_STATE_DORMANT)
8242+
if (!(trans->ctx.table->flags & __NFT_TABLE_F_UPDATE)) {
8243+
nft_trans_destroy(trans);
8244+
break;
8245+
}
8246+
if (trans->ctx.table->flags & NFT_TABLE_F_DORMANT)
82348247
nf_tables_table_disable(net, trans->ctx.table);
82358248

8236-
trans->ctx.table->flags = nft_trans_table_flags(trans);
8249+
trans->ctx.table->flags &= ~__NFT_TABLE_F_UPDATE;
82378250
} else {
82388251
nft_clear(net, trans->ctx.table);
82398252
}
@@ -8455,9 +8468,17 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
84558468
switch (trans->msg_type) {
84568469
case NFT_MSG_NEWTABLE:
84578470
if (nft_trans_table_update(trans)) {
8458-
if (nft_trans_table_state(trans) == NFT_TABLE_STATE_WAKEUP)
8471+
if (!(trans->ctx.table->flags & __NFT_TABLE_F_UPDATE)) {
8472+
nft_trans_destroy(trans);
8473+
break;
8474+
}
8475+
if (trans->ctx.table->flags & __NFT_TABLE_F_WAS_DORMANT) {
84598476
nf_tables_table_disable(net, trans->ctx.table);
8460-
8477+
trans->ctx.table->flags |= NFT_TABLE_F_DORMANT;
8478+
} else if (trans->ctx.table->flags & __NFT_TABLE_F_WAS_AWAKEN) {
8479+
trans->ctx.table->flags &= ~NFT_TABLE_F_DORMANT;
8480+
}
8481+
trans->ctx.table->flags &= ~__NFT_TABLE_F_UPDATE;
84618482
nft_trans_destroy(trans);
84628483
} else {
84638484
list_del_rcu(&trans->ctx.table->list);

0 commit comments

Comments
 (0)