Re: [PATCH RFC] netfilter: nf_tables: add flowtable map for xdp offload

[Florian Westphal <fw@strlen.de>]

Florian Westphal <fw@strlen.de> wrote:
> This adds a small internal mapping table so that a new bpf (xdp) kfunc
> A device cannot be added to multiple flowtables, the mapping needs
> to be unique.

This breaks two cases:
1.  Two-Phase commmit protocol:
nft -f - <<EOF
flush ruleset
table t {
	flowtable ...
EOF

fails when called a 2nd time.  This problem also exists
for at least the mlx hw offload too.

It would be good to fix this generically but I do not see
how given this problem is nftables specific and not really
flowtable related.

2. currently nftables supports
table ip t {
	flowtable f {
		devices = { eth0 ...

table ip6 t {
	flowtable f {
		devices = { eth0 ...

table inet t {
	flowtable f {
		devices = { eth0 ...

... and this works, i.e. same device can be part of
up to 6 flowtables.

This one is easier to fix, the program can guess ip/ip6
based to packet data and can a family to the kfunc as a
function argument.

inet would be shadowed / hidden when an ip/ip6 flowtable
mapping exists as well.

This is not nice, but the ip(6) and inet thing should
not occur in practice and nothing breaks here because
existing sw path is still going to work.

> +static int nf_flow_offload_xdp_setup(struct nf_flowtable *flowtable,
> +				     struct net_device *dev,
> +				     enum flow_block_command cmd)
> +{
> +	switch (cmd) {
> +	case FLOW_BLOCK_BIND:
> +		return nf_flowtable_by_dev_insert(flowtable, dev);

This is fine or at least can be made to work.

> +	case FLOW_BLOCK_UNBIND:
> +		nf_flowtable_by_dev_remove(dev);

This is broken.  UNBIND comes too late when things are torn down.

I only see two solutions:

1. add a new nf_flow_offload_unbind_prepare() that does this
2. Decouple nf_flowtable from nft_flowtable and make nf_flowtable
   refcounted.  As-is, the UNBIND will result in UAF because the
   underlying structures will be free'd immediately after this,
   without any synchronize_rcu().