Re: [PATCH nf v2] netfilter: x_tables: Switch synchronization to RCU

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Wed, Nov 25, 2020 at 11:27:22AM -0700, Subash Abhinov Kasiviswanathan wrote:
> When running concurrent iptables rules replacement with data, the per CPU
> sequence count is checked after the assignment of the new information.
> The sequence count is used to synchronize with the packet path without the
> use of any explicit locking. If there are any packets in the packet path using
> the table information, the sequence count is incremented to an odd value and
> is incremented to an even after the packet process completion.
> 
> The new table value assignment is followed by a write memory barrier so every
> CPU should see the latest value. If the packet path has started with the old
> table information, the sequence counter will be odd and the iptables
> replacement will wait till the sequence count is even prior to freeing the
> old table info.
> 
> However, this assumes that the new table information assignment and the memory
> barrier is actually executed prior to the counter check in the replacement
> thread. If CPU decides to execute the assignment later as there is no user of
> the table information prior to the sequence check, the packet path in another
> CPU may use the old table information. The replacement thread would then free
> the table information under it leading to a use after free in the packet
> processing context-
> 
> Unable to handle kernel NULL pointer dereference at virtual
> address 000000000000008e
> pc : ip6t_do_table+0x5d0/0x89c
> lr : ip6t_do_table+0x5b8/0x89c
> ip6t_do_table+0x5d0/0x89c
> ip6table_filter_hook+0x24/0x30
> nf_hook_slow+0x84/0x120
> ip6_input+0x74/0xe0
> ip6_rcv_finish+0x7c/0x128
> ipv6_rcv+0xac/0xe4
> __netif_receive_skb+0x84/0x17c
> process_backlog+0x15c/0x1b8
> napi_poll+0x88/0x284
> net_rx_action+0xbc/0x23c
> __do_softirq+0x20c/0x48c
> 
> This could be fixed by forcing instruction order after the new table
> information assignment or by switching to RCU for the synchronization.

Applied, thanks.