Skip to content

Commit ddc653d

Browse files
gvrose8192gvrose8192
committed
netfilter: nf_tables: fix table flag updates
jira VUlN-597 subsystem-sync netfilter:nf_tables 4.18.0-511 commit-author Pablo Neira Ayuso <pablo@netfilter.org> 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: syzbot+7ad5cd1615f2d89c6e7e@syzkaller.appspotmail.com Fixes: 0ce7cf4 ("netfilter: nftables: update table flags from the commit phase") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> (cherry picked from commit 179d9ba) Signed-off-by: Greg Rose <g.v.rose@ciq.com>
1 parent eeff826 commit ddc653d

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
}
@@ -8231,10 +8240,14 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
82318240
switch (trans->msg_type) {
82328241
case NFT_MSG_NEWTABLE:
82338242
if (nft_trans_table_update(trans)) {
8234-
if (nft_trans_table_state(trans) == NFT_TABLE_STATE_DORMANT)
8243+
if (!(trans->ctx.table->flags & __NFT_TABLE_F_UPDATE)) {
8244+
nft_trans_destroy(trans);
8245+
break;
8246+
}
8247+
if (trans->ctx.table->flags & NFT_TABLE_F_DORMANT)
82358248
nf_tables_table_disable(net, trans->ctx.table);
82368249

8237-
trans->ctx.table->flags = nft_trans_table_flags(trans);
8250+
trans->ctx.table->flags &= ~__NFT_TABLE_F_UPDATE;
82388251
} else {
82398252
nft_clear(net, trans->ctx.table);
82408253
}
@@ -8456,9 +8469,17 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
84568469
switch (trans->msg_type) {
84578470
case NFT_MSG_NEWTABLE:
84588471
if (nft_trans_table_update(trans)) {
8459-
if (nft_trans_table_state(trans) == NFT_TABLE_STATE_WAKEUP)
8472+
if (!(trans->ctx.table->flags & __NFT_TABLE_F_UPDATE)) {
8473+
nft_trans_destroy(trans);
8474+
break;
8475+
}
8476+
if (trans->ctx.table->flags & __NFT_TABLE_F_WAS_DORMANT) {
84608477
nf_tables_table_disable(net, trans->ctx.table);
8461-
8478+
trans->ctx.table->flags |= NFT_TABLE_F_DORMANT;
8479+
} else if (trans->ctx.table->flags & __NFT_TABLE_F_WAS_AWAKEN) {
8480+
trans->ctx.table->flags &= ~NFT_TABLE_F_DORMANT;
8481+
}
8482+
trans->ctx.table->flags &= ~__NFT_TABLE_F_UPDATE;
84628483
nft_trans_destroy(trans);
84638484
} else {
84648485
list_del_rcu(&trans->ctx.table->list);

0 commit comments

Comments
 (0)