Re: [NETFILTER 01/03]: nf_conntrack: fix helper module unload races

[Patrick McHardy <kaber@trash.net>]

Yasuyuki KOZAKAI wrote:
> nfct_help(ct) also needs to be protected because of the race between
> nfctnetlink and helper. Sorry, I did not test the following patch because
> I don't have SMP machine.
> 
> You might dislike this.  Other idea is to use call_rcu() in
> nfctnetlink_change_helper(), but I need to study the way to use it more.
> 
> 
> [NETFILTER]: nf_conntrack: Fixes race on changing helper
> 
> nfctnetlink may change the helper assigned to a conntrack and clear
> private area in the conntrack. To fix the race, this exports spin locks
> of helpers and nfctnetlink uses them. Some helpers do not need to grab
> spin lock, because they do not store any state in the private area.
> 
> Other staff does not need to grab spin lock.
> - unhelp() assigns NULL to conntrack, but it does not touch the private
>   area.
> - init_conntrack(), nf_conntrack_alter_reply(), and
>   nfctnetlink_create_conntrack() assign a helper to a conntrack,
>   but the conntrack is unconfirmed. No competitor exists.


The fix looks correct, but considering the other problems that changing
helpers cause for ct_extend I'm wondering whether we could restrict the
helper change operation to conntracks that don't already have a helper
assigned. I can't imagine any use for changing helpers of a connection
that already has one assigned and it will always be racy anyway.
Pablo, are you aware of anything using/needing this feature?