Re: [PATCH bpf-next] bpf: Fix stream capacity leak and spurious -ENOSPC in bpf_stream_stage_commit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Dead code and commit message mismatch regarding a structurally impossible capacity leak.
- [Low] The patch inconsistently drops 0-byte stream commits when `ss->len == 0`.
--

--- Patch [1]: [PATCH bpf-next] bpf: Fix stream capacity leak and spurious -ENOSPC in bpf_stream_stage_commit ---
commit 55dd4746dd7b2e3d562a36b8b50cd0857f9d9bcc
Author: Khawar Ahemad <ahemadkhawar123@gmail.com>

bpf: Fix stream capacity leak and spurious -ENOSPC in bpf_stream_stage_commit

This patch updates bpf_stream_stage_commit() to return early if the staged
length is 0, avoiding a spurious -ENOSPC. It also adds a rollback to release
consumed capacity if the staging list is empty when detached.

> diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
> index 7a5c3ac8676b9..3ad58c82846ba 100644
> --- a/kernel/bpf/stream.c
> +++ b/kernel/bpf/stream.c
> @@ -350,15 +350,21 @@ int bpf_stream_stage_commit(struct bpf_stream_stage *ss, struct bpf_prog *prog,
>  	if (!stream)
>  		return -EINVAL;
>  
> +	if (!ss->len)
> +		return 0;

[Severity: Low]
Does this early return introduce an inconsistency with 0-byte elements?

If a BPF program stages only 0-byte elements, this early return discards them
entirely without transferring them to the stream. However, if a 0-byte
element is staged alongside a >0-byte element, ss->len will be > 0, and the
0-byte element will be successfully pushed to the stream.

> +
>  	ret = bpf_stream_consume_capacity(stream, ss->len);
>  	if (ret)
>  		return ret;
>  
>  	list = llist_del_all(&ss->log);
> -	head = tail = list;
> -
> -	if (!list)
> +	if (!list) {
> +		bpf_stream_release_capacity(stream, ss->len);
>  		return 0;
> +	}

[Severity: Medium]
Is this rollback path actually reachable?

The commit message states this fixes a capacity leak if llist_del_all()
returns NULL. But because of the new if (!ss->len) check above, execution
only reaches here when ss->len > 0. Since ss->len is the sum of the lengths
of elements successfully added to ss->log, having ss->len > 0 seems to
guarantee that ss->log is not empty, meaning list will never be NULL.

Additionally, in the unpatched code, if ss->len was 0,
bpf_stream_consume_capacity() consumed exactly 0 capacity. Returning early
without releasing 0 capacity wouldn't have actually leaked anything.

Could the commit message be clarified, or is this dead code?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825113948.85020-1-ahemadkhawar123@gmail.com?part=1