Re: [PATCH bpf-next v1 3/6] bpf: Separate stream readiness from capacity accounting

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unbounded accumulation of zero-length stream elements leading to memory leak and potential kernel OOM.
--

commit 0e18588b75a5e2877e4350149476217ae3dc124c
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Separate stream readiness from capacity accounting

This commit separates stream capacity accounting from readiness by
introducing a new readable byte counter. The new counter tracks how many
bytes are actually available for userspace to read, while the existing
capacity counter remains solely for enforcing the stream limit.

> diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
> index 99a89533eaef6..b1e6767d8754d 100644
> --- a/kernel/bpf/stream.c
> +++ b/kernel/bpf/stream.c
[ ... ]
> @@ -88,6 +88,21 @@ static void bpf_stream_notify(struct irq_work *work)
>  	wake_up_interruptible_poll(&stream->waitq, EPOLLIN | EPOLLRDNORM);
>  }
>  
> +static int bpf_stream_readable_bytes(struct bpf_stream *stream)
> +{
> +	return atomic_read_acquire(&stream->readable);
> +}
> +
> +static void bpf_stream_publish(struct bpf_stream *stream, int len)
> +{
> +	if (!len)
> +		return;

[Severity: High]
If a 0-length string is pushed, does this early return bypass incrementing
the readable counter? It appears the 0-length element has already been
allocated and added to the log in __bpf_stream_push_str() prior to this.

> +
> +	/* Pairs with atomic_read_acquire() in bpf_stream_readable_bytes(). */
> +	(void)atomic_add_return_release(len, &stream->readable);
> +	irq_work_queue(&stream->notify_work);
> +}
[ ... ]
> @@ -176,14 +191,16 @@ static bool bpf_stream_consume_elem(struct bpf_stream_elem *elem, int *len)
>  
>  static int bpf_stream_read(struct bpf_stream *stream, void __user *buf, int len)
>  {
> -	int rem_len = len, cons_len, ret = 0;
> +	int read_len, rem_len, cons_len, ret = 0;
>  	struct bpf_stream_elem *elem = NULL;
>  	struct llist_node *node;
>  
>  	mutex_lock(&stream->lock);
> +	read_len = min(len, bpf_stream_readable_bytes(stream));
> +	rem_len = read_len;
>  
>  	while (rem_len) {

[Severity: High]
Can this lead to an unbounded accumulation of 0-length elements in the log?

Because 0-length pushes skip incrementing the readable counter in
bpf_stream_publish(), bpf_stream_readable_bytes() can return 0 here. This
makes read_len and rem_len both 0, which entirely skips the while loop.

Since the loop is bypassed, readers cannot drain and free these 0-length
elements. Furthermore, because bpf_stream_consume_capacity() adds 0, the
allocations never hit the stream limit. Could a BPF program repeatedly
pushing empty strings cause a memory leak and potential kernel OOM?

> -		int pos = len - rem_len;
> +		int pos = read_len - rem_len;
>  		int chunk, n;
>  		bool cont;
[ ... ]

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