From patchwork Thu Jan 9 18:42:31 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick McHardy X-Patchwork-Id: 308919 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id B94CE2C0089 for ; Fri, 10 Jan 2014 05:43:26 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756833AbaAISnQ (ORCPT ); Thu, 9 Jan 2014 13:43:16 -0500 Received: from stinky.trash.net ([213.144.137.162]:51677 "EHLO stinky.trash.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751137AbaAISnN (ORCPT ); Thu, 9 Jan 2014 13:43:13 -0500 Received: from stinky.trash.net (unknown [127.0.0.1]) by stinky.trash.net (Postfix) with ESMTP id BB3BB9D2DE; Thu, 9 Jan 2014 19:43:10 +0100 (MET) From: Patrick McHardy To: pablo@netfilter.org Cc: netfilter-devel@vger.kernel.org Subject: [PATCH 01/13] netfilter: nf_tables: split chain policy validation from actually setting it Date: Thu, 9 Jan 2014 18:42:31 +0000 Message-Id: <1389292963-4089-2-git-send-email-kaber@trash.net> X-Mailer: git-send-email 1.8.4.2 In-Reply-To: <1389292963-4089-1-git-send-email-kaber@trash.net> References: <1389292963-4089-1-git-send-email-kaber@trash.net> Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Currently nf_tables_newchain() atomicity is broken because of having validation of some netlink attributes performed after changing attributes of the chain. The chain policy is (currently) fine, but split it up as preparation for the following fixes and to avoid future mistakes. Signed-off-by: Patrick McHardy --- net/netfilter/nf_tables_api.c | 56 ++++++++++++++++--------------------------- 1 file changed, 20 insertions(+), 36 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 572d88d..30fad4f 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -760,22 +760,6 @@ err: return err; } -static int -nf_tables_chain_policy(struct nft_base_chain *chain, const struct nlattr *attr) -{ - switch (ntohl(nla_get_be32(attr))) { - case NF_DROP: - chain->policy = NF_DROP; - break; - case NF_ACCEPT: - chain->policy = NF_ACCEPT; - break; - default: - return -EINVAL; - } - return 0; -} - static const struct nla_policy nft_counter_policy[NFTA_COUNTER_MAX + 1] = { [NFTA_COUNTER_PACKETS] = { .type = NLA_U64 }, [NFTA_COUNTER_BYTES] = { .type = NLA_U64 }, @@ -834,6 +818,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb, struct nlattr *ha[NFTA_HOOK_MAX + 1]; struct net *net = sock_net(skb->sk); int family = nfmsg->nfgen_family; + u8 policy = NF_ACCEPT; u64 handle = 0; unsigned int i; int err; @@ -869,6 +854,22 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb, } } + if (nla[NFTA_CHAIN_POLICY]) { + if ((chain != NULL && + !(chain->flags & NFT_BASE_CHAIN)) || + nla[NFTA_CHAIN_HOOK] == NULL) + return -EOPNOTSUPP; + + policy = nla_get_be32(nla[NFTA_CHAIN_POLICY]); + switch (policy) { + case NF_DROP: + case NF_ACCEPT: + break; + default: + return -EINVAL; + } + } + if (chain != NULL) { if (nlh->nlmsg_flags & NLM_F_EXCL) return -EEXIST; @@ -879,15 +880,8 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb, !IS_ERR(nf_tables_chain_lookup(table, nla[NFTA_CHAIN_NAME]))) return -EEXIST; - if (nla[NFTA_CHAIN_POLICY]) { - if (!(chain->flags & NFT_BASE_CHAIN)) - return -EOPNOTSUPP; - - err = nf_tables_chain_policy(nft_base_chain(chain), - nla[NFTA_CHAIN_POLICY]); - if (err < 0) - return err; - } + if (nla[NFTA_CHAIN_POLICY]) + nft_base_chain(chain)->policy = policy; if (nla[NFTA_CHAIN_COUNTERS]) { if (!(chain->flags & NFT_BASE_CHAIN)) @@ -958,17 +952,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb, } chain->flags |= NFT_BASE_CHAIN; - - if (nla[NFTA_CHAIN_POLICY]) { - err = nf_tables_chain_policy(basechain, - nla[NFTA_CHAIN_POLICY]); - if (err < 0) { - free_percpu(basechain->stats); - kfree(basechain); - return err; - } - } else - basechain->policy = NF_ACCEPT; + basechain->policy = policy; if (nla[NFTA_CHAIN_COUNTERS]) { err = nf_tables_counters(basechain,