Coverity: nf_tables_addchain(): Memory - corruptions

Coverity: nf_tables_addchain(): Memory - corruptions

[coverity-bot]

Hello!

This is an experimental semi-automated report about issues detected by
Coverity from a scan of next-20210608 as part of the linux-next scan project:
https://scan.coverity.com/projects/linux-next-weekly-scan

You're getting this email because you were associated with the identified
lines of code (noted below) that were touched by commits:

  Mon Jun 7 12:23:38 2021 +0200
    7b4b2fa37587 ("netfilter: annotate nf_tables base hook ops")

Coverity reported the following:

*** CID 1505166:  Memory - corruptions  (UNINIT)
/net/netfilter/nf_tables_api.c: 2188 in nf_tables_addchain()
2182     		err = PTR_ERR(trans);
2183     		goto err_unregister_hook;
2184     	}
2185
2186     	nft_trans_chain_policy(trans) = NFT_CHAIN_POLICY_UNSET;
2187     	if (nft_is_base_chain(chain)) {
vvv     CID 1505166:  Memory - corruptions  (UNINIT)
vvv     Using uninitialized value "basechain".
2188     		basechain->ops.hook_ops_type = NF_HOOK_OP_NF_TABLES;
2189     		nft_trans_chain_policy(trans) = policy;
2190     	}
2191
2192     	err = nft_chain_add(table, chain);
2193     	if (err < 0) {

If this is a false positive, please let us know so we can mark it as
such, or teach the Coverity rules to be smarter. If not, please make
sure fixes get into linux-next. :) For patches fixing this, please
include these lines (but double-check the "Fixes" first):

Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
Addresses-Coverity-ID: 1505166 ("Memory - corruptions")
Fixes: 7b4b2fa37587 ("netfilter: annotate nf_tables base hook ops")

Thanks for your attention!

-- 
Coverity-bot

Re: Coverity: nf_tables_addchain(): Memory - corruptions

[Florian Westphal]

coverity-bot <keescook@chromium.org> wrote:
> 2186     	nft_trans_chain_policy(trans) = NFT_CHAIN_POLICY_UNSET;
> 2187     	if (nft_is_base_chain(chain)) {
> vvv     CID 1505166:  Memory - corruptions  (UNINIT)
> vvv     Using uninitialized value "basechain".
> 2188     		basechain->ops.hook_ops_type = NF_HOOK_OP_NF_TABLES;
> 2189     		nft_trans_chain_policy(trans) = policy;

Hmm, I do not see how is_base_chain() can be true while basechain
pointer is garbage.

However its probably better for readability to move the NF_HOOK_OP_...
assignment closer to where the basechain ops are allocated.