Skip to content

Commit 515ad53

Browse files
Thadeu Lima de Souza Cascardoummakynes
Thadeu Lima de Souza Cascardo
authored andcommitted
netfilter: nf_tables: do not ignore genmask when looking up chain by id
When adding a rule to a chain referring to its ID, if that chain had been deleted on the same batch, the rule might end up referring to a deleted chain. This will lead to a WARNING like following: [ 33.098431] ------------[ cut here ]------------ [ 33.098678] WARNING: CPU: 5 PID: 69 at net/netfilter/nf_tables_api.c:2037 nf_tables_chain_destroy+0x23d/0x260 [ 33.099217] Modules linked in: [ 33.099388] CPU: 5 PID: 69 Comm: kworker/5:1 Not tainted 6.4.0+ #409 [ 33.099726] Workqueue: events nf_tables_trans_destroy_work [ 33.100018] RIP: 0010:nf_tables_chain_destroy+0x23d/0x260 [ 33.100306] Code: 8b 7c 24 68 e8 64 9c ed fe 4c 89 e7 e8 5c 9c ed fe 48 83 c4 08 5b 41 5c 41 5d 41 5e 41 5f 5d 31 c0 89 c6 89 c7 c3 cc cc cc cc <0f> 0b 48 83 c4 08 5b 41 5c 41 5d 41 5e 41 5f 5d 31 c0 89 c6 89 c7 [ 33.101271] RSP: 0018:ffffc900004ffc48 EFLAGS: 00010202 [ 33.101546] RAX: 0000000000000001 RBX: ffff888006fc0a28 RCX: 0000000000000000 [ 33.101920] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 [ 33.102649] RBP: ffffc900004ffc78 R08: 0000000000000000 R09: 0000000000000000 [ 33.103018] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8880135ef500 [ 33.103385] R13: 0000000000000000 R14: dead000000000122 R15: ffff888006fc0a10 [ 33.103762] FS: 0000000000000000(0000) GS:ffff888024c80000(0000) knlGS:0000000000000000 [ 33.104184] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 33.104493] CR2: 00007fe863b56a50 CR3: 00000000124b0001 CR4: 0000000000770ee0 [ 33.104872] PKRU: 55555554 [ 33.104999] Call Trace: [ 33.105113] <TASK> [ 33.105214] ? show_regs+0x72/0x90 [ 33.105371] ? __warn+0xa5/0x210 [ 33.105520] ? nf_tables_chain_destroy+0x23d/0x260 [ 33.105732] ? report_bug+0x1f2/0x200 [ 33.105902] ? handle_bug+0x46/0x90 [ 33.106546] ? exc_invalid_op+0x19/0x50 [ 33.106762] ? asm_exc_invalid_op+0x1b/0x20 [ 33.106995] ? nf_tables_chain_destroy+0x23d/0x260 [ 33.107249] ? nf_tables_chain_destroy+0x30/0x260 [ 33.107506] nf_tables_trans_destroy_work+0x669/0x680 [ 33.107782] ? mark_held_locks+0x28/0xa0 [ 33.107996] ? __pfx_nf_tables_trans_destroy_work+0x10/0x10 [ 33.108294] ? _raw_spin_unlock_irq+0x28/0x70 [ 33.108538] process_one_work+0x68c/0xb70 [ 33.108755] ? lock_acquire+0x17f/0x420 [ 33.108977] ? __pfx_process_one_work+0x10/0x10 [ 33.109218] ? do_raw_spin_lock+0x128/0x1d0 [ 33.109435] ? _raw_spin_lock_irq+0x71/0x80 [ 33.109634] worker_thread+0x2bd/0x700 [ 33.109817] ? __pfx_worker_thread+0x10/0x10 [ 33.110254] kthread+0x18b/0x1d0 [ 33.110410] ? __pfx_kthread+0x10/0x10 [ 33.110581] ret_from_fork+0x29/0x50 [ 33.110757] </TASK> [ 33.110866] irq event stamp: 1651 [ 33.111017] hardirqs last enabled at (1659): [<ffffffffa206a209>] __up_console_sem+0x79/0xa0 [ 33.111379] hardirqs last disabled at (1666): [<ffffffffa206a1ee>] __up_console_sem+0x5e/0xa0 [ 33.111740] softirqs last enabled at (1616): [<ffffffffa1f5d40e>] __irq_exit_rcu+0x9e/0xe0 [ 33.112094] softirqs last disabled at (1367): [<ffffffffa1f5d40e>] __irq_exit_rcu+0x9e/0xe0 [ 33.112453] ---[ end trace 0000000000000000 ]--- This is due to the nft_chain_lookup_byid ignoring the genmask. After this change, adding the new rule will fail as it will not find the chain. Fixes: 837830a ("netfilter: nf_tables: add NFTA_RULE_CHAIN_ID attribute") Cc: [email protected] Reported-by: Mingi Cho of Theori working with ZDI Signed-off-by: Thadeu Lima de Souza Cascardo <[email protected]> Reviewed-by: Florian Westphal <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]>
1 parent eaf9e71 commit 515ad53

File tree

1 file changed

+7
-4
lines changed

1 file changed

+7
-4
lines changed

net/netfilter/nf_tables_api.c

+7-4
Original file line numberDiff line numberDiff line change
@@ -2699,7 +2699,7 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
26992699

27002700
static struct nft_chain *nft_chain_lookup_byid(const struct net *net,
27012701
const struct nft_table *table,
2702-
const struct nlattr *nla)
2702+
const struct nlattr *nla, u8 genmask)
27032703
{
27042704
struct nftables_pernet *nft_net = nft_pernet(net);
27052705
u32 id = ntohl(nla_get_be32(nla));
@@ -2710,7 +2710,8 @@ static struct nft_chain *nft_chain_lookup_byid(const struct net *net,
27102710

27112711
if (trans->msg_type == NFT_MSG_NEWCHAIN &&
27122712
chain->table == table &&
2713-
id == nft_trans_chain_id(trans))
2713+
id == nft_trans_chain_id(trans) &&
2714+
nft_active_genmask(chain, genmask))
27142715
return chain;
27152716
}
27162717
return ERR_PTR(-ENOENT);
@@ -3814,7 +3815,8 @@ static int nf_tables_newrule(struct sk_buff *skb, const struct nfnl_info *info,
38143815
return -EOPNOTSUPP;
38153816

38163817
} else if (nla[NFTA_RULE_CHAIN_ID]) {
3817-
chain = nft_chain_lookup_byid(net, table, nla[NFTA_RULE_CHAIN_ID]);
3818+
chain = nft_chain_lookup_byid(net, table, nla[NFTA_RULE_CHAIN_ID],
3819+
genmask);
38183820
if (IS_ERR(chain)) {
38193821
NL_SET_BAD_ATTR(extack, nla[NFTA_RULE_CHAIN_ID]);
38203822
return PTR_ERR(chain);
@@ -10540,7 +10542,8 @@ static int nft_verdict_init(const struct nft_ctx *ctx, struct nft_data *data,
1054010542
genmask);
1054110543
} else if (tb[NFTA_VERDICT_CHAIN_ID]) {
1054210544
chain = nft_chain_lookup_byid(ctx->net, ctx->table,
10543-
tb[NFTA_VERDICT_CHAIN_ID]);
10545+
tb[NFTA_VERDICT_CHAIN_ID],
10546+
genmask);
1054410547
if (IS_ERR(chain))
1054510548
return PTR_ERR(chain);
1054610549
} else {

0 commit comments

Comments
 (0)