Skip to contentSkip to conversation

Coverity: nf_tables_addchain(): Memory - corruptions

DISCUSSION coverity-bot 2 participants 2 messages Jun 8, 2021 Linux-next tree

lore
Share & tools
lore
#0 lore raw

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):

Addresses-Coverity-ID: 1505166 ("Memory - corruptions")

Fixes 7b4b2fa37587 ("netfilter: annotate nf_tables base hook ops")

Thanks for your attention!

Signature · 1 line
Coverity-bot
#1 lore raw
Quoted 6 lines · coverity-bot

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.