On Sat, Oct 31, 2026 at 02:55:55PM -0300, Joas Antonio wrote:
Hi Pablo,
Sorry, the description was too terse. Answering both mails here.
There is no memory safety issue, and I should have said so explicitly.
It is a correctness problem: after GC the set no longer holds what was
committed, and the packet path matches keys that were never added.
Exact sequence, raw nfnetlink. The set has NFT_SET_INTERVAL |
NFT_SET_TIMEOUT, an ipv4_addr key, no default timeout, and
NFTA_SET_GC_INTERVAL = 3600000, so the commit-time scan does not run and
only the insertion path is involved.
Batch 1 (NEWSETELEM): start 10 with a 50ms timeout, end 20 with no
timeout, start 30 and end 40 with no timeout. Only start 10 expires;
[30,40) is permanent.
Wait 200ms.
Batch 2: DELSETELEM of end 20 only, then NEWSETELEM of [12,13). This is
not something nft(8) generates, but the kernel accepts it. Inserting 12
runs nft_rbtree_gc_elem() on the expired start 10. The walk skips end
20 because it is being deleted in this transaction, passes start 30 and
takes end 40 as the end of 10. 10 and 40 are removed, and start 30 is
left without an end.
At commit, nft_rbtree_commit() turns 30 into an interval with ->from =
30 and ->to = NULL, and nft_array_lookup_cmp() treats ->to == NULL as
open-ended. So nft_rbtree_lookup() matches every key >= 30, while
userspace believes the set holds only [30,40). Reading back with
GETSETELEM, which uses the same array: without the patch 45 is found,
end 40 is gone and deleting [30,40) fails with ENOENT; with the patch
45 is not found, 40 is present and the delete succeeds.
To be fair about the limits: the only trigger I have is crafted
nfnetlink from whoever already owns the ruleset (CAP_NET_ADMIN in that
netns), so it does not cross a privilege boundary, and I have not found
an nft(8) command sequence that reaches the insertion-path case.
Ok, then you confirm such sequence makes no sense for nft(8) command.
The commit-path case (nft_rbtree_gc_scan() not clearing rbe_end)
uses a different sequence without the lone end delete; I can send it
in the same detail if useful.
If you consider sets built from such batches unsupported, I think the
better fix is to reject deleting a lone interval end element, and I can
send that instead; or drop this if it is not worth the churn.
While it would be good if this interface could be tighten in the
future to reject nonsense, I think it is not worth the churn at this
stage.