|
Hello,
On Thu, 28 Aug 2025, Zhang Tengfei wrote:
> 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 `enable` checks in the fast-path hooks
> (`ip_vs_in_hook`, `ip_vs_out_hook`, `ip_vs_forward_icmp`) are
> removed. They are considered unnecessary because the `enable=0`
It was good idea to mention about the 857ca89711de
commit here as in the previous v2 version. You can even add
it as Fixes tag as suggested here:
scripts/checkpatch.pl --strict /tmp/file.patch
As for the Subject line, you probably can use
[PATCH v3 nf-next] ipvs: Use READ_ONCE/WRITE_ONCE for ipvs->enable
to specify the desired target tree ('nf-next' or 'nf' if such
data-race needs it).
As for the patch code, it looks ok.
> condition they check for can only occur in two rare and non-fatal
> scenarios: 1) after hooks are registered but before the flag is set,
> and 2) after hooks are unregistered on cleanup_net. In the worst
> case, a single packet might be mishandled (e.g., dropped), which
> does not lead to a system crash or data corruption. Adding a check
> in the performance-critical fast-path to handle this harmless
> condition is not a worthwhile trade-off.
>
> 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 from maintainers.
...
Regards
--
Julian Anastasov <ja@xxxxxx>
|
