[PATCH 0/1] sparc32: prevent stale writes by failed bit-lock waiters

[PATCH 0/1] sparc32: prevent stale writes by failed bit-lock waiters

[Magnus Lindholm]

I traced an SMP HyperSPARC pathname-lookup livelock to a failed bit-lock
waiter restoring an unlinked dentry. Skip unchanged stores in the SPARC32
emulated atomics and bit helpers, without changing generic bitops or ABI.

For background, PA-RISC made test-and-set/clear conditional in 2006
(af5917f0cd60). The 2018 generic rewrite (84c6591103db) routed bit locks
through atomic_fetch_*(), which needs the same unchanged-store rule.
I keep this fix within arch/sparc; PA-RISC's fetch helpers need separate
review and are outside this patch's scope. The SPARC32 fetch helpers,
including fetch-add, gain a comparison; I claim no performance improvement.

The forced-interleaving host test reproduces the old lost update and
checks the corrected generic acquisition path. SMP and UP builds and
QEMU smoke tests passed for the architecture-only implementation.

I completed a native GDB 18.1 build on a dual-CPU HyperSPARC RT626
SPARCstation 20 with 512 MB RAM and a native SPARC32 SMP kernel containing
the combined fixes and optimizations, plus separate platform prerequisites.
I observed no core dumps, livelock or new kernel messages during the run.
This validates the combined integration, not each patch in isolation.

Magnus Lindholm (1):
  sparc32: do not write back unchanged values in emulated atomics

 arch/sparc/lib/atomic32.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)


base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.43.0

[PATCH 1/1] sparc32: do not write back unchanged values in emulated atomics

[Magnus Lindholm]

A failed bit-lock acquisition can restore an unlinked hlist_bl head and
leave pathname lookup retrying a dead dentry indefinitely. The owner
updates the pointer bits with a plain store while holding the bit lock.
SPARC32's hashed atomic lock does not exclude that store: an unconditional
fetch-or can write back a stale pointer even when the lock bit was set.

Skip unchanged stores in the emulated atomic fetch operations and legacy
set/clear bit helpers. This covers current test_and_set_bit()-based locks
and older generic bit-lock implementations. Keep the hash lock and IRQ
exclusion on every path, preserving ordering, return values and
changed-value updates.

This fixes the bit-lock owner/waiter protocol, not arbitrary races between
plain stores and changed-value emulated atomic operations.

Fixes: 26333576fd0d ("bitops: introduce lock ops")
Cc: stable@vger.kernel.org
Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
---
Validation: forced-interleaving host regression; SMP/UP builds and QEMU
smoke boots. A native GDB 18.1 build also passed on dual-CPU HyperSPARC
as part of the combined integration; see the cover letter for scope.

 arch/sparc/lib/atomic32.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/sparc/lib/atomic32.c b/arch/sparc/lib/atomic32.c
index 8ae880ebf07a..30b4e0c12c07 100644
--- a/arch/sparc/lib/atomic32.c
+++ b/arch/sparc/lib/atomic32.c
@@ -28,15 +28,19 @@ static DEFINE_SPINLOCK(dummy);
 
 #endif /* SMP */
 
+/* Failed bit-lock acquisition must not overwrite an owner's plain store. */
 #define ATOMIC_FETCH_OP(op, c_op)					\
 int arch_atomic_fetch_##op(int i, atomic_t *v)				\
 {									\
-	int ret;							\
+	int ret, new;							\
 	unsigned long flags;						\
 	spin_lock_irqsave(ATOMIC_HASH(v), flags);			\
 									\
-	ret = v->counter;						\
-	v->counter c_op i;						\
+	ret = READ_ONCE(v->counter);					\
+	new = ret;							\
+	new c_op i;							\
+	if (new != ret)							\
+		WRITE_ONCE(v->counter, new);				\
 									\
 	spin_unlock_irqrestore(ATOMIC_HASH(v), flags);			\
 	return ret;							\
@@ -125,8 +129,9 @@ unsigned long sp32___set_bit(unsigned long *addr, unsigned long mask)
 	unsigned long old, flags;
 
 	spin_lock_irqsave(ATOMIC_HASH(addr), flags);
-	old = *addr;
-	*addr = old | mask;
+	old = READ_ONCE(*addr);
+	if (!(old & mask))
+		WRITE_ONCE(*addr, old | mask);
 	spin_unlock_irqrestore(ATOMIC_HASH(addr), flags);
 
 	return old & mask;
@@ -138,8 +143,9 @@ unsigned long sp32___clear_bit(unsigned long *addr, unsigned long mask)
 	unsigned long old, flags;
 
 	spin_lock_irqsave(ATOMIC_HASH(addr), flags);
-	old = *addr;
-	*addr = old & ~mask;
+	old = READ_ONCE(*addr);
+	if (old & mask)
+		WRITE_ONCE(*addr, old & ~mask);
 	spin_unlock_irqrestore(ATOMIC_HASH(addr), flags);
 
 	return old & mask;
-- 
2.43.0