|
On Wed, Aug 27, 2025 at 10:43:42PM +0800, Zhang Tengfei wrote:
> Hi everyone,
>
> Here is the v2 patch that incorporates the feedback.
Patch without subject will not fly too far, I'm afraid you will have
to resubmit. One more comment below.
> Many thanks to Julian for his thorough review and for providing
> the detailed plan for this new version, and thanks to Florian
> and Eric for suggestions.
>
> Subject: [PATCH v2] net/netfilter/ipvs: Use READ_ONCE/WRITE_ONCE for
> ipvs->enable
>
> KCSAN reported a data-race on the `ipvs->enable` flag, which is
> written in the control path and read concurrently from many other
> contexts.
>
> Following a suggestion by Julian, this patch fixes the race by
> converting all accesses to use `WRITE_ONCE()/READ_ONCE()`.
> This lightweight approach ensures atomic access and acts as a
> compiler barrier, preventing unsafe optimizations where the flag
> is checked in loops (e.g., in ip_vs_est.c).
>
> Additionally, the now-obsolete `enable` checks in the fast path
> hooks (`ip_vs_in_hook`, `ip_vs_out_hook`, `ip_vs_forward_icmp`)
> are removed. These are unnecessary since commit 857ca89711de
> ("ipvs: register hooks only with services").
>
> Reported-by: syzbot+1651b5234028c294c339@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=1651b5234028c294c339
> Suggested-by: Julian Anastasov <ja@xxxxxx>
> Link:
> https://lore.kernel.org/lvs-devel/2189fc62-e51e-78c9-d1de-d35b8e3657e3@xxxxxx/
> Signed-off-by: Zhang Tengfei <zhtfdev@xxxxxxxxx>
>
> ---
> v2:
> - Switched from atomic_t to the suggested READ_ONCE()/WRITE_ONCE().
> - Removed obsolete checks from the packet processing hooks.
> - Polished commit message based on feedback.
> ---
> net/netfilter/ipvs/ip_vs_conn.c | 4 ++--
> net/netfilter/ipvs/ip_vs_core.c | 11 ++++-------
> net/netfilter/ipvs/ip_vs_ctl.c | 6 +++---
> net/netfilter/ipvs/ip_vs_est.c | 16 ++++++++--------
> 4 files changed, 17 insertions(+), 20 deletions(-)
[...]
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index c7a8a08b7..5ea7ab8bf 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -1353,9 +1353,6 @@ ip_vs_out_hook(void *priv, struct sk_buff *skb, const
> struct nf_hook_state *stat
> if (unlikely(!skb_dst(skb)))
> return NF_ACCEPT;
>
> - if (!ipvs->enable)
> - return NF_ACCEPT;
Patch does say why is this going away? If you think this is not
necessary, then make a separated patch and example why this is needed?
Thanks
> ip_vs_fill_iph_skb(af, skb, false, &iph);
> #ifdef CONFIG_IP_VS_IPV6
> if (af == AF_INET6) {
|
