On Fri, Mar 13, 2020 at 06:42:06PM +0100, Phil Sutter wrote:
> If kernel ruleset is constantly changing, code called by
> nft_is_table_compatible() may crash: For each item in table's chain
> list, nft_is_chain_compatible() is called. This in turn calls
> nft_build_cache() to fetch chain's rules. Though if kernel genid has changed
> meanwhile, cache is flushed and rebuilt from scratch, thereby freeing
> table's chain list - the foreach loop in nft_is_table_compatible() then
> operates on freed memory.
>
> A simple reproducer (may need a few calls):
>
> | RULESET='*filter
> | :INPUT ACCEPT [10517:1483527]
> | :FORWARD ACCEPT [0:0]
> | :OUTPUT ACCEPT [1714:105671]
> | COMMIT
> | '
> |
> | for ((i = 0; i < 100; i++)); do
> | iptables-nft-restore <<< "$RULESET" &
> | done &
> | iptables-nft-save
>
> To fix the problem, basically revert commit ab1cd3b510fa5 ("nft: ensure
> cache consistency") so that __nft_build_cache() no longer flushes the
> cache. Instead just record kernel's genid when fetching for the first
> time. If kernel rule set changes until the changes are committed, the
> commit simply fails and local cache is being rebuilt.
>
> Signed-off-by: Phil Sutter <phil@xxxxxx>
> ---
> Pablo,
>
> This is a similar situation as with commit c550c81fd373e ("nft: cache:
> Fix nft_release_cache() under stress"): Your caching rewrite avoids this
> problem as well, but kills some of my performance improvements. I'm
> currently working on restoring them with your approach but that's not a
> quick "fix". Can we go with this patch until I'm done?
Fair enough.
