netfilter: nft_set_pipapo: don't check genbit from packetpath lookups
authorFlorian Westphal <fw@strlen.de>
Wed, 10 Sep 2025 08:02:18 +0000 (10:02 +0200)
committerFlorian Westphal <fw@strlen.de>
Wed, 10 Sep 2025 18:30:37 +0000 (20:30 +0200)
commitc4eaca2e1052adfd67bed0a36a9d4b8e515666e4
tree9ac8e468d44159b45f1c6dace0e42009c6efabd3
parent5e13f2c491a4100d208e77e92fe577fe3dbad6c2
netfilter: nft_set_pipapo: don't check genbit from packetpath lookups

The pipapo set type is special in that it has two copies of its
datastructure: one live copy containing only valid elements and one
on-demand clone used during transaction where adds/deletes happen.

This clone is not visible to the datapath.

This is unlike all other set types in nftables, those all link new
elements into their live hlist/tree.

For those sets, the lookup functions must skip the new elements while the
transaction is ongoing to ensure consistency.

As the clone is shallow, removal does have an effect on the packet path:
once the transaction enters the commit phase the 'gencursor' bit that
determines which elements are active and which elements should be ignored
(because they are no longer valid) is flipped.

This causes the datapath lookup to ignore these elements if they are found
during lookup.

This opens up a small race window where pipapo has an inconsistent view of
the dataset from when the transaction-cpu flipped the genbit until the
transaction-cpu calls nft_pipapo_commit() to swap live/clone pointers:

cpu0 cpu1
  has added new elements to clone
  has marked elements as being
  inactive in new generation
perform lookup in the set
  enters commit phase:

I) increments the genbit
A) observes new genbit
  removes elements from the clone so
  they won't be found anymore
B) lookup in datastructure
   can't see new elements yet,
   but old elements are ignored
   -> Only matches elements that
   were not changed in the
   transaction
II) calls nft_pipapo_commit(), clone
    and live pointers are swapped.
C New nft_lookup happening now
          will find matching elements.

Consider a packet matching range r1-r2:

cpu0 processes following transaction:
1. remove r1-r2
2. add r1-r3

P is contained in both ranges. Therefore, cpu1 should always find a match
for P.  Due to above race, this is not the case:

cpu1 does find r1-r2, but then ignores it due to the genbit indicating
the range has been removed.

At the same time, r1-r3 is not visible yet, because it can only be found
in the clone.

The situation persists for all lookups until after cpu0 hits II).

The fix is easy: Don't check the genbit from pipapo lookup functions.
This is possible because unlike the other set types, the new elements are
not reachable from the live copy of the dataset.

The clone/live pointer swap is enough to avoid matching on old elements
while at the same time all new elements are exposed in one go.

After this change, step B above returns a match in r1-r2.
This is fine: r1-r2 only becomes truly invalid the moment they get freed.
This happens after a synchronize_rcu() call and rcu read lock is held
via netfilter hook traversal (nf_hook_slow()).

Cc: Stefano Brivio <sbrivio@redhat.com>
Fixes: 3c4287f62044 ("nf_tables: Add set type for arbitrary concatenation of ranges")
Signed-off-by: Florian Westphal <fw@strlen.de>
net/netfilter/nft_set_pipapo.c
net/netfilter/nft_set_pipapo_avx2.c