Re: [PATCH nf] Revert "netfilter: unlock xt_table earlier in __do_replace"

[stranche@codeaurora.org]

On 2020-01-23 03:29, Florian Westphal wrote:
> 
> I don't see how this changes anything wrt. packet path.
> This disallows another instance of iptables(-restore) to come in
> before the counters have been copied/freed and the destructors have 
> run.
> 
> But as those have nothing to do with the jumpstack I don't see how this
> helps.

Based on on the stack of the iptables-restore task that freed the 
jumpstack being accessed in the ipt_do_table() routine, we end up in 
__do_replace()
       0xFFFFFF9239243AE0, ->kvfree
       0xFFFFFF923A1969EC, ->xt_free_table_info+0x50
       0xFFFFFF923A2100E0, ->__do_replace+0x200

Prior to the original patch, this xt_free_table_info was under lock, so 
it seems that having this call under lock guarantees that the new 
table->private entry that contains the jumpstack is seen across all 
CPUs.

> 
> But the packet path doesn't grab the table mutex.
> 

Good point. Perhaps the reason that moving this lock helps is because it 
prevents multiple writers from stepping on one another in such a way 
that the private entry is left in a bad state. Or this whole thing is a 
red herring and the problem is actually that xt_replace_table() is able 
to return prematurely and not all CPUs are finished with the old 
jumpstack by the time the old table info is freed.