netfilter: nf_tables: don't allow to rename to already-pending name

Its possible to rename two chains to the same name in one
transaction:

nft add chain t c1
nft add chain t c2
nft 'rename chain t c1 c3;rename chain t c2 c3'

This creates two chains named 'c3'.

Appears to be harmless, both chains can still be deleted both
by name or handle, but, nevertheless, its a bug.

Walk transaction log and also compare vs. the pending renames.

Both chains can still be deleted, but nevertheless it is a bug as
we don't allow to create chains with identical names, so we should
prevent this from happening-by-rename too.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
This commit is contained in:
Florian Westphal 2018-07-17 07:17:56 +02:00 committed by Pablo Neira Ayuso
parent 9f8aac0be2
commit c6cc94df65
1 changed files with 29 additions and 13 deletions

View File

@ -1598,7 +1598,6 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
struct nft_base_chain *basechain; struct nft_base_chain *basechain;
struct nft_stats *stats = NULL; struct nft_stats *stats = NULL;
struct nft_chain_hook hook; struct nft_chain_hook hook;
const struct nlattr *name;
struct nf_hook_ops *ops; struct nf_hook_ops *ops;
struct nft_trans *trans; struct nft_trans *trans;
int err; int err;
@ -1646,12 +1645,11 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
return PTR_ERR(stats); return PTR_ERR(stats);
} }
err = -ENOMEM;
trans = nft_trans_alloc(ctx, NFT_MSG_NEWCHAIN, trans = nft_trans_alloc(ctx, NFT_MSG_NEWCHAIN,
sizeof(struct nft_trans_chain)); sizeof(struct nft_trans_chain));
if (trans == NULL) { if (trans == NULL)
free_percpu(stats); goto err;
return -ENOMEM;
}
nft_trans_chain_stats(trans) = stats; nft_trans_chain_stats(trans) = stats;
nft_trans_chain_update(trans) = true; nft_trans_chain_update(trans) = true;
@ -1661,19 +1659,37 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
else else
nft_trans_chain_policy(trans) = -1; nft_trans_chain_policy(trans) = -1;
name = nla[NFTA_CHAIN_NAME]; if (nla[NFTA_CHAIN_HANDLE] &&
if (nla[NFTA_CHAIN_HANDLE] && name) { nla[NFTA_CHAIN_NAME]) {
nft_trans_chain_name(trans) = struct nft_trans *tmp;
nla_strdup(name, GFP_KERNEL); char *name;
if (!nft_trans_chain_name(trans)) {
kfree(trans); err = -ENOMEM;
free_percpu(stats); name = nla_strdup(nla[NFTA_CHAIN_NAME], GFP_KERNEL);
return -ENOMEM; if (!name)
goto err;
err = -EEXIST;
list_for_each_entry(tmp, &ctx->net->nft.commit_list, list) {
if (tmp->msg_type == NFT_MSG_NEWCHAIN &&
tmp->ctx.table == table &&
nft_trans_chain_update(tmp) &&
nft_trans_chain_name(tmp) &&
strcmp(name, nft_trans_chain_name(tmp)) == 0) {
kfree(name);
goto err;
}
} }
nft_trans_chain_name(trans) = name;
} }
list_add_tail(&trans->list, &ctx->net->nft.commit_list); list_add_tail(&trans->list, &ctx->net->nft.commit_list);
return 0; return 0;
err:
free_percpu(stats);
kfree(trans);
return err;
} }
static int nf_tables_newchain(struct net *net, struct sock *nlsk, static int nf_tables_newchain(struct net *net, struct sock *nlsk,