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.
