[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <e7c311bb305d985c1a9ea19aef3fcd4094c7e03c600dfba9fc83438b285f5946@mail.kernel.org>
Date: Fri, 21 Nov 2025 21:27:45 +0000 (UTC)
From: bot+bpf-ci@...nel.org
To: ameryhung@...il.com,bpf@...r.kernel.org
Cc: netdev@...r.kernel.org,alexei.starovoitov@...il.com,andrii@...nel.org,daniel@...earbox.net,memxor@...il.com,david.laight.linux@...il.com,dave@...olabs.net,paulmck@...nel.org,josh@...htriplett.org,ameryhung@...il.com,kernel-team@...a.com,ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,martin.lau@...nel.org,eddyz87@...il.com,yonghong.song@...ux.dev,clm@...a.com,ihor.solodrai@...ux.dev
Subject: Re: [PATCH bpf-next v2 1/2] rqspinlock: Annotate rqspinlock lock acquiring functions with __must_check
> diff --git a/include/asm-generic/rqspinlock.h b/include/asm-generic/rqspinlock.h
> index 6d4244d64..855c09435 100644
> --- a/include/asm-generic/rqspinlock.h
> +++ b/include/asm-generic/rqspinlock.h
[ ... ]
> @@ -171,7 +171,7 @@ static __always_inline void release_held_lock_entry(void)
> * * -EDEADLK - Lock acquisition failed because of AA/ABBA deadlock.
> * * -ETIMEDOUT - Lock acquisition failed because of timeout.
> */
> -static __always_inline int res_spin_lock(rqspinlock_t *lock)
> +static __always_inline __must_check int res_spin_lock(rqspinlock_t *lock)
> {
> int val = 0;
>
> @@ -223,27 +223,36 @@ static __always_inline void res_spin_unlock(rqspinlock_t *lock)
> #define raw_res_spin_lock_init(lock) ({ *(lock) = (rqspinlock_t){0}; })
> #endif
>
> -#define raw_res_spin_lock(lock) \
> - ({ \
> - int __ret; \
> - preempt_disable(); \
> - __ret = res_spin_lock(lock); \
> - if (__ret) \
> - preempt_enable(); \
> - __ret; \
> - })
> +static __always_inline __must_check int raw_res_spin_lock(rqspinlock_t *lock)
> +{
> + int ret;
> +
> + preempt_disable();
> + ret = res_spin_lock(lock);
> + if (ret)
> + preempt_enable();
> +
> + return ret;
> +}
>
> #define raw_res_spin_unlock(lock) ({ res_spin_unlock(lock); preempt_enable(); })
>
> -#define raw_res_spin_lock_irqsave(lock, flags) \
> - ({ \
> - int __ret; \
> - local_irq_save(flags); \
> - __ret = raw_res_spin_lock(lock); \
> - if (__ret) \
> - local_irq_restore(flags); \
> - __ret; \
> - })
> +static __always_inline __must_check int
> +__raw_res_spin_lock_irqsave(rqspinlock_t *lock, unsigned long *flags)
> +{
> + unsigned long __flags;
> + int ret;
> +
> + local_irq_save(__flags);
> + ret = raw_res_spin_lock(lock);
> + if (ret)
> + local_irq_restore(__flags);
> +
> + *flags = __flags;
> + return ret;
> +}
> +
> +#define raw_res_spin_lock_irqsave(lock, flags) __raw_res_spin_lock_irqsave(lock, &flags)
The __must_check annotations will cause compiler warnings in
kernel/locking/locktorture.c where torture_raw_res_spin_write_lock()
and torture_raw_res_spin_write_lock_irq() ignore lock acquisition
failures.
When raw_res_spin_lock() returns an error (deadlock or timeout), the
torture test functions still return 0 to indicate success. The torture
framework then calls the unlock functions, but the lock was never
acquired. In res_spin_unlock(), this causes an out-of-bounds array
access when computing locks[cnt - 1] with cnt=0, potentially corrupting
per-CPU memory.
Should the torture test handle lock acquisition failures, or use
different lock types that cannot fail?
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19583558278
Powered by blists - more mailing lists