Re: [PATCH net] netfilter: nf_conntrack: prevent uninit-value in gc_worker

[Florian Westphal <fw@strlen.de>]

Eric Dumazet <edumazet@google.com> wrote:
> KMSAN reported use of uninit-value in gc_worker [1]
> 
> We need to clear ct->timeout in __nf_conntrack_alloc()
> otherwise __nf_conntrack_confirm() might propagate garbage when
> adding nfct_time_stamp to ct->timeout :
> 
> 	ct->timeout += nfct_time_stamp;
>
> [1]
> BUG: KMSAN: uninit-value in gc_worker+0x89e/0x1530 net/netfilter/nf_conntrack_core.c:1028
> CPU: 1 PID: 19 Comm: kworker/1:0 Not tainted 4.18.0-rc4+ #24
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Workqueue: events_power_efficient gc_worker
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x185/0x1e0 lib/dump_stack.c:113
>  kmsan_report+0x195/0x2c0 mm/kmsan/kmsan.c:1092
>  __msan_warning_32+0x7d/0xe0 mm/kmsan/kmsan_instr.c:640
>  gc_worker+0x89e/0x1530 net/netfilter/nf_conntrack_core.c:1028

I wonder how this can happen.

All trackers are supposed to set ->timeout to the correct value,
otherwise (assuming init-to-0), we add a ct entry to global hash that
is expired.

For instance, tcp calls
nf_ct_refresh_acct() at end of its ->packet() callback to set
a timeout based on the connection state.

That being said, I don't see any harm in initing to 0 of course.