| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20250121170458.20ae0ba33a493d3232e7fa04@kernel.org>
Date: Tue, 21 Jan 2025 17:04:58 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org, Masami
Hiramatsu <mhiramat@...nel.org>, Mark Rutland <mark.rutland@....com>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Andrew Morton
<akpm@...ux-foundation.org>, Peter Zijlstra <peterz@...radead.org>, Thomas
Gleixner <tglx@...utronix.de>, Linus Torvalds
<torvalds@...ux-foundation.org>, Ludwig Rydberg
<ludwig.rydberg@...sler.com>, Andreas Larsson <andreas@...sler.com>,
stable@...r.kernel.org
Subject: Re: [PATCH 2/2] atomic64: Use arch_spin_locks instead of
raw_spin_locks
On Mon, 20 Jan 2025 18:56:57 -0500
Steven Rostedt <rostedt@...dmis.org> wrote:
> From: Steven Rostedt <rostedt@...dmis.org>
>
> raw_spin_locks can be traced by lockdep or tracing itself. Atomic64
> operations can be used in the tracing infrastructure. When an architecture
> does not have true atomic64 operations it can use the generic version that
> disables interrupts and uses spin_locks.
>
> The tracing ring buffer code uses atomic64 operations for the time
> keeping. But because some architectures use the default operations, the
> locking inside the atomic operations can cause an infinite recursion.
>
> As atomic64 is an architecture specific operation, it should not be using
> raw_spin_locks() but instead arch_spin_locks as that is the purpose of
> arch_spin_locks. To be used in architecture specific implementations of
> generic infrastructure like atomic64 operations.
>
Looks good to me.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>
Thanks,
> Cc: stable@...r.kernel.org
> Fixes: c84897c0ff592 ("ring-buffer: Remove 32bit timestamp logic")
> Closes: https://lore.kernel.org/all/86fb4f86-a0e4-45a2-a2df-3154acc4f086@gaisler.com/
> Reported-by: Ludwig Rydberg <ludwig.rydberg@...sler.com>
> Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
> ---
> lib/atomic64.c | 78 +++++++++++++++++++++++++++++++-------------------
> 1 file changed, 48 insertions(+), 30 deletions(-)
>
> diff --git a/lib/atomic64.c b/lib/atomic64.c
> index caf895789a1e..1a72bba36d24 100644
> --- a/lib/atomic64.c
> +++ b/lib/atomic64.c
> @@ -25,15 +25,15 @@
> * Ensure each lock is in a separate cacheline.
> */
> static union {
> - raw_spinlock_t lock;
> + arch_spinlock_t lock;
> char pad[L1_CACHE_BYTES];
> } atomic64_lock[NR_LOCKS] __cacheline_aligned_in_smp = {
> [0 ... (NR_LOCKS - 1)] = {
> - .lock = __RAW_SPIN_LOCK_UNLOCKED(atomic64_lock.lock),
> + .lock = __ARCH_SPIN_LOCK_UNLOCKED,
> },
> };
>
> -static inline raw_spinlock_t *lock_addr(const atomic64_t *v)
> +static inline arch_spinlock_t *lock_addr(const atomic64_t *v)
> {
> unsigned long addr = (unsigned long) v;
>
> @@ -45,12 +45,14 @@ static inline raw_spinlock_t *lock_addr(const atomic64_t *v)
> s64 generic_atomic64_read(const atomic64_t *v)
> {
> unsigned long flags;
> - raw_spinlock_t *lock = lock_addr(v);
> + arch_spinlock_t *lock = lock_addr(v);
> s64 val;
>
> - raw_spin_lock_irqsave(lock, flags);
> + local_irq_save(flags);
> + arch_spin_lock(lock);
> val = v->counter;
> - raw_spin_unlock_irqrestore(lock, flags);
> + arch_spin_unlock(lock);
> + local_irq_restore(flags);
> return val;
> }
> EXPORT_SYMBOL(generic_atomic64_read);
> @@ -58,11 +60,13 @@ EXPORT_SYMBOL(generic_atomic64_read);
> void generic_atomic64_set(atomic64_t *v, s64 i)
> {
> unsigned long flags;
> - raw_spinlock_t *lock = lock_addr(v);
> + arch_spinlock_t *lock = lock_addr(v);
>
> - raw_spin_lock_irqsave(lock, flags);
> + local_irq_save(flags);
> + arch_spin_lock(lock);
> v->counter = i;
> - raw_spin_unlock_irqrestore(lock, flags);
> + arch_spin_unlock(lock);
> + local_irq_restore(flags);
> }
> EXPORT_SYMBOL(generic_atomic64_set);
>
> @@ -70,11 +74,13 @@ EXPORT_SYMBOL(generic_atomic64_set);
> void generic_atomic64_##op(s64 a, atomic64_t *v) \
> { \
> unsigned long flags; \
> - raw_spinlock_t *lock = lock_addr(v); \
> + arch_spinlock_t *lock = lock_addr(v); \
> \
> - raw_spin_lock_irqsave(lock, flags); \
> + local_irq_save(flags); \
> + arch_spin_lock(lock); \
> v->counter c_op a; \
> - raw_spin_unlock_irqrestore(lock, flags); \
> + arch_spin_unlock(lock); \
> + local_irq_restore(flags); \
> } \
> EXPORT_SYMBOL(generic_atomic64_##op);
>
> @@ -82,12 +88,14 @@ EXPORT_SYMBOL(generic_atomic64_##op);
> s64 generic_atomic64_##op##_return(s64 a, atomic64_t *v) \
> { \
> unsigned long flags; \
> - raw_spinlock_t *lock = lock_addr(v); \
> + arch_spinlock_t *lock = lock_addr(v); \
> s64 val; \
> \
> - raw_spin_lock_irqsave(lock, flags); \
> + local_irq_save(flags); \
> + arch_spin_lock(lock); \
> val = (v->counter c_op a); \
> - raw_spin_unlock_irqrestore(lock, flags); \
> + arch_spin_unlock(lock); \
> + local_irq_restore(flags); \
> return val; \
> } \
> EXPORT_SYMBOL(generic_atomic64_##op##_return);
> @@ -96,13 +104,15 @@ EXPORT_SYMBOL(generic_atomic64_##op##_return);
> s64 generic_atomic64_fetch_##op(s64 a, atomic64_t *v) \
> { \
> unsigned long flags; \
> - raw_spinlock_t *lock = lock_addr(v); \
> + arch_spinlock_t *lock = lock_addr(v); \
> s64 val; \
> \
> - raw_spin_lock_irqsave(lock, flags); \
> + local_irq_save(flags); \
> + arch_spin_lock(lock); \
> val = v->counter; \
> v->counter c_op a; \
> - raw_spin_unlock_irqrestore(lock, flags); \
> + arch_spin_unlock(lock); \
> + local_irq_restore(flags); \
> return val; \
> } \
> EXPORT_SYMBOL(generic_atomic64_fetch_##op);
> @@ -131,14 +141,16 @@ ATOMIC64_OPS(xor, ^=)
> s64 generic_atomic64_dec_if_positive(atomic64_t *v)
> {
> unsigned long flags;
> - raw_spinlock_t *lock = lock_addr(v);
> + arch_spinlock_t *lock = lock_addr(v);
> s64 val;
>
> - raw_spin_lock_irqsave(lock, flags);
> + local_irq_save(flags);
> + arch_spin_lock(lock);
> val = v->counter - 1;
> if (val >= 0)
> v->counter = val;
> - raw_spin_unlock_irqrestore(lock, flags);
> + arch_spin_unlock(lock);
> + local_irq_restore(flags);
> return val;
> }
> EXPORT_SYMBOL(generic_atomic64_dec_if_positive);
> @@ -146,14 +158,16 @@ EXPORT_SYMBOL(generic_atomic64_dec_if_positive);
> s64 generic_atomic64_cmpxchg(atomic64_t *v, s64 o, s64 n)
> {
> unsigned long flags;
> - raw_spinlock_t *lock = lock_addr(v);
> + arch_spinlock_t *lock = lock_addr(v);
> s64 val;
>
> - raw_spin_lock_irqsave(lock, flags);
> + local_irq_save(flags);
> + arch_spin_lock(lock);
> val = v->counter;
> if (val == o)
> v->counter = n;
> - raw_spin_unlock_irqrestore(lock, flags);
> + arch_spin_unlock(lock);
> + local_irq_restore(flags);
> return val;
> }
> EXPORT_SYMBOL(generic_atomic64_cmpxchg);
> @@ -161,13 +175,15 @@ EXPORT_SYMBOL(generic_atomic64_cmpxchg);
> s64 generic_atomic64_xchg(atomic64_t *v, s64 new)
> {
> unsigned long flags;
> - raw_spinlock_t *lock = lock_addr(v);
> + arch_spinlock_t *lock = lock_addr(v);
> s64 val;
>
> - raw_spin_lock_irqsave(lock, flags);
> + local_irq_save(flags);
> + arch_spin_lock(lock);
> val = v->counter;
> v->counter = new;
> - raw_spin_unlock_irqrestore(lock, flags);
> + arch_spin_unlock(lock);
> + local_irq_restore(flags);
> return val;
> }
> EXPORT_SYMBOL(generic_atomic64_xchg);
> @@ -175,14 +191,16 @@ EXPORT_SYMBOL(generic_atomic64_xchg);
> s64 generic_atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u)
> {
> unsigned long flags;
> - raw_spinlock_t *lock = lock_addr(v);
> + arch_spinlock_t *lock = lock_addr(v);
> s64 val;
>
> - raw_spin_lock_irqsave(lock, flags);
> + local_irq_save(flags);
> + arch_spin_lock(lock);
> val = v->counter;
> if (val != u)
> v->counter += a;
> - raw_spin_unlock_irqrestore(lock, flags);
> + arch_spin_unlock(lock);
> + local_irq_restore(flags);
>
> return val;
> }
> --
> 2.45.2
>
>
--
Masami Hiramatsu (Google) <mhiramat@...nel.org>
Powered by blists - more mailing lists