NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

kern/60734: wg(4): missing membar in wg_update_endpoint_if_necessary



>Number:         60734
>Category:       kern
>Synopsis:       wg(4): missing membar in wg_update_endpoint_if_necessary
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Sep 16 13:35:00 +0000 2026
>Originator:     Taylor R Campbell
>Release:        current, 11, 10, ...
>Organization:
The NetWG Barrier, Inc.
>Environment:
>Description:

   2998 static void
   2999 wg_update_endpoint_if_necessary(struct wg_peer *wgp,
   3000     const struct sockaddr *src)
   3001 {
...
   3021 		if (atomic_swap_uint(&wgp->wgp_endpoint_changing, 1) == 0) {
   3022 			wg_change_endpoint(wgp, src);
   3023 		}
...
   3027 }
...
   3618 static void
   3619 wg_task_endpoint_changed(struct wg_softc *wg, struct wg_peer *wgp)
   3620 {
...
   3626 	if (atomic_load_relaxed(&wgp->wgp_endpoint_changing)) {
   3627 		pserialize_perform(wgp->wgp_psz);
   3628 		mutex_exit(wgp->wgp_lock);
   3629 		psref_target_destroy(&wgp->wgp_endpoint0->wgsa_psref,
   3630 		    wg_psref_class);
   3631 		psref_target_init(&wgp->wgp_endpoint0->wgsa_psref,
   3632 		    wg_psref_class);
   3633 		mutex_enter(wgp->wgp_lock);
   3634 		atomic_store_release(&wgp->wgp_endpoint_changing, 0);
   3635 	}
   3636 }

	https://nxr.NetBSD.org/xref/src/sys/net/if_wg.c?r=1.143#2998

	There's no atomic_load_acquire or membar_acquire matching
	atomic_store_release in wg_task_endpoint_changed.  Presumably
	we need the psref target destroy/init cycle to happen-before
	the next wg_change_endpoint, but without any matching barrier,
	this is not guaranteed.

	It's also unclear to me why wg_task_endpoint_changed checks
	wgp_endpoint_changing -- I think this can be an assertion.

>How-To-Repeat:

	1. code inspection

	2. run wg(4) with rapidly varying endpoints on non-TSO CPUs
	   like arm/powerpc/riscv

>Fix:

	1. Insert membar_acquire in wg_update_endpoint_if_necessary
	   between atomic_swap_uint and wg_change_endpoint.

	2. Add comments cross-referencing membar_acquire and
	   atomic_store_release.

	3. Consider changing

   3626 	if (atomic_load_relaxed(&wgp->wgp_endpoint_changing)) {

	   to an assertion instead.




Home | Main Index | Thread Index | Old Index