From patchwork Thu Sep 28 16:52:37 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 1840932 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=nwl.cc header.i=@nwl.cc header.a=rsa-sha256 header.s=mail2022 header.b=SpqauHIe; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=2620:137:e000::1:20; helo=out1.vger.email; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=patchwork.ozlabs.org) Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by legolas.ozlabs.org (Postfix) with ESMTP id 4RxKKh4mD3z1yp8 for ; Fri, 29 Sep 2023 02:52:56 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231589AbjI1Qwy (ORCPT ); Thu, 28 Sep 2023 12:52:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59892 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231377AbjI1Qww (ORCPT ); Thu, 28 Sep 2023 12:52:52 -0400 Received: from orbyte.nwl.cc (orbyte.nwl.cc [IPv6:2001:41d0:e:133a::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E69081AC for ; Thu, 28 Sep 2023 09:52:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nwl.cc; s=mail2022; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=ef/C5H+KN92sEYt6WZASjZ2f4kc/BJ3sBgTYh7VlXos=; b=SpqauHIeSyZ/fxjRKzzFncmALS FK0WAHjYePSp3USMf6+NpfubiJc7AT1R03zOj0IZimCVflhK4ZFIQjF/VPWvO7tQ5ccLgCrMgXxSP wn8JCpRzt2V5BVN4MeljS9QmZvJT65xceParg5yl6sz7KfZxLMHT522q9p3/7cVnD6JNntGR5oWzk pUa8FjL8qTQgtqdPX9qpThYC89P+D2RmfLi+txDUfPhqTGCAgEKj9QIqg3lTzyGl8gf5VZhR8NSZ+ rXVXuXGfSVvRCZRAAE3l4Dg0Jv7N3EYrBwNlCjUcFQWGITcHLU3yHOCMjUTRwiEfcj0NvFWZouZNd p4KBFbFQ==; Received: from localhost ([::1] helo=xic) by orbyte.nwl.cc with esmtp (Exim 4.94.2) (envelope-from ) id 1qluFt-0004wX-9y; Thu, 28 Sep 2023 18:52:49 +0200 From: Phil Sutter To: Pablo Neira Ayuso Cc: Florian Westphal , netfilter-devel@vger.kernel.org Subject: [nf PATCH v2 1/8] netfilter: nf_tables: Don't allocate nft_rule_dump_ctx Date: Thu, 28 Sep 2023 18:52:37 +0200 Message-ID: <20230928165244.7168-2-phil@nwl.cc> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230928165244.7168-1-phil@nwl.cc> References: <20230928165244.7168-1-phil@nwl.cc> MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Eliminate the direct use of netlink_callback::args when dumping rules by casting nft_rule_dump_ctx over netlink_callback::ctx as suggested in the struct's comment. The value for 's_idx' has to be stored inside nft_rule_dump_ctx now and make it hold the 'reset' boolean as well. Note how this patch removes the zeroing of netlink_callback::args[1-5] - none of the rule dump callbacks seem to make use of them. Signed-off-by: Phil Sutter --- Changes since v1: - Fix description: 'idx' is not moved into the struct --- net/netfilter/nf_tables_api.c | 81 ++++++++++++++--------------------- 1 file changed, 32 insertions(+), 49 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 4356189360fb8..511508407867d 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -3441,20 +3441,21 @@ static void audit_log_rule_reset(const struct nft_table *table, } struct nft_rule_dump_ctx { + unsigned int s_idx; char *table; char *chain; + bool reset; }; static int __nf_tables_dump_rules(struct sk_buff *skb, unsigned int *idx, struct netlink_callback *cb, const struct nft_table *table, - const struct nft_chain *chain, - bool reset) + const struct nft_chain *chain) { + struct nft_rule_dump_ctx *ctx = (void *)cb->ctx; struct net *net = sock_net(skb->sk); const struct nft_rule *rule, *prule; - unsigned int s_idx = cb->args[0]; unsigned int entries = 0; int ret = 0; u64 handle; @@ -3463,12 +3464,9 @@ static int __nf_tables_dump_rules(struct sk_buff *skb, list_for_each_entry_rcu(rule, &chain->rules, list) { if (!nft_is_active(net, rule)) goto cont_skip; - if (*idx < s_idx) + if (*idx < ctx->s_idx) goto cont; - if (*idx > s_idx) { - memset(&cb->args[1], 0, - sizeof(cb->args) - sizeof(cb->args[0])); - } + if (prule) handle = prule->handle; else @@ -3479,7 +3477,7 @@ static int __nf_tables_dump_rules(struct sk_buff *skb, NFT_MSG_NEWRULE, NLM_F_MULTI | NLM_F_APPEND, table->family, - table, chain, rule, handle, reset) < 0) { + table, chain, rule, handle, ctx->reset) < 0) { ret = 1; break; } @@ -3491,7 +3489,7 @@ static int __nf_tables_dump_rules(struct sk_buff *skb, (*idx)++; } - if (reset && entries) + if (ctx->reset && entries) audit_log_rule_reset(table, cb->seq, entries); return ret; @@ -3501,17 +3499,13 @@ static int nf_tables_dump_rules(struct sk_buff *skb, struct netlink_callback *cb) { const struct nfgenmsg *nfmsg = nlmsg_data(cb->nlh); - const struct nft_rule_dump_ctx *ctx = cb->data; + struct nft_rule_dump_ctx *ctx = (void *)cb->ctx; struct nft_table *table; const struct nft_chain *chain; unsigned int idx = 0; struct net *net = sock_net(skb->sk); int family = nfmsg->nfgen_family; struct nftables_pernet *nft_net; - bool reset = false; - - if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) == NFT_MSG_GETRULE_RESET) - reset = true; rcu_read_lock(); nft_net = nft_pernet(net); @@ -3521,10 +3515,10 @@ static int nf_tables_dump_rules(struct sk_buff *skb, if (family != NFPROTO_UNSPEC && family != table->family) continue; - if (ctx && ctx->table && strcmp(ctx->table, table->name) != 0) + if (ctx->table && strcmp(ctx->table, table->name) != 0) continue; - if (ctx && ctx->table && ctx->chain) { + if (ctx->table && ctx->chain) { struct rhlist_head *list, *tmp; list = rhltable_lookup(&table->chains_ht, ctx->chain, @@ -3536,7 +3530,7 @@ static int nf_tables_dump_rules(struct sk_buff *skb, if (!nft_is_active(net, chain)) continue; __nf_tables_dump_rules(skb, &idx, - cb, table, chain, reset); + cb, table, chain); break; } goto done; @@ -3544,62 +3538,51 @@ static int nf_tables_dump_rules(struct sk_buff *skb, list_for_each_entry_rcu(chain, &table->chains, list) { if (__nf_tables_dump_rules(skb, &idx, - cb, table, chain, reset)) + cb, table, chain)) goto done; } - if (ctx && ctx->table) + if (ctx->table) break; } done: rcu_read_unlock(); - cb->args[0] = idx; + ctx->s_idx = idx; return skb->len; } static int nf_tables_dump_rules_start(struct netlink_callback *cb) { + struct nft_rule_dump_ctx *ctx = (void *)cb->ctx; const struct nlattr * const *nla = cb->data; - struct nft_rule_dump_ctx *ctx = NULL; - if (nla[NFTA_RULE_TABLE] || nla[NFTA_RULE_CHAIN]) { - ctx = kzalloc(sizeof(*ctx), GFP_ATOMIC); - if (!ctx) - return -ENOMEM; + BUILD_BUG_ON(sizeof(*ctx) > sizeof(cb->ctx)); - if (nla[NFTA_RULE_TABLE]) { - ctx->table = nla_strdup(nla[NFTA_RULE_TABLE], - GFP_ATOMIC); - if (!ctx->table) { - kfree(ctx); - return -ENOMEM; - } - } - if (nla[NFTA_RULE_CHAIN]) { - ctx->chain = nla_strdup(nla[NFTA_RULE_CHAIN], - GFP_ATOMIC); - if (!ctx->chain) { - kfree(ctx->table); - kfree(ctx); - return -ENOMEM; - } + if (nla[NFTA_RULE_TABLE]) { + ctx->table = nla_strdup(nla[NFTA_RULE_TABLE], GFP_ATOMIC); + if (!ctx->table) + return -ENOMEM; + } + if (nla[NFTA_RULE_CHAIN]) { + ctx->chain = nla_strdup(nla[NFTA_RULE_CHAIN], GFP_ATOMIC); + if (!ctx->chain) { + kfree(ctx->table); + return -ENOMEM; } } + if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) == NFT_MSG_GETRULE_RESET) + ctx->reset = true; - cb->data = ctx; return 0; } static int nf_tables_dump_rules_done(struct netlink_callback *cb) { - struct nft_rule_dump_ctx *ctx = cb->data; + struct nft_rule_dump_ctx *ctx = (void *)cb->ctx; - if (ctx) { - kfree(ctx->table); - kfree(ctx->chain); - kfree(ctx); - } + kfree(ctx->table); + kfree(ctx->chain); return 0; } From patchwork Thu Sep 28 16:52:38 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 1840935 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=nwl.cc header.i=@nwl.cc header.a=rsa-sha256 header.s=mail2022 header.b=ChXUyuW/; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=2620:137:e000::1:20; helo=out1.vger.email; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=patchwork.ozlabs.org) Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by legolas.ozlabs.org (Postfix) with ESMTP id 4RxKKj6wLDz1yqY for ; Fri, 29 Sep 2023 02:52:57 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231614AbjI1Qwz (ORCPT ); Thu, 28 Sep 2023 12:52:55 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59916 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231508AbjI1Qwx (ORCPT ); Thu, 28 Sep 2023 12:52:53 -0400 Received: from orbyte.nwl.cc (orbyte.nwl.cc [IPv6:2001:41d0:e:133a::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3BC681AE for ; Thu, 28 Sep 2023 09:52:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nwl.cc; s=mail2022; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=pAeyjne6puAHF+bYIyQn9h53gqNPO22hPBRP93lRe5U=; b=ChXUyuW/ViXn5fBZGxt6cXiotl dLjPAbWlZxHf31Dj8eWdJ+C7l0/t+5QNZa82dIPZaHFq1Uh8J1Pd356DDwEtwrNhRmTyUVz5KIzY3 SHQ3mH6oti5g1jaxh+mQPI00jihdbxWuEHMcCD6E/YGC/bBqHOvgmMeeQaRQ+mpmxsN1+Oq1gTkKS WEqCWp5HB2vRgDg8KyJ65YbwBj45g6eEu+WIcYpUILhRqCsNpF6z/+uBfobH1Lk6zYl6TvagbXBYk VjDgmCi+3XCnV0dn1Y1U6K4KSZiJgMpCSVWa6QX8N2G61gfjKvhxGUOsVyfo1QLpBoAybRbF+shWd 6+r1I0Fg==; Received: from localhost ([::1] helo=xic) by orbyte.nwl.cc with esmtp (Exim 4.94.2) (envelope-from ) id 1qluFt-0004wc-K8; Thu, 28 Sep 2023 18:52:49 +0200 From: Phil Sutter To: Pablo Neira Ayuso Cc: Florian Westphal , netfilter-devel@vger.kernel.org Subject: [nf PATCH v2 2/8] netfilter: nf_tables: Introduce nf_tables_getrule_single() Date: Thu, 28 Sep 2023 18:52:38 +0200 Message-ID: <20230928165244.7168-3-phil@nwl.cc> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230928165244.7168-1-phil@nwl.cc> References: <20230928165244.7168-1-phil@nwl.cc> MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Outsource the reply skb preparation for non-dump getrule requests into a distinct function. Prep work for rule reset locking. Signed-off-by: Phil Sutter --- Changes since v1: - New patch --- net/netfilter/nf_tables_api.c | 88 +++++++++++++++++++++++------------ 1 file changed, 59 insertions(+), 29 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 511508407867d..641ae9bde46fa 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -3587,8 +3587,9 @@ static int nf_tables_dump_rules_done(struct netlink_callback *cb) } /* called with rcu_read_lock held */ -static int nf_tables_getrule(struct sk_buff *skb, const struct nfnl_info *info, - const struct nlattr * const nla[]) +static struct sk_buff * +nf_tables_getrule_single(u32 portid, const struct nfnl_info *info, + const struct nlattr * const nla[], bool reset) { struct netlink_ext_ack *extack = info->extack; u8 genmask = nft_genmask_cur(info->net); @@ -3598,60 +3599,89 @@ static int nf_tables_getrule(struct sk_buff *skb, const struct nfnl_info *info, struct net *net = info->net; struct nft_table *table; struct sk_buff *skb2; - bool reset = false; int err; - if (info->nlh->nlmsg_flags & NLM_F_DUMP) { - struct netlink_dump_control c = { - .start= nf_tables_dump_rules_start, - .dump = nf_tables_dump_rules, - .done = nf_tables_dump_rules_done, - .module = THIS_MODULE, - .data = (void *)nla, - }; - - return nft_netlink_dump_start_rcu(info->sk, skb, info->nlh, &c); - } - table = nft_table_lookup(net, nla[NFTA_RULE_TABLE], family, genmask, 0); if (IS_ERR(table)) { NL_SET_BAD_ATTR(extack, nla[NFTA_RULE_TABLE]); - return PTR_ERR(table); + return ERR_CAST(table); } chain = nft_chain_lookup(net, table, nla[NFTA_RULE_CHAIN], genmask); if (IS_ERR(chain)) { NL_SET_BAD_ATTR(extack, nla[NFTA_RULE_CHAIN]); - return PTR_ERR(chain); + return ERR_CAST(chain); } rule = nft_rule_lookup(chain, nla[NFTA_RULE_HANDLE]); if (IS_ERR(rule)) { NL_SET_BAD_ATTR(extack, nla[NFTA_RULE_HANDLE]); - return PTR_ERR(rule); + return ERR_CAST(rule); } skb2 = alloc_skb(NLMSG_GOODSIZE, GFP_ATOMIC); if (!skb2) + return ERR_PTR(-ENOMEM); + + err = nf_tables_fill_rule_info(skb2, net, portid, + info->nlh->nlmsg_seq, NFT_MSG_NEWRULE, 0, + family, table, chain, rule, 0, reset); + if (err < 0) { + kfree_skb(skb2); + return ERR_PTR(err); + } + + return skb2; +} + +static int nf_tables_getrule(struct sk_buff *skb, const struct nfnl_info *info, + const struct nlattr * const nla[]) +{ + u8 family = info->nfmsg->nfgen_family; + u32 portid = NETLINK_CB(skb).portid; + struct net *net = info->net; + struct sk_buff *skb2; + bool reset = false; + char *tablename; + + if (info->nlh->nlmsg_flags & NLM_F_DUMP) { + struct netlink_dump_control c = { + .start= nf_tables_dump_rules_start, + .dump = nf_tables_dump_rules, + .done = nf_tables_dump_rules_done, + .module = THIS_MODULE, + .data = (void *)nla, + }; + + return nft_netlink_dump_start_rcu(info->sk, skb, info->nlh, &c); + } + + if (!nla[NFTA_RULE_TABLE]) + return -EINVAL; + + tablename = nla_strdup(nla[NFTA_RULE_TABLE], GFP_ATOMIC); + if (!tablename) return -ENOMEM; if (NFNL_MSG_TYPE(info->nlh->nlmsg_type) == NFT_MSG_GETRULE_RESET) reset = true; - err = nf_tables_fill_rule_info(skb2, net, NETLINK_CB(skb).portid, - info->nlh->nlmsg_seq, NFT_MSG_NEWRULE, 0, - family, table, chain, rule, 0, reset); - if (err < 0) - goto err_fill_rule_info; + skb2 = nf_tables_getrule_single(portid, info, nla, reset); + if (IS_ERR(skb2)) { + kfree(tablename); + return PTR_ERR(skb2); + } - if (reset) - audit_log_rule_reset(table, nft_pernet(net)->base_seq, 1); + if (reset) { + char *buf = kasprintf(GFP_ATOMIC, "%s:%u", + tablename, nft_pernet(net)->base_seq); - return nfnetlink_unicast(skb2, net, NETLINK_CB(skb).portid); + audit_log_nfcfg(buf, family, 1, + AUDIT_NFT_OP_RULE_RESET, GFP_ATOMIC); + kfree(buf); + } -err_fill_rule_info: - kfree_skb(skb2); - return err; + return nfnetlink_unicast(skb2, net, portid); } void nf_tables_rule_destroy(const struct nft_ctx *ctx, struct nft_rule *rule) From patchwork Thu Sep 28 16:52:39 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 1840933 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=nwl.cc header.i=@nwl.cc header.a=rsa-sha256 header.s=mail2022 header.b=MhsqQaKJ; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=2620:137:e000::1:20; helo=out1.vger.email; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=patchwork.ozlabs.org) Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by legolas.ozlabs.org (Postfix) with ESMTP id 4RxKKj0hBCz1ypN for ; Fri, 29 Sep 2023 02:52:57 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231332AbjI1Qwy (ORCPT ); Thu, 28 Sep 2023 12:52:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59912 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231571AbjI1Qwx (ORCPT ); Thu, 28 Sep 2023 12:52:53 -0400 Received: from orbyte.nwl.cc (orbyte.nwl.cc [IPv6:2001:41d0:e:133a::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0E2A31A5 for ; Thu, 28 Sep 2023 09:52:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nwl.cc; s=mail2022; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=L3w8WxheC71ObxLFiK2CxEL1VfPapPr4wm0c4p74b1g=; b=MhsqQaKJ9yPnvHWrNFu+YmL62/ TLK43kp2ijEV2e5LCq7ELJH4t0oD6095yunj7CwOQ5K8uq3pyrEKRKsfvELE0uF3ZCy0g70s8yoMX S3vroOUBmIhyESLxDn4ihP/J3jdU7MHNox0dt/LmOfuS7oF184dVdw6rkX7kNEyQ6k70Sf8vWMrZm K7GFvzgi7GHaYi/o4D8ic1rs1VWi8X46ajIo4dqnU6R3/EnalsLZ8cVUi6nB57sbAdTjqLJT1zk2/ hB3uVKS3sJZUS+SYXCd3NUjujt5r/tBWiYi+B8ldAD0rCEdvZQla3MkX/B0wGP593ccOCfNaZrUZH eC05ggtw==; Received: from localhost ([::1] helo=xic) by orbyte.nwl.cc with esmtp (Exim 4.94.2) (envelope-from ) id 1qluFs-0004wH-Au; Thu, 28 Sep 2023 18:52:48 +0200 From: Phil Sutter To: Pablo Neira Ayuso Cc: Florian Westphal , netfilter-devel@vger.kernel.org Subject: [nf PATCH v2 3/8] netfilter: nf_tables: Add locking for NFT_MSG_GETRULE_RESET requests Date: Thu, 28 Sep 2023 18:52:39 +0200 Message-ID: <20230928165244.7168-4-phil@nwl.cc> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230928165244.7168-1-phil@nwl.cc> References: <20230928165244.7168-1-phil@nwl.cc> MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Rule reset is not concurrency-safe per-se, so multiple CPUs may reset the same rule at the same time. At least counter and quota expressions will suffer from value underruns in this case. Prevent this by introducing dedicated locking callbacks for nfnetlink and the asynchronous dump handling to serialize access. Signed-off-by: Phil Sutter --- Changes since v1: - commit_mutex instead of dedicated spinlock --- net/netfilter/nf_tables_api.c | 85 ++++++++++++++++++++++++++--------- 1 file changed, 65 insertions(+), 20 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 641ae9bde46fa..2e0402a4078cf 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -3552,6 +3552,19 @@ static int nf_tables_dump_rules(struct sk_buff *skb, return skb->len; } +static int nf_tables_dumpreset_rules(struct sk_buff *skb, + struct netlink_callback *cb) +{ + struct nftables_pernet *nft_net = nft_pernet(sock_net(skb->sk)); + int ret; + + mutex_lock(&nft_net->commit_mutex); + ret = nf_tables_dump_rules(skb, cb); + mutex_unlock(&nft_net->commit_mutex); + + return ret; +} + static int nf_tables_dump_rules_start(struct netlink_callback *cb) { struct nft_rule_dump_ctx *ctx = (void *)cb->ctx; @@ -3571,12 +3584,18 @@ static int nf_tables_dump_rules_start(struct netlink_callback *cb) return -ENOMEM; } } - if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) == NFT_MSG_GETRULE_RESET) - ctx->reset = true; - return 0; } +static int nf_tables_dumpreset_rules_start(struct netlink_callback *cb) +{ + struct nft_rule_dump_ctx *ctx = (void *)cb->ctx; + + ctx->reset = true; + + return nf_tables_dump_rules_start(cb); +} + static int nf_tables_dump_rules_done(struct netlink_callback *cb) { struct nft_rule_dump_ctx *ctx = (void *)cb->ctx; @@ -3637,12 +3656,8 @@ nf_tables_getrule_single(u32 portid, const struct nfnl_info *info, static int nf_tables_getrule(struct sk_buff *skb, const struct nfnl_info *info, const struct nlattr * const nla[]) { - u8 family = info->nfmsg->nfgen_family; u32 portid = NETLINK_CB(skb).portid; - struct net *net = info->net; struct sk_buff *skb2; - bool reset = false; - char *tablename; if (info->nlh->nlmsg_flags & NLM_F_DUMP) { struct netlink_dump_control c = { @@ -3656,6 +3671,35 @@ static int nf_tables_getrule(struct sk_buff *skb, const struct nfnl_info *info, return nft_netlink_dump_start_rcu(info->sk, skb, info->nlh, &c); } + skb2 = nf_tables_getrule_single(portid, info, nla, false); + if (IS_ERR(skb2)) + return PTR_ERR(skb2); + + return nfnetlink_unicast(skb2, info->net, portid); +} + +static int nf_tables_getrule_reset(struct sk_buff *skb, + const struct nfnl_info *info, + const struct nlattr * const nla[]) +{ + struct nftables_pernet *nft_net = nft_pernet(info->net); + u8 family = info->nfmsg->nfgen_family; + u32 portid = NETLINK_CB(skb).portid; + char *tablename, *buf; + struct sk_buff *skb2; + + if (info->nlh->nlmsg_flags & NLM_F_DUMP) { + struct netlink_dump_control c = { + .start= nf_tables_dumpreset_rules_start, + .dump = nf_tables_dumpreset_rules, + .done = nf_tables_dump_rules_done, + .module = THIS_MODULE, + .data = (void *)nla, + }; + + return nft_netlink_dump_start_rcu(info->sk, skb, info->nlh, &c); + } + if (!nla[NFTA_RULE_TABLE]) return -EINVAL; @@ -3663,25 +3707,26 @@ static int nf_tables_getrule(struct sk_buff *skb, const struct nfnl_info *info, if (!tablename) return -ENOMEM; - if (NFNL_MSG_TYPE(info->nlh->nlmsg_type) == NFT_MSG_GETRULE_RESET) - reset = true; + if (!try_module_get(THIS_MODULE)) + return -EINVAL; + rcu_read_unlock(); + mutex_lock(&nft_net->commit_mutex); + skb2 = nf_tables_getrule_single(portid, info, nla, true); + mutex_unlock(&nft_net->commit_mutex); + rcu_read_lock(); + module_put(THIS_MODULE); - skb2 = nf_tables_getrule_single(portid, info, nla, reset); if (IS_ERR(skb2)) { kfree(tablename); return PTR_ERR(skb2); } - if (reset) { - char *buf = kasprintf(GFP_ATOMIC, "%s:%u", - tablename, nft_pernet(net)->base_seq); - - audit_log_nfcfg(buf, family, 1, - AUDIT_NFT_OP_RULE_RESET, GFP_ATOMIC); - kfree(buf); - } + buf = kasprintf(GFP_ATOMIC, "%s:%u", tablename, nft_net->base_seq); + audit_log_nfcfg(buf, family, 1, AUDIT_NFT_OP_RULE_RESET, GFP_ATOMIC); + kfree(buf); + kfree(tablename); - return nfnetlink_unicast(skb2, net, portid); + return nfnetlink_unicast(skb2, info->net, portid); } void nf_tables_rule_destroy(const struct nft_ctx *ctx, struct nft_rule *rule) @@ -8980,7 +9025,7 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = { .policy = nft_rule_policy, }, [NFT_MSG_GETRULE_RESET] = { - .call = nf_tables_getrule, + .call = nf_tables_getrule_reset, .type = NFNL_CB_RCU, .attr_count = NFTA_RULE_MAX, .policy = nft_rule_policy, From patchwork Thu Sep 28 16:52:40 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 1840937 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=nwl.cc header.i=@nwl.cc header.a=rsa-sha256 header.s=mail2022 header.b=eWw2PVmS; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=2620:137:e000::1:20; helo=out1.vger.email; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=patchwork.ozlabs.org) Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by legolas.ozlabs.org (Postfix) with ESMTP id 4RxKKk4Mcjz1yp0 for ; Fri, 29 Sep 2023 02:52:58 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231607AbjI1Qw4 (ORCPT ); Thu, 28 Sep 2023 12:52:56 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59912 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231666AbjI1Qwx (ORCPT ); Thu, 28 Sep 2023 12:52:53 -0400 Received: from orbyte.nwl.cc (orbyte.nwl.cc [IPv6:2001:41d0:e:133a::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 26D9D1A4 for ; Thu, 28 Sep 2023 09:52:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nwl.cc; s=mail2022; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=p+HcC33exid3LS9lsF4rVtkERFRPdSSZMIi0xwXDYJs=; b=eWw2PVmS7oeK0CxhQUD8fV0U94 cpgGOW3w+DAhQrHgmv1F1Y0hbIKu1d9roq0AWnN//3VCqHvyGglC3mrEq2T4PBVBk4GKRFtyAu2mS 8Ueqy9X9tNXH/Nm8pMhduILygVosyhlL7hyYVrYiL1aiqpSQqAM7XfsPIywUtWTDm215sG+Z6++pw YT1X/IAI092BCP7RrqP0tkJc2AzSoTJ69eKW++hULFGt3QZ4EukP6LNfx7SuGz19uxxhfnOgLBXGN 6EG1HVp1y/THkHoRVgvMdU498zozZ3934vFR+gQNmqEe8HQ7eYSEJCqmwB0JysIWlUyFy36HjdR9N SnK+iX2A==; Received: from localhost ([::1] helo=xic) by orbyte.nwl.cc with esmtp (Exim 4.94.2) (envelope-from ) id 1qluFu-0004ww-H1; Thu, 28 Sep 2023 18:52:50 +0200 From: Phil Sutter To: Pablo Neira Ayuso Cc: Florian Westphal , netfilter-devel@vger.kernel.org Subject: [nf PATCH v2 4/8] netfilter: nf_tables: Introduce struct nft_obj_dump_ctx Date: Thu, 28 Sep 2023 18:52:40 +0200 Message-ID: <20230928165244.7168-5-phil@nwl.cc> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230928165244.7168-1-phil@nwl.cc> References: <20230928165244.7168-1-phil@nwl.cc> MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Convert struct nft_obj_filter into a dump context data structure equivalent to nft_rule_dump_ctx. Make it reside inside struct netlink_callback::ctx scratch area so there's no need to allocate and free it. Also carry the 'reset' boolean instead of determining this upon each call to nf_tables_dump_obj() by inspecting the nlmsg_type. Signed-off-by: Phil Sutter --- Changes since v1: - None --- net/netfilter/nf_tables_api.c | 66 ++++++++++++++--------------------- 1 file changed, 26 insertions(+), 40 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 2e0402a4078cf..67ee0d09cb844 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -7673,25 +7673,23 @@ static int nf_tables_fill_obj_info(struct sk_buff *skb, struct net *net, return -1; } -struct nft_obj_filter { +struct nft_obj_dump_ctx { + unsigned int s_idx; char *table; u32 type; + bool reset; }; static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb) { const struct nfgenmsg *nfmsg = nlmsg_data(cb->nlh); - const struct nft_table *table; - unsigned int idx = 0, s_idx = cb->args[0]; - struct nft_obj_filter *filter = cb->data; + struct nft_obj_dump_ctx *ctx = (void *)cb->ctx; struct net *net = sock_net(skb->sk); int family = nfmsg->nfgen_family; struct nftables_pernet *nft_net; + const struct nft_table *table; struct nft_object *obj; - bool reset = false; - - if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) == NFT_MSG_GETOBJ_RESET) - reset = true; + unsigned int idx = 0; rcu_read_lock(); nft_net = nft_pernet(net); @@ -7704,19 +7702,14 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb) list_for_each_entry_rcu(obj, &table->objects, list) { if (!nft_is_active(net, obj)) goto cont; - if (idx < s_idx) + if (idx < ctx->s_idx) goto cont; - if (idx > s_idx) - memset(&cb->args[1], 0, - sizeof(cb->args) - sizeof(cb->args[0])); - if (filter && filter->table && - strcmp(filter->table, table->name)) + if (ctx->table && strcmp(ctx->table, table->name)) goto cont; - if (filter && - filter->type != NFT_OBJECT_UNSPEC && - obj->ops->type->type != filter->type) + if (ctx->type != NFT_OBJECT_UNSPEC && + obj->ops->type->type != ctx->type) goto cont; - if (reset) { + if (ctx->reset) { char *buf = kasprintf(GFP_ATOMIC, "%s:%u", table->name, @@ -7735,7 +7728,7 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb) NFT_MSG_NEWOBJ, NLM_F_MULTI | NLM_F_APPEND, table->family, table, - obj, reset) < 0) + obj, ctx->reset) < 0) goto done; nl_dump_check_consistent(cb, nlmsg_hdr(skb)); @@ -7746,44 +7739,37 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb) done: rcu_read_unlock(); - cb->args[0] = idx; + ctx->s_idx = idx; return skb->len; } static int nf_tables_dump_obj_start(struct netlink_callback *cb) { + struct nft_obj_dump_ctx *ctx = (void *)cb->ctx; const struct nlattr * const *nla = cb->data; - struct nft_obj_filter *filter = NULL; - if (nla[NFTA_OBJ_TABLE] || nla[NFTA_OBJ_TYPE]) { - filter = kzalloc(sizeof(*filter), GFP_ATOMIC); - if (!filter) + BUILD_BUG_ON(sizeof(*ctx) > sizeof(cb->ctx)); + + if (nla[NFTA_OBJ_TABLE]) { + ctx->table = nla_strdup(nla[NFTA_OBJ_TABLE], GFP_ATOMIC); + if (!ctx->table) return -ENOMEM; + } - if (nla[NFTA_OBJ_TABLE]) { - filter->table = nla_strdup(nla[NFTA_OBJ_TABLE], GFP_ATOMIC); - if (!filter->table) { - kfree(filter); - return -ENOMEM; - } - } + if (nla[NFTA_OBJ_TYPE]) + ctx->type = ntohl(nla_get_be32(nla[NFTA_OBJ_TYPE])); - if (nla[NFTA_OBJ_TYPE]) - filter->type = ntohl(nla_get_be32(nla[NFTA_OBJ_TYPE])); - } + if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) == NFT_MSG_GETOBJ_RESET) + ctx->reset = true; - cb->data = filter; return 0; } static int nf_tables_dump_obj_done(struct netlink_callback *cb) { - struct nft_obj_filter *filter = cb->data; + struct nft_obj_dump_ctx *ctx = (void *)cb->ctx; - if (filter) { - kfree(filter->table); - kfree(filter); - } + kfree(ctx->table); return 0; } From patchwork Thu Sep 28 16:52:41 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 1840930 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=nwl.cc header.i=@nwl.cc header.a=rsa-sha256 header.s=mail2022 header.b=TlaSl/p/; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=2620:137:e000::1:20; helo=out1.vger.email; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=patchwork.ozlabs.org) Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by legolas.ozlabs.org (Postfix) with ESMTP id 4RxKKg5d1yz1ynX for ; Fri, 29 Sep 2023 02:52:55 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231654AbjI1Qwx (ORCPT ); Thu, 28 Sep 2023 12:52:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59878 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229581AbjI1Qww (ORCPT ); Thu, 28 Sep 2023 12:52:52 -0400 Received: from orbyte.nwl.cc (orbyte.nwl.cc [IPv6:2001:41d0:e:133a::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 411ED99 for ; Thu, 28 Sep 2023 09:52:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nwl.cc; s=mail2022; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=7p65C5HEHDh8/5Ysc4OS8xzqNUEiobXCkyB0iXI1wJ8=; b=TlaSl/p/rUih3phrzjGKUwsfRn WCrTlNOaKJ8mkMtARuImtODZpS+X21R40roA7x9dYyxkBcTJ3Rdsv7tjIJxd/PS9llLaSi95RHhfg E1nTa8uz81+haA2T2BtO+22uzf6mSZOzFij6XB0HXbsfYOjObd3PGk32l4jSXS1qNeIrz4c0B5yN2 7mE5lCYwlqdKdKz1F8nXp+l25gP41oJ+lR/+7b/uUDawMXhJtFyjvEB6lJhc8EHCazHboSXBcnPaQ ghvgjoiRd923n3uWF2F7sK2JGc9jvLoUPktOpgiXCmqmpgw1kqoqfdBzoFJLPEX27s6oR3WjgeTbT LpYkhyng==; Received: from localhost ([::1] helo=xic) by orbyte.nwl.cc with esmtp (Exim 4.94.2) (envelope-from ) id 1qluFs-0004wN-L6; Thu, 28 Sep 2023 18:52:48 +0200 From: Phil Sutter To: Pablo Neira Ayuso Cc: Florian Westphal , netfilter-devel@vger.kernel.org Subject: [nf PATCH v2 5/8] netfilter: nf_tables: Introduce nf_tables_getobj_single Date: Thu, 28 Sep 2023 18:52:41 +0200 Message-ID: <20230928165244.7168-6-phil@nwl.cc> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230928165244.7168-1-phil@nwl.cc> References: <20230928165244.7168-1-phil@nwl.cc> MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Outsource the reply skb preparation for non-dump getrule requests into a distinct function. Prep work for object reset locking. Signed-off-by: Phil Sutter --- Changes since v1: - New patch --- net/netfilter/nf_tables_api.c | 66 +++++++++++++++++++++++++++-------- 1 file changed, 51 insertions(+), 15 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 67ee0d09cb844..eee149ea98b41 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -7775,19 +7775,65 @@ static int nf_tables_dump_obj_done(struct netlink_callback *cb) } /* called with rcu_read_lock held */ +static struct sk_buff * +nf_tables_getobj_single(u32 portid, const struct nfnl_info *info, + const struct nlattr * const nla[], bool reset) +{ + struct netlink_ext_ack *extack = info->extack; + u8 genmask = nft_genmask_cur(info->net); + u8 family = info->nfmsg->nfgen_family; + const struct nft_table *table; + struct net *net = info->net; + struct nft_object *obj; + struct sk_buff *skb2; + u32 objtype; + int err; + + if (!nla[NFTA_OBJ_NAME] || + !nla[NFTA_OBJ_TYPE]) + return ERR_PTR(-EINVAL); + + table = nft_table_lookup(net, nla[NFTA_OBJ_TABLE], family, genmask, 0); + if (IS_ERR(table)) { + NL_SET_BAD_ATTR(extack, nla[NFTA_OBJ_TABLE]); + return ERR_CAST(table); + } + + objtype = ntohl(nla_get_be32(nla[NFTA_OBJ_TYPE])); + obj = nft_obj_lookup(net, table, nla[NFTA_OBJ_NAME], objtype, genmask); + if (IS_ERR(obj)) { + NL_SET_BAD_ATTR(extack, nla[NFTA_OBJ_NAME]); + return ERR_CAST(obj); + } + + skb2 = alloc_skb(NLMSG_GOODSIZE, GFP_ATOMIC); + if (!skb2) + return ERR_PTR(-ENOMEM); + + err = nf_tables_fill_obj_info(skb2, net, portid, + info->nlh->nlmsg_seq, NFT_MSG_NEWOBJ, 0, + family, table, obj, reset); + if (err < 0) { + kfree_skb(skb2); + return ERR_PTR(err); + } + + return skb2; +} + static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info, const struct nlattr * const nla[]) { struct netlink_ext_ack *extack = info->extack; u8 genmask = nft_genmask_cur(info->net); u8 family = info->nfmsg->nfgen_family; + u32 portid = NETLINK_CB(skb).portid; const struct nft_table *table; struct net *net = info->net; struct nft_object *obj; struct sk_buff *skb2; bool reset = false; u32 objtype; - int err; if (info->nlh->nlmsg_flags & NLM_F_DUMP) { struct netlink_dump_control c = { @@ -7818,10 +7864,6 @@ static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info, return PTR_ERR(obj); } - skb2 = alloc_skb(NLMSG_GOODSIZE, GFP_ATOMIC); - if (!skb2) - return -ENOMEM; - if (NFNL_MSG_TYPE(info->nlh->nlmsg_type) == NFT_MSG_GETOBJ_RESET) reset = true; @@ -7840,17 +7882,11 @@ static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info, kfree(buf); } - err = nf_tables_fill_obj_info(skb2, net, NETLINK_CB(skb).portid, - info->nlh->nlmsg_seq, NFT_MSG_NEWOBJ, 0, - family, table, obj, reset); - if (err < 0) - goto err_fill_obj_info; - - return nfnetlink_unicast(skb2, net, NETLINK_CB(skb).portid); + skb2 = nf_tables_getobj_single(portid, info, nla, reset); + if (IS_ERR(skb2)) + return PTR_ERR(skb2); -err_fill_obj_info: - kfree_skb(skb2); - return err; + return nfnetlink_unicast(skb2, net, portid); } static void nft_obj_destroy(const struct nft_ctx *ctx, struct nft_object *obj) From patchwork Thu Sep 28 16:52:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 1840938 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=nwl.cc header.i=@nwl.cc header.a=rsa-sha256 header.s=mail2022 header.b=E8FHTIGA; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=2620:137:e000::1:20; helo=out1.vger.email; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=patchwork.ozlabs.org) Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by legolas.ozlabs.org (Postfix) with ESMTP id 4RxKKl0XRvz26jS for ; Fri, 29 Sep 2023 02:52:59 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231508AbjI1Qw5 (ORCPT ); Thu, 28 Sep 2023 12:52:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59952 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231488AbjI1Qwy (ORCPT ); Thu, 28 Sep 2023 12:52:54 -0400 Received: from orbyte.nwl.cc (orbyte.nwl.cc [IPv6:2001:41d0:e:133a::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7B0C71B3 for ; Thu, 28 Sep 2023 09:52:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nwl.cc; s=mail2022; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=ctU64Xgr1fCsxBOhbdNN7jmazWrOJGp5UfjqZkbAE0I=; b=E8FHTIGA0OLcJuPlF7rPXaSElz dDHVDTLDRgtQGYV62NR39GFviDo7XBQEW8hrTmrl+lpJV/NozeSDqJuioNNhYwPz6aw143tBj56li zFdNBSRujfFxE/yRRWsJuyfbucQJRxBmkbpHOw+h0IaJvp9b2XPYDPJxRuDG2ov9h8KfEAgEhkMhz /NerJTWJEso+aVwqkxE/bdTKGBgsY1BNvyCUCPSi1WZVOCMW6/iLBZpooWNM+f5NJ279Av0t5kiLY GvDMzNT+0S/51ORpu3zEzX2MPuqd5w5hcJ6nF4mYU2SeZacF6MhgIU1ybrj3tIf/u1brwR4g53hNH THns00kg==; Received: from localhost ([::1] helo=xic) by orbyte.nwl.cc with esmtp (Exim 4.94.2) (envelope-from ) id 1qluFu-0004x2-RU; Thu, 28 Sep 2023 18:52:50 +0200 From: Phil Sutter To: Pablo Neira Ayuso Cc: Florian Westphal , netfilter-devel@vger.kernel.org Subject: [nf PATCH v2 6/8] netfilter: nf_tables: Add locking for NFT_MSG_GETOBJ_RESET requests Date: Thu, 28 Sep 2023 18:52:42 +0200 Message-ID: <20230928165244.7168-7-phil@nwl.cc> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230928165244.7168-1-phil@nwl.cc> References: <20230928165244.7168-1-phil@nwl.cc> MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Objects' dump callbacks are not concurrency-safe per-se with reset bit set. If two CPUs perform a reset at the same time, at least counter and quota objects suffer from value underrun. Prevent this by introducing dedicated locking callbacks for nfnetlink and the asynchronous dump handling to serialize access. Signed-off-by: Phil Sutter --- Changes since v1: - Proper commit description - commit_mutex instead of dedicated spinlock --- net/netfilter/nf_tables_api.c | 91 +++++++++++++++++++++++++---------- 1 file changed, 66 insertions(+), 25 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index eee149ea98b41..f154fcc341421 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -7743,6 +7743,19 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb) return skb->len; } +static int nf_tables_dumpreset_obj(struct sk_buff *skb, + struct netlink_callback *cb) +{ + struct nftables_pernet *nft_net = nft_pernet(sock_net(skb->sk)); + int ret; + + mutex_lock(&nft_net->commit_mutex); + ret = nf_tables_dump_obj(skb, cb); + mutex_unlock(&nft_net->commit_mutex); + + return ret; +} + static int nf_tables_dump_obj_start(struct netlink_callback *cb) { struct nft_obj_dump_ctx *ctx = (void *)cb->ctx; @@ -7759,12 +7772,18 @@ static int nf_tables_dump_obj_start(struct netlink_callback *cb) if (nla[NFTA_OBJ_TYPE]) ctx->type = ntohl(nla_get_be32(nla[NFTA_OBJ_TYPE])); - if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) == NFT_MSG_GETOBJ_RESET) - ctx->reset = true; - return 0; } +static int nf_tables_dumpreset_obj_start(struct netlink_callback *cb) +{ + struct nft_obj_dump_ctx *ctx = (void *)cb->ctx; + + ctx->reset = true; + + return nf_tables_dump_obj_start(cb); +} + static int nf_tables_dump_obj_done(struct netlink_callback *cb) { struct nft_obj_dump_ctx *ctx = (void *)cb->ctx; @@ -7824,6 +7843,33 @@ nf_tables_getobj_single(u32 portid, const struct nfnl_info *info, static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info, const struct nlattr * const nla[]) { + u32 portid = NETLINK_CB(skb).portid; + struct sk_buff *skb2; + + if (info->nlh->nlmsg_flags & NLM_F_DUMP) { + struct netlink_dump_control c = { + .start = nf_tables_dump_obj_start, + .dump = nf_tables_dump_obj, + .done = nf_tables_dump_obj_done, + .module = THIS_MODULE, + .data = (void *)nla, + }; + + return nft_netlink_dump_start_rcu(info->sk, skb, info->nlh, &c); + } + + skb2 = nf_tables_getobj_single(portid, info, nla, false); + if (IS_ERR(skb2)) + return PTR_ERR(skb2); + + return nfnetlink_unicast(skb2, info->net, portid); +} + +static int nf_tables_getobj_reset(struct sk_buff *skb, + const struct nfnl_info *info, + const struct nlattr * const nla[]) +{ + struct nftables_pernet *nft_net = nft_pernet(info->net); struct netlink_ext_ack *extack = info->extack; u8 genmask = nft_genmask_cur(info->net); u8 family = info->nfmsg->nfgen_family; @@ -7832,13 +7878,13 @@ static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info, struct net *net = info->net; struct nft_object *obj; struct sk_buff *skb2; - bool reset = false; u32 objtype; + char *buf; if (info->nlh->nlmsg_flags & NLM_F_DUMP) { struct netlink_dump_control c = { - .start = nf_tables_dump_obj_start, - .dump = nf_tables_dump_obj, + .start = nf_tables_dumpreset_obj_start, + .dump = nf_tables_dumpreset_obj, .done = nf_tables_dump_obj_done, .module = THIS_MODULE, .data = (void *)nla, @@ -7864,28 +7910,23 @@ static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info, return PTR_ERR(obj); } - if (NFNL_MSG_TYPE(info->nlh->nlmsg_type) == NFT_MSG_GETOBJ_RESET) - reset = true; - - if (reset) { - const struct nftables_pernet *nft_net; - char *buf; - - nft_net = nft_pernet(net); - buf = kasprintf(GFP_ATOMIC, "%s:%u", table->name, nft_net->base_seq); - - audit_log_nfcfg(buf, - family, - obj->handle, - AUDIT_NFT_OP_OBJ_RESET, - GFP_ATOMIC); - kfree(buf); - } + if (!try_module_get(THIS_MODULE)) + return -EINVAL; + rcu_read_unlock(); + mutex_lock(&nft_net->commit_mutex); + skb2 = nf_tables_getobj_single(portid, info, nla, true); + mutex_unlock(&nft_net->commit_mutex); + rcu_read_lock(); + module_put(THIS_MODULE); - skb2 = nf_tables_getobj_single(portid, info, nla, reset); if (IS_ERR(skb2)) return PTR_ERR(skb2); + buf = kasprintf(GFP_ATOMIC, "%s:%u", table->name, nft_net->base_seq); + audit_log_nfcfg(buf, family, obj->handle, + AUDIT_NFT_OP_OBJ_RESET, GFP_ATOMIC); + kfree(buf); + return nfnetlink_unicast(skb2, net, portid); } @@ -9147,7 +9188,7 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = { .policy = nft_obj_policy, }, [NFT_MSG_GETOBJ_RESET] = { - .call = nf_tables_getobj, + .call = nf_tables_getobj_reset, .type = NFNL_CB_RCU, .attr_count = NFTA_OBJ_MAX, .policy = nft_obj_policy, From patchwork Thu Sep 28 16:52:43 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 1840934 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=nwl.cc header.i=@nwl.cc header.a=rsa-sha256 header.s=mail2022 header.b=f16rdvEE; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=2620:137:e000::1:20; helo=out1.vger.email; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=patchwork.ozlabs.org) Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by legolas.ozlabs.org (Postfix) with ESMTP id 4RxKKj3dy9z1yqJ for ; Fri, 29 Sep 2023 02:52:57 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231571AbjI1Qwz (ORCPT ); Thu, 28 Sep 2023 12:52:55 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59944 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231614AbjI1Qwx (ORCPT ); Thu, 28 Sep 2023 12:52:53 -0400 Received: from orbyte.nwl.cc (orbyte.nwl.cc [IPv6:2001:41d0:e:133a::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 883661B0 for ; Thu, 28 Sep 2023 09:52:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nwl.cc; s=mail2022; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=hdDAVBctpyZ/Tqgx4SblgX2duCs87eBSi8oTygKn9A0=; b=f16rdvEExxSAHMYuW/Mb5uwedQ vrx+TwTkmoMqAEXWlM2bsw8Zy2opFE0DuK2duetOJ7PIW0XskSq7jYTSBPWMFU8JGvySY13GAIbbk ljnaKPjr0aS8oWPJy9Si2rOshylEZcFcdAzG21TojFedYsXP42qOKWYiz2HQEf92Iyy7JEVMqTw+6 PXoGvHX90cBjh0sQLTY5pyDDtumegDfsFAn4Bq7PrB3N4b+KmFxMJAneYej8TCYjHesMDchNsmgBD KtazxgpDFB5qjw7mIiqOQNzRLaC73pvCyUBvKrX6h2RxTzwlUtsK2F43rkXYVTqN4rPBLJqezPZDm qZPvTKrw==; Received: from localhost ([::1] helo=xic) by orbyte.nwl.cc with esmtp (Exim 4.94.2) (envelope-from ) id 1qluFt-0004wh-Tt; Thu, 28 Sep 2023 18:52:49 +0200 From: Phil Sutter To: Pablo Neira Ayuso Cc: Florian Westphal , netfilter-devel@vger.kernel.org Subject: [nf PATCH v2 7/8] netfilter: nf_tables: Pass reset bit in nft_set_dump_ctx Date: Thu, 28 Sep 2023 18:52:43 +0200 Message-ID: <20230928165244.7168-8-phil@nwl.cc> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230928165244.7168-1-phil@nwl.cc> References: <20230928165244.7168-1-phil@nwl.cc> MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Relieve the dump callback from having to check nlmsg_type upon each call. Prep work for set element reset locking. Signed-off-by: Phil Sutter --- Changes since v1: - New patch --- net/netfilter/nf_tables_api.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index f154fcc341421..1491d4c65fed9 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -5731,6 +5731,7 @@ static void audit_log_nft_set_reset(const struct nft_table *table, struct nft_set_dump_ctx { const struct nft_set *set; struct nft_ctx ctx; + bool reset; }; static int nft_set_catchall_dump(struct net *net, struct sk_buff *skb, @@ -5770,7 +5771,6 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb) bool set_found = false; struct nlmsghdr *nlh; struct nlattr *nest; - bool reset = false; u32 portid, seq; int event; @@ -5818,12 +5818,9 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb) if (nest == NULL) goto nla_put_failure; - if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) == NFT_MSG_GETSETELEM_RESET) - reset = true; - args.cb = cb; args.skb = skb; - args.reset = reset; + args.reset = dump_ctx->reset; args.iter.genmask = nft_genmask_cur(net); args.iter.skip = cb->args[0]; args.iter.count = 0; @@ -5833,11 +5830,11 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb) if (!args.iter.err && args.iter.count == cb->args[0]) args.iter.err = nft_set_catchall_dump(net, skb, set, - reset, cb->seq); + dump_ctx->reset, cb->seq); nla_nest_end(skb, nest); nlmsg_end(skb, nlh); - if (reset && args.iter.count > args.iter.skip) + if (dump_ctx->reset && args.iter.count > args.iter.skip) audit_log_nft_set_reset(table, cb->seq, args.iter.count - args.iter.skip); @@ -6088,6 +6085,9 @@ static int nf_tables_getsetelem(struct sk_buff *skb, nft_ctx_init(&ctx, net, skb, info->nlh, family, table, NULL, nla); + if (NFNL_MSG_TYPE(info->nlh->nlmsg_type) == NFT_MSG_GETSETELEM_RESET) + reset = true; + if (info->nlh->nlmsg_flags & NLM_F_DUMP) { struct netlink_dump_control c = { .start = nf_tables_dump_set_start, @@ -6098,6 +6098,7 @@ static int nf_tables_getsetelem(struct sk_buff *skb, struct nft_set_dump_ctx dump_ctx = { .set = set, .ctx = ctx, + .reset = reset, }; c.data = &dump_ctx; @@ -6107,9 +6108,6 @@ static int nf_tables_getsetelem(struct sk_buff *skb, if (!nla[NFTA_SET_ELEM_LIST_ELEMENTS]) return -EINVAL; - if (NFNL_MSG_TYPE(info->nlh->nlmsg_type) == NFT_MSG_GETSETELEM_RESET) - reset = true; - nla_for_each_nested(attr, nla[NFTA_SET_ELEM_LIST_ELEMENTS], rem) { err = nft_get_set_elem(&ctx, set, attr, reset); if (err < 0) { From patchwork Thu Sep 28 16:52:44 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 1840931 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=nwl.cc header.i=@nwl.cc header.a=rsa-sha256 header.s=mail2022 header.b=QDcARQI5; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=2620:137:e000::1:20; helo=out1.vger.email; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=patchwork.ozlabs.org) Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by legolas.ozlabs.org (Postfix) with ESMTP id 4RxKKh1W0kz1yp0 for ; Fri, 29 Sep 2023 02:52:56 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229581AbjI1Qwx (ORCPT ); Thu, 28 Sep 2023 12:52:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59896 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231332AbjI1Qww (ORCPT ); Thu, 28 Sep 2023 12:52:52 -0400 Received: from orbyte.nwl.cc (orbyte.nwl.cc [IPv6:2001:41d0:e:133a::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 955BB1A7 for ; Thu, 28 Sep 2023 09:52:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nwl.cc; s=mail2022; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=lCMyq+V0pj+Z4iIbB0McLZnhrPHr56X3xmOZ1zzURYI=; b=QDcARQI53BxOJ+LmeytwexQg/f AL+0dY5AQ+ggs5KmeZ7LmZ85EKsNf2j537plHSDEDsUYToM+xVqhuoVil/iMNjGyAiRk7xQ+SlQIJ Ab58oZAp+BaWgb+T16s5UAXjOUS7ApJAmguass8BvdA+sIF17iBaax3u11XHzogqWIYQu572wi+ul cEOIFqkWSKINecYRU08nEVLSoxpVN7BVGI/GWZtgjOokUu+vtQPxoSW8OjSrTih3sTYaGwPWMxnoy iXYGc3odgNZ0pUtRSQjittCfB10+G/EU0JQMOzQBwfvTuCY/2frdfq+LecrAEbFRzrjOmWqKCyQxK +982taQg==; Received: from localhost ([::1] helo=xic) by orbyte.nwl.cc with esmtp (Exim 4.94.2) (envelope-from ) id 1qluFs-0004wS-VC; Thu, 28 Sep 2023 18:52:49 +0200 From: Phil Sutter To: Pablo Neira Ayuso Cc: Florian Westphal , netfilter-devel@vger.kernel.org Subject: [nf PATCH v2 8/8] netfilter: nf_tables: Add locking for NFT_MSG_GETSETELEM_RESET requests Date: Thu, 28 Sep 2023 18:52:44 +0200 Message-ID: <20230928165244.7168-9-phil@nwl.cc> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230928165244.7168-1-phil@nwl.cc> References: <20230928165244.7168-1-phil@nwl.cc> MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Set expressions' dump callbacks are not concurrency-safe per-se with reset bit set. If two CPUs reset the same element at the same time, values may underrun at least with element-attached counters and quotas. Prevent this by introducing dedicated callbacks for nfnetlink and the asynchronous dump handling to serialize access. Signed-off-by: Phil Sutter --- Changes since v1: - Improved commit description - Unrelated chunk moved into a previous patch --- net/netfilter/nf_tables_api.c | 105 +++++++++++++++++++++++++++++----- 1 file changed, 91 insertions(+), 14 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 1491d4c65fed9..a855b43fe72db 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -5834,10 +5834,6 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb) nla_nest_end(skb, nest); nlmsg_end(skb, nlh); - if (dump_ctx->reset && args.iter.count > args.iter.skip) - audit_log_nft_set_reset(table, cb->seq, - args.iter.count - args.iter.skip); - rcu_read_unlock(); if (args.iter.err && args.iter.err != -EMSGSIZE) @@ -5853,6 +5849,24 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb) return -ENOSPC; } +static int nf_tables_dumpreset_set(struct sk_buff *skb, + struct netlink_callback *cb) +{ + struct nftables_pernet *nft_net = nft_pernet(sock_net(skb->sk)); + struct nft_set_dump_ctx *dump_ctx = cb->data; + int ret, skip = cb->args[0]; + + mutex_lock(&nft_net->commit_mutex); + ret = nf_tables_dump_set(skb, cb); + mutex_unlock(&nft_net->commit_mutex); + + if (cb->args[0] > skip) + audit_log_nft_set_reset(dump_ctx->ctx.table, cb->seq, + cb->args[0] - skip); + + return ret; +} + static int nf_tables_dump_set_start(struct netlink_callback *cb) { struct nft_set_dump_ctx *dump_ctx = cb->data; @@ -6070,7 +6084,6 @@ static int nf_tables_getsetelem(struct sk_buff *skb, struct nft_set *set; struct nlattr *attr; struct nft_ctx ctx; - bool reset = false; table = nft_table_lookup(net, nla[NFTA_SET_ELEM_LIST_TABLE], family, genmask, 0); @@ -6085,9 +6098,6 @@ static int nf_tables_getsetelem(struct sk_buff *skb, nft_ctx_init(&ctx, net, skb, info->nlh, family, table, NULL, nla); - if (NFNL_MSG_TYPE(info->nlh->nlmsg_type) == NFT_MSG_GETSETELEM_RESET) - reset = true; - if (info->nlh->nlmsg_flags & NLM_F_DUMP) { struct netlink_dump_control c = { .start = nf_tables_dump_set_start, @@ -6098,7 +6108,67 @@ static int nf_tables_getsetelem(struct sk_buff *skb, struct nft_set_dump_ctx dump_ctx = { .set = set, .ctx = ctx, - .reset = reset, + .reset = false, + }; + + c.data = &dump_ctx; + return nft_netlink_dump_start_rcu(info->sk, skb, info->nlh, &c); + } + + if (!nla[NFTA_SET_ELEM_LIST_ELEMENTS]) + return -EINVAL; + + nla_for_each_nested(attr, nla[NFTA_SET_ELEM_LIST_ELEMENTS], rem) { + err = nft_get_set_elem(&ctx, set, attr, false); + if (err < 0) { + NL_SET_BAD_ATTR(extack, attr); + break; + } + nelems++; + } + + return err; +} + +static int nf_tables_getsetelem_reset(struct sk_buff *skb, + const struct nfnl_info *info, + const struct nlattr * const nla[]) +{ + struct nftables_pernet *nft_net = nft_pernet(info->net); + struct netlink_ext_ack *extack = info->extack; + u8 genmask = nft_genmask_cur(info->net); + u8 family = info->nfmsg->nfgen_family; + int rem, err = 0, nelems = 0; + struct net *net = info->net; + struct nft_table *table; + struct nft_set *set; + struct nlattr *attr; + struct nft_ctx ctx; + + table = nft_table_lookup(net, nla[NFTA_SET_ELEM_LIST_TABLE], family, + genmask, 0); + if (IS_ERR(table)) { + NL_SET_BAD_ATTR(extack, nla[NFTA_SET_ELEM_LIST_TABLE]); + return PTR_ERR(table); + } + + set = nft_set_lookup(table, nla[NFTA_SET_ELEM_LIST_SET], genmask); + if (IS_ERR(set)) + return PTR_ERR(set); + + nft_ctx_init(&ctx, net, skb, info->nlh, family, table, NULL, nla); + + if (info->nlh->nlmsg_flags & NLM_F_DUMP) { + struct netlink_dump_control c = { + .start = nf_tables_dump_set_start, + .dump = nf_tables_dumpreset_set, + .done = nf_tables_dump_set_done, + .module = THIS_MODULE, + }; + struct nft_set_dump_ctx dump_ctx = { + .set = set, + .ctx = ctx, + .reset = true, }; c.data = &dump_ctx; @@ -6108,18 +6178,25 @@ static int nf_tables_getsetelem(struct sk_buff *skb, if (!nla[NFTA_SET_ELEM_LIST_ELEMENTS]) return -EINVAL; + if (!try_module_get(THIS_MODULE)) + return -EINVAL; + rcu_read_unlock(); + mutex_lock(&nft_net->commit_mutex); + rcu_read_lock(); nla_for_each_nested(attr, nla[NFTA_SET_ELEM_LIST_ELEMENTS], rem) { - err = nft_get_set_elem(&ctx, set, attr, reset); + err = nft_get_set_elem(&ctx, set, attr, true); if (err < 0) { NL_SET_BAD_ATTR(extack, attr); break; } nelems++; } + rcu_read_unlock(); + mutex_unlock(&nft_net->commit_mutex); + rcu_read_lock(); + module_put(THIS_MODULE); - if (reset) - audit_log_nft_set_reset(table, nft_pernet(net)->base_seq, - nelems); + audit_log_nft_set_reset(table, nft_net->base_seq, nelems); return err; } @@ -9140,7 +9217,7 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = { .policy = nft_set_elem_list_policy, }, [NFT_MSG_GETSETELEM_RESET] = { - .call = nf_tables_getsetelem, + .call = nf_tables_getsetelem_reset, .type = NFNL_CB_RCU, .attr_count = NFTA_SET_ELEM_LIST_MAX, .policy = nft_set_elem_list_policy,