On Mon, Sep 12, 2022 at 09:41:00PM +0900, Tetsuo Handa wrote:
> syzbot is reporting underflow of nft_counters_enabled counter at
> nf_tables_addchain() [1], for commit 43eb8949cfdffa76 ("netfilter:
> nf_tables: do not leave chain stats enabled on error") missed that
> nf_tables_chain_destroy() after nft_basechain_init() in the error path of
> nf_tables_addchain() decrements the counter because nft_basechain_init()
> makes nft_is_base_chain() return true by setting NFT_CHAIN_BASE flag.
>
> Increment the counter immediately after returning from
> nft_basechain_init().
Patch LGTM, thanks
> Link: https://syzkaller.appspot.com/bug?extid=b5d82a651b71cd8a75ab [1]
> Reported-by: syzbot <syzbot+b5d82a651b71cd8a75ab@xxxxxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> Tested-by: syzbot <syzbot+b5d82a651b71cd8a75ab@xxxxxxxxxxxxxxxxxxxxxxxxx>
> Fixes: 43eb8949cfdffa76 ("netfilter: nf_tables: do not leave chain stats enabled on error")
> ---
> net/netfilter/nf_tables_api.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 816052089b33..e062754dc6cc 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -2197,7 +2197,6 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
> struct netlink_ext_ack *extack)
> {
> const struct nlattr * const *nla = ctx->nla;
> - struct nft_stats __percpu *stats = NULL;
> struct nft_table *table = ctx->table;
> struct nft_base_chain *basechain;
> struct net *net = ctx->net;
> @@ -2212,6 +2211,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
> return -EOVERFLOW;
>
> if (nla[NFTA_CHAIN_HOOK]) {
> + struct nft_stats __percpu *stats = NULL;
> struct nft_chain_hook hook;
>
> if (flags & NFT_CHAIN_BINDING)
> @@ -2245,6 +2245,8 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
> kfree(basechain);
> return err;
> }
> + if (stats)
> + static_branch_inc(&nft_counters_enabled);
> } else {
> if (flags & NFT_CHAIN_BASE)
> return -EINVAL;
> @@ -2319,9 +2321,6 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
> goto err_unregister_hook;
> }
>
> - if (stats)
> - static_branch_inc(&nft_counters_enabled);
> -
> table->use++;
>
> return 0;
> --
> 2.18.4
>
