|
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?
> 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).
|
