[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <0a0eaa11c4b429d41ea2adfc36b56b75b61eacd777dffb2a25cd2857f72d5f2d@mail.kernel.org>
Date: Sat, 31 Jan 2026 05:33:28 +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,martin.lau@...nel.org,kpsingh@...nel.org,yonghong.song@...ux.dev,song@...nel.org,haoluo@...gle.com,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 v4 05/16] bpf: Change local_storage->lock and b->lock to rqspinlock
> diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c
> index 824d05325..7661319ad 100644
> --- a/kernel/bpf/bpf_local_storage.c
> +++ b/kernel/bpf/bpf_local_storage.c
[ ... ]
> @@ -572,7 +591,9 @@ bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
> if (!alloc_selem)
> return ERR_PTR(-ENOMEM);
>
> - raw_spin_lock_irqsave(&local_storage->lock, flags);
> + err = raw_res_spin_lock_irqsave(&local_storage->lock, flags);
> + if (err)
> + return ERR_PTR(err);
If the lock acquisition fails here, doesn't this leak alloc_selem? The
allocation at the top of this function succeeded:
alloc_selem = selem = bpf_selem_alloc(smap, owner, value, ...);
if (!alloc_selem)
return ERR_PTR(-ENOMEM);
But on lock failure, the early return bypasses the cleanup at the end of
the function:
if (alloc_selem) {
mem_uncharge(smap, owner, smap->elem_size);
bpf_selem_free(alloc_selem, true);
}
Should this goto unlock instead of returning directly, or add explicit
cleanup before returning?
[ ... ]
This isn't a bug, but the commit message has a minor wording issue:
> Change bpf_local_storage::lock and bpf_local_storage_map_bucket::lock to
> from raw_spin_lock to rqspinlock.
"to from" should be "from".
> For, __bpf_local_storage_map_cache(), instead of handling the error,
> skip updating the cache.
There's an extra comma after "For".
---
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/21539340647
AI-authorship-score: low
AI-authorship-explanation: The commit follows standard kernel development patterns with appropriate technical detail and no AI-typical phrasing or structure.
issues-found: 1
issue-severity-score: low
issue-severity-explanation: Memory leak on error path in bpf_local_storage_update when rqspinlock times out; affects only rare lock failure scenarios.
Powered by blists - more mailing lists