Re: crash in death_by_timeout()

[Patrick McHardy <kaber@trash.net>]

BORBELY Zoltan wrote:
> Hi,
> 
> There's a race in the nfct netlink code, the result is a crash in the
> death_by_timeout() function. When a timer interrupt occures during a
> new entry addition, the kernel will crash due to a NULL deref. The
> attached patch has solved the problem for us. I haven't tested it on
> the latest kernels, but the problem still seems to be there.

> --- /tmp/nf_conntrack_netlink.c-orig	2008-09-29 23:28:55.000000000 +0200
> +++ /tmp/nf_conntrack_netlink.c	2008-09-29 23:29:11.000000000 +0200
> @@ -1177,8 +1177,8 @@
>  		ct->master = master_ct;
>  	}
>  
> -	add_timer(&ct->timeout);
>  	nf_conntrack_hash_insert(ct);
> +	add_timer(&ct->timeout);
>  	rcu_read_unlock();

That code looks very fishy. We should be holding the conntrack lock,
otherwise the addition is not only racy against the timer, but also
against addition of identical conntracks. Let me look into what
happened here.