Re: [PATCH] locking issue in __unexpect_related

[Patrick McHardy <kaber@trash.net>]

Pablo Neira wrote:
> Hi Harald and list,
> 
> In the function __unexpect_related:
> 
>         /* delete from global and local lists */
>         list_del(&expect->list);
>         list_del(&expect->expected_list);
> 
> If an entry of the global list of expectation is erased, shouldn't this 
> operation be write_lock'ed? If missing something, please let me know.

Your patch will deadlock, the function should only be called under
write-locked ip_conntrack_lock, which you can see by the
MUST_BE_WRITE_LOCKED assertion. Check the callers ..

Regards
Patrick


> 
> regards,
> Pablo
> 
> 
> ------------------------------------------------------------------------
> 
> --- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_core.c	2004-04-14 21:32:58.000000000 +0200
> +++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_core.c	2004-04-22 00:55:42.000000000 +0200
> @@ -226,7 +226,9 @@
>  	IP_NF_ASSERT(!expect->sibling);
>  
>  	/* delete from global and local lists */
> +	WRITE_LOCK(&ip_conntrack_expect_list);
>  	list_del(&expect->list);
> +	WRITE_UNLOCK(&ip_conntrack_expect_list);
>  	list_del(&expect->expected_list);
>  
>  	/* decrement expect-count of master conntrack */