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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ