On Thu, Oct 19, 2023 at 01:33:47PM +0200, Phil Sutter wrote:
> 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 <phil@xxxxxx>
> ---
> Changes since v2:
> - Keep local variable 'nft_net' in nf_tables_getrule_reset()
> - No need for local variable 'family' in same function (used only once
> after all the churn)
> ---
> net/netfilter/nf_tables_api.c | 74 ++++++++++++++++++++++++++++-------
> 1 file changed, 60 insertions(+), 14 deletions(-)
>
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 584d3b204372..fbb688c9903c 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
[...]
> +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);
NACK.
This just mitigates the problem we are discussing, when there is an
interference with an ongoing transaction.
