[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <48114f7cb5d9dbe88299056f69a43708f29f4f4569aa0c04d4411d5140e932c3@mail.kernel.org>
Date: Mon, 9 Feb 2026 03:05:46 +0000 (UTC)
From: bot+bpf-ci@...nel.org
To: ankur.a.arora@...cle.com,linux-kernel@...r.kernel.org,linux-arch@...r.kernel.org,linux-arm-kernel@...ts.infradead.org,linux-pm@...r.kernel.org,bpf@...r.kernel.org
Cc: arnd@...db.de,catalin.marinas@....com,will@...nel.org,peterz@...radead.org,akpm@...ux-foundation.org,mark.rutland@....com,harisokn@...zon.com,cl@...two.org,ast@...nel.org,rafael@...nel.org,daniel.lezcano@...aro.org,memxor@...il.com,zhenglifeng1@...wei.com,xueshuai@...ux.alibaba.com,joao.m.martins@...cle.com,boris.ostrovsky@...cle.com,konrad.wilk@...cle.com,ankur.a.arora@...cle.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 v9 09/12] bpf/rqspinlock: switch check_timeout() to a clock interface
> diff --git a/kernel/bpf/rqspinlock.c b/kernel/bpf/rqspinlock.c
> index 2fdfa828e3d3..f02b1c5f0222 100644
> --- a/kernel/bpf/rqspinlock.c
> +++ b/kernel/bpf/rqspinlock.c
[ ... ]
> @@ -340,6 +349,7 @@ int __lockfunc resilient_queued_spin_lock_slowpath(rqspinlock_t *lock, u32 val)
> {
> struct mcs_spinlock *prev, *next, *node;
> struct rqspinlock_timeout ts;
> + s64 timeout_err = 0;
> int idx, ret = 0;
> u32 old, tail;
[ ... ]
In arch_mcs_spin_lock_contended(), when a waiter in the MCS queue receives
a timeout signal (RES_TIMEOUT_VAL) from the previous waiter:
> if (old & _Q_TAIL_MASK) {
> int val;
>
> prev = decode_tail(old, rqnodes);
>
> /* Link @node into the waitqueue. */
> WRITE_ONCE(prev->next, node);
>
> val = arch_mcs_spin_lock_contended(&node->locked);
> if (val == RES_TIMEOUT_VAL) {
> ret = -ETIMEDOUT;
> goto waitq_timeout;
> }
This sets ret = -ETIMEDOUT but does not set timeout_err. When execution
reaches the waitq_timeout label:
> waitq_timeout:
> - if (ret) {
> + if (timeout_err < 0) {
The condition evaluates to false since timeout_err is still 0. This causes
the timeout handling to be skipped entirely, and execution falls through
to the lock acquisition path. If the lock is acquired, the function
returns ret (which is -ETIMEDOUT), making the caller believe the lock was
not acquired when it actually was. This appears to be a lock leak.
Should the goto waitq_timeout path also set timeout_err = -ETIMEDOUT?
---
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/21810635323
AI-authorship-score: low
AI-authorship-explanation: The commit shows typical human developer patterns including a missed edge case during refactoring, which is a common human error rather than systematic AI-generated code issues.
issues-found: 1
issue-severity-score: high
issue-severity-explanation: Lock leak bug where the function may acquire a lock but return an error code, causing callers to think acquisition failed when it actually succeeded.
Powered by blists - more mailing lists