|
On Tue, Aug 26, 2025 at 7:18 AM Florian Westphal <fw@xxxxxxxxx> wrote:
>
> Zhang Tengfei <zhtfdev@xxxxxxxxx> 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?
KCSAN + panic_on_warn=1 : Only in debug environment
>
> > 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).
+2
|
