Sean Tranchetti <stranche@xxxxxxxxxxxxxx> wrote:
[ CC Xin Long ]
> A recently reported crash in the x_tables framework seems to stem from
> a potential race condition between adding rules to a table and having a
> packet traversing the table at the same time.
>
> In the crash, the jumpstack being used by the table traversal was freed
> by the table replace code. After performing some bisection, it seems that
> commit f31e5f1a891f ("netfilter: unlock xt_table earlier in __do_replace")
> exposed this race condition by unlocking the table before the
> get_old_counters() routine was called to perform the synchronization.
But the packet path doesn't grab the table mutex.
> Call Stack:
> Unable to handle kernel paging request at virtual address
> 006b6b6b6b6b6bc5
>
> pc : ipt_do_table+0x3b8/0x660
> lr : ipt_do_table+0x31c/0x660
> Call trace:
> ipt_do_table+0x3b8/0x660
> iptable_mangle_hook+0x58/0xf8
> nf_hook_slow+0x48/0xd8
> __ip_local_out+0xf4/0x138
> __ip_queue_xmit+0x348/0x3a0
> ip_queue_xmit+0x10/0x18
>
> Signed-off-by: Sean Tranchetti <stranche@xxxxxxxxxxxxxx>
> ---
> @@ -921,8 +921,6 @@ static int __do_replace(struct net *net, const char *name,
> (newinfo->number <= oldinfo->initial_entries))
> module_put(t->me);
>
> - xt_table_unlock(t);
> -
> get_old_counters(oldinfo, counters);
>
> /* Decrease module usage counts and free resource */
> @@ -937,6 +935,7 @@ static int __do_replace(struct net *net, const char *name,
> net_warn_ratelimited("arptables: counters copy to user failed while replacing table\n");
> }
> vfree(counters);
> + xt_table_unlock(t);
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.
