Re: [PATCH] net/netfilter/ipvs: Fix data-race in ip_vs_add_service / ip_vs_out_hook

Florian Westphal <[email protected]> Tue, 26 Aug 2025 16:18:43 +0200
Newsgroups org.kernel.vger.lvs-devel,org.kernel.vger.netfilter-devel
Message-ID <[email protected]>
Zhang Tengfei <[email protected]> wrote:
> A data-race was detected by KCSAN between ip_vs_add_service() which
> acts as a writer, and ip_vs_out_hook() which acts as a reader. This
> can lead to unpredictable behavior and crashes.

Really?  How can this cause a crash?

> The race occurs on the `enable` flag within the `netns_ipvs`
> struct. This flag was being written in the configuration path without
> any protection, while concurrently being read in the packet processing
> path. This lack of synchronization means a reader on one CPU could see a
> partially initialized service, leading to incorrect behavior.
> 
> To fix this, convert the `enable` flag from a plain integer to an
> atomic_t. This ensures that all reads and writes to the flag are atomic.
> More importantly, using atomic_set() and atomic_read() provides the
> necessary memory barriers to guarantee that changes to other fields of
> the service are visible to the reader CPU before the service is marked
> as enabled.

> -	int			enable;		/* enable like nf_hooks do */
> +	atomic_t	enable;		/* enable like nf_hooks do */

Julian, Simon, I will defer to your judgment but I dislike this,
because I see no reason for atomic_t.  To me is seems better to use
READ/WRITE_ONCE as ->enable is only ever set but not modified
(increment for instance).