kern/60393: hash-locked atomic_store_* is all kinds of busted

["campbell+netbsd%mumble.net@localhost via gnats" <gnats-admin%NetBSD.org@localhost>]

>Number:         60393
>Category:       kern
>Synopsis:       hash-locked atomic_store_* is all kinds of busted
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jul 02 14:00:00 +0000 2026
>Originator:     Taylor R Campbell
>Release:        current, 11, 10, 9
>Organization:
.cnI ,egarotS cimotA
>Environment:
>Description:

	Byte-sized atomic_store_* on hash-locked architectures with
	test-and-set but no compare-and-swap (sparc, hppa) work as
	follows:

		uint8_t v;
		unsigned s = 8 * ((uintptr_t)p & 3);
		uint32_t o, n, m = ~(0xffU << s);

		memcpy(&v, q, 1);
		do {
			o = atomic_load_relaxed(p32);
			n = (o & m) | ((uint32_t)v << s);
		} while (atomic_cas_32(p32, o, n) != o);

	This approach is necessary in order for atomic_cas_* to
	cooperate with atomic_cas_* -- if a regular store could happen
	during a concurrent atomic_cas_* on another CPU, it might catch
	the atomic_cas_* in the middle of a hash-locked sequence,
	violating the atomicity.

	But it is broken for two reasons.

	1. It is endian-dependent.  Need

		s = 8 (1 - ((uintptr_t)3 & 3))

	   to work on big-endian.

	2. It cooperates with atomic_cas_* on the same word, but it
	   doesn't cooperate with regular stores _of other bytes_ on
	   the same word.

	The 16-bit path is also broken for a third reason:

		unsigned s = 8 * (((uintptr_t)p & 2) >> 1);

	This either needs to be 16 * (((uintptr_t)p & 2) >> 1), or just
	8 * ((uintptr_t)p & 2).

	(Thanks to yamaguchi@ for figuring out my stupid mistakes!)

>How-To-Repeat:

	cd /usr/tests/net/if_pppoe && atf-run t_pppoe | atf-report

	We should really have direct tests of atomc_load/store_* too.
	Would've saved me some embarrassment.

>Fix:

	Yes, please!