Phil Oester wrote:
> I agree I am likely experiencing the deadlock you refer to:
> 
> CPU1:
> conntrack-helper:help: lock(private_lock)
> ip_conntrack_expect_related: write_lock(ip_conntrack_lock)
> 
> CPU2:
> nat-core:do_bindings: read_lock(ip_conntrack_lock)
> nat-helper:help: lock(private_lock)
> 
> However, it's unclear to me that the ip_ftp_lock can be trivially eliminated.
> 
> This code path looks particularly prickly in ip_nat_ftp.c:
> 
> help
>   ftp_data_fixup
>     ip_conntrack_change_expect
> 
> so the nat helper is changing the expectation -- potentially at the same time
> the conntrack helper is calling ip_conntrack_expect.  If the private lock
> were removed, could this not cause a race condition if the expectation got
> created just after the nat-helper changed the expectation?  

I don't understand what you mean. The nat-helper can not change the
expectation before it's created, and once it's created it can not
be touched by the conntrack helper anymore. ip_ftp_lock is protecting
ftp_buffer and ct_ftp_info, so it can be dropped directly after the
call to find_pattern. The help function in the nat_helper only touches
the expectation, which is protected by ip_conntrack_lock, so it doesn't
need ip_ftp_lock lock at all.

> 
> It seems the ip_ftp_lock is needed, but perhaps needs to be reworked to avoid
> the deadlock condition illustrated above.

I've attached an untested patch which should fix this problem for the
ftp helper. Care to give it a try ?

Regards
Patrick

> 
> Thoughts?
> 
> Phil