All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf] bpf: roll back stream capacity when allocation fails
@ 2026-07-20  1:23 Jianlin Shi
  2026-07-20  1:35 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Jianlin Shi @ 2026-07-20  1:23 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, memxor, linux-kernel

bpf_stream_push_str() accounts the string length before allocating a
stream element. If the allocation fails, the length remains charged even
though no element is queued and therefore cannot be released by a reader.
Repeated failures can exhaust the stream capacity permanently until the
BPF program is freed.

Roll back the capacity charge when creating the stream element fails.

Fixes: 5ab154f1463a ("bpf: Introduce BPF standard streams")
Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
 kernel/bpf/stream.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
index be9ce98e9..4b8a74b91 100644
--- a/kernel/bpf/stream.c
+++ b/kernel/bpf/stream.c
@@ -79,7 +79,14 @@ static int bpf_stream_push_str(struct bpf_stream *stream, const char *str, int l
 {
 	int ret = bpf_stream_consume_capacity(stream, len);
 
-	return ret ?: __bpf_stream_push_str(&stream->log, str, len);
+	if (ret)
+		return ret;
+
+	ret = __bpf_stream_push_str(&stream->log, str, len);
+	if (ret)
+		atomic_sub(len, &stream->capacity);
+
+	return ret;
 }
 
 static struct bpf_stream *bpf_stream_get(enum bpf_stream_id stream_id, struct bpf_prog_aux *aux)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf] bpf: roll back stream capacity when allocation fails
  2026-07-20  1:23 [PATCH bpf] bpf: roll back stream capacity when allocation fails Jianlin Shi
@ 2026-07-20  1:35 ` sashiko-bot
  2026-07-20  3:40 ` Pu Lehui
  2026-07-20  5:13 ` [PATCH bpf-next v2] " Jianlin Shi
  2 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-07-20  1:35 UTC (permalink / raw)
  To: Jianlin Shi; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch addresses a capacity leak in `bpf_stream_push_str()` when stream element allocation fails, but entirely misses the identical capacity leak in the staging logic (`bpf_stream_stage_printk()` and `bpf_stream_stage_commit()`).
- [High] If copy_to_user() fails during bpf_stream_read() after some elements have already been consumed, the function returns -EFAULT instead of the number of bytes successfully read, resulting in silent data loss.
--

--- Patch [1]: [PATCH bpf] bpf: roll back stream capacity when allocation fails ---
commit 06c82cbb65aa09c192205a6718b0fa398300f1ac
Author: Jianlin Shi <shijianlin11@foxmail.com>

bpf: roll back stream capacity when allocation fails

The patch fixes a stream capacity leak in bpf_stream_push_str().
If allocating a stream element fails, the previously charged capacity
is now rolled back to avoid permanent exhaustion.

> diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
> index be9ce98e94692..4b8a74b9162e1 100644
> --- a/kernel/bpf/stream.c
> +++ b/kernel/bpf/stream.c
> @@ -79,7 +79,14 @@ static int bpf_stream_push_str(struct bpf_stream *stream, const char *str, int l
>  {
>  	int ret = bpf_stream_consume_capacity(stream, len);
>  
> -	return ret ?: __bpf_stream_push_str(&stream->log, str, len);
> +	if (ret)
> +		return ret;
> +
> +	ret = __bpf_stream_push_str(&stream->log, str, len);
> +	if (ret)
> +		atomic_sub(len, &stream->capacity);
> +
> +	return ret;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but it seems there's an identical
capacity leak in the staging logic that this patch doesn't address.

In bpf_stream_stage_printk(), ss->len is unconditionally incremented even if
allocation fails:

kernel/bpf/stream.c:bpf_stream_stage_printk() {
    ...
    ss->len += ret;
    /* Exclude NULL byte during push. */
    ret = __bpf_stream_push_str(&ss->log, buf->buf, ret);
    ...
}

Then in include/linux/bpf.h, the bpf_stream_stage() macro unconditionally
calls bpf_stream_stage_commit():

#define bpf_stream_stage(ss, prog, stream_id, expr)            \
    ({                                                     \
        bpf_stream_stage_init(&ss);                    \
        (expr);                                        \
        bpf_stream_stage_commit(&ss, prog, stream_id); \
        bpf_stream_stage_free(&ss);                    \
    })

Which then permanently charges the falsely inflated length against the capacity:

kernel/bpf/stream.c:bpf_stream_stage_commit() {
    ...
    ret = bpf_stream_consume_capacity(stream, ss->len);
    ...
}

Should the staging logic also be updated to prevent permanently leaking
stream capacity when memory allocation fails?

[Severity: High]
This is a pre-existing issue, but I noticed another problem related to how
stream errors are handled in bpf_stream_read() when copy_to_user() fails.

If some elements are successfully consumed and freed:

kernel/bpf/stream.c:bpf_stream_read() {
    ...
    bpf_stream_backlog_pop(stream);
    bpf_stream_release_capacity(stream, elem);
    bpf_stream_free_elem(elem);
    ...
}

But a subsequent copy_to_user() fails on an unmapped page, the function returns
-EFAULT unconditionally:

    ...
    mutex_unlock(&stream->lock);
    return ret ? ret : len - rem_len;
}

Does this mean the successfully copied elements from earlier loop iterations
are permanently lost without the reader knowing how many bytes were read?
Should it return the bytes successfully read (len - rem_len) when some
elements were already consumed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_E69EAE29327E25B3548A9AF3F4FA289A6806@qq.com?part=1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf] bpf: roll back stream capacity when allocation fails
  2026-07-20  1:23 [PATCH bpf] bpf: roll back stream capacity when allocation fails Jianlin Shi
  2026-07-20  1:35 ` sashiko-bot
@ 2026-07-20  3:40 ` Pu Lehui
  2026-07-20  5:13   ` Jianlin Shi
  2026-07-20  5:13 ` [PATCH bpf-next v2] " Jianlin Shi
  2 siblings, 1 reply; 20+ messages in thread
From: Pu Lehui @ 2026-07-20  3:40 UTC (permalink / raw)
  To: Jianlin Shi, bpf; +Cc: ast, daniel, andrii, memxor, linux-kernel


On 2026/7/20 9:23, Jianlin Shi wrote:
> bpf_stream_push_str() accounts the string length before allocating a
> stream element. If the allocation fails, the length remains charged even
> though no element is queued and therefore cannot be released by a reader.
> Repeated failures can exhaust the stream capacity permanently until the
> BPF program is freed.
> 
> Roll back the capacity charge when creating the stream element fails.
> 
> Fixes: 5ab154f1463a ("bpf: Introduce BPF standard streams")
> Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
> ---
>   kernel/bpf/stream.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
> index be9ce98e9..4b8a74b91 100644
> --- a/kernel/bpf/stream.c
> +++ b/kernel/bpf/stream.c
> @@ -79,7 +79,14 @@ static int bpf_stream_push_str(struct bpf_stream *stream, const char *str, int l
>   {
>   	int ret = bpf_stream_consume_capacity(stream, len);
>   
> -	return ret ?: __bpf_stream_push_str(&stream->log, str, len);
> +	if (ret)
> +		return ret;
> +
> +	ret = __bpf_stream_push_str(&stream->log, str, len);
> +	if (ret)
> +		atomic_sub(len, &stream->capacity);
> +
> +	return ret;
>   }
>   
>   static struct bpf_stream *bpf_stream_get(enum bpf_stream_id stream_id, struct bpf_prog_aux *aux)

looks good to me, but it might be better to target bpf-next, as this 
isn't a critical issue.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf] bpf: roll back stream capacity when allocation fails
  2026-07-20  3:40 ` Pu Lehui
@ 2026-07-20  5:13   ` Jianlin Shi
  0 siblings, 0 replies; 20+ messages in thread
From: Jianlin Shi @ 2026-07-20  5:13 UTC (permalink / raw)
  To: pulehui; +Cc: bpf, ast, daniel, andrii, memxor, linux-kernel

On 2026/7/20 11:40, Pu Lehui wrote:
> looks good to me, but it might be better to target bpf-next, as this
> isn't a critical issue.

Thanks for the review. Will resend as v2 targeting bpf-next.

Thanks,
Jianlin


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v2] bpf: roll back stream capacity when allocation fails
  2026-07-20  1:23 [PATCH bpf] bpf: roll back stream capacity when allocation fails Jianlin Shi
  2026-07-20  1:35 ` sashiko-bot
  2026-07-20  3:40 ` Pu Lehui
@ 2026-07-20  5:13 ` Jianlin Shi
  2026-07-20  5:26   ` sashiko-bot
                     ` (5 more replies)
  2 siblings, 6 replies; 20+ messages in thread
From: Jianlin Shi @ 2026-07-20  5:13 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, memxor, pulehui, linux-kernel

bpf_stream_push_str() accounts the string length before allocating a
stream element. If the allocation fails, the length remains charged even
though no element is queued and therefore cannot be released by a reader.
Repeated failures can exhaust the stream capacity permanently until the
BPF program is freed.

Roll back the capacity charge when creating the stream element fails.

Fixes: 5ab154f1463a ("bpf: Introduce BPF standard streams")
Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
v2:
- Retarget to bpf-next as suggested by Pu Lehui.

 kernel/bpf/stream.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
index be9ce98e9..4b8a74b91 100644
--- a/kernel/bpf/stream.c
+++ b/kernel/bpf/stream.c
@@ -79,7 +79,14 @@ static int bpf_stream_push_str(struct bpf_stream *stream, const char *str, int l
 {
 	int ret = bpf_stream_consume_capacity(stream, len);
 
-	return ret ?: __bpf_stream_push_str(&stream->log, str, len);
+	if (ret)
+		return ret;
+
+	ret = __bpf_stream_push_str(&stream->log, str, len);
+	if (ret)
+		atomic_sub(len, &stream->capacity);
+
+	return ret;
 }
 
 static struct bpf_stream *bpf_stream_get(enum bpf_stream_id stream_id, struct bpf_prog_aux *aux)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v2] bpf: roll back stream capacity when allocation fails
  2026-07-20  5:13 ` [PATCH bpf-next v2] " Jianlin Shi
@ 2026-07-20  5:26   ` sashiko-bot
  2026-07-27  3:40   ` Jianlin Shi
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-07-20  5:26 UTC (permalink / raw)
  To: Jianlin Shi; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch fixes a capacity leak in `bpf_stream_push_str()` when stream element allocation fails, but misses an identical leak in `bpf_stream_stage_printk()`.
- [High] `bpf_stream_read` drops successfully read data if a subsequent `copy_to_user` fails, violating API contracts.
- [Medium] Log messages are entirely dropped and return an error instead of pushing the truncated message when the formatted output exceeds the internal string buffer size.
--

--- Patch [1]: [PATCH bpf-next v2] bpf: roll back stream capacity when allocation fails ---
commit 3fa8598d2b8afa51091cf463bee21d3c9f140975
Author: Jianlin Shi <shijianlin11@foxmail.com>

bpf: roll back stream capacity when allocation fails

bpf_stream_push_str() accounts the string length before allocating a
stream element. If the allocation fails, the length remains charged even
though no element is queued and therefore cannot be released by a reader.
Repeated failures can exhaust the stream capacity permanently until the
BPF program is freed.

Roll back the capacity charge when creating the stream element fails.

> diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
> index be9ce98e94692..4b8a74b9162e1 100644
> --- a/kernel/bpf/stream.c
> +++ b/kernel/bpf/stream.c
> @@ -79,7 +79,14 @@ static int bpf_stream_push_str(struct bpf_stream *stream, const char *str, int l
>  {
>  	int ret = bpf_stream_consume_capacity(stream, len);
>  
> -	return ret ?: __bpf_stream_push_str(&stream->log, str, len);
> +	if (ret)
> +		return ret;
> +
> +	ret = __bpf_stream_push_str(&stream->log, str, len);
> +	if (ret)
> +		atomic_sub(len, &stream->capacity);
> +
> +	return ret;
>  }

[Severity: High]
This is a pre-existing issue, but does bpf_stream_stage_printk() suffer from
the identical capacity leak that this patch fixes here?

Looking at bpf_stream_stage_printk():

kernel/bpf/stream.c:bpf_stream_stage_printk() {
    ...
	ss->len += ret;
	/* Exclude NULL byte during push. */
	ret = __bpf_stream_push_str(&ss->log, buf->buf, ret);
    ...
}

If __bpf_stream_push_str() fails to allocate memory, ss->len remains incremented.
Later, bpf_stream_stage_commit() unconditionally consumes this erroneously
inflated ss->len from the stream capacity:

kernel/bpf/stream.c:bpf_stream_stage_commit() {
    ...
	ret = bpf_stream_consume_capacity(stream, ss->len);
	if (ret)
		return ret;
    ...
}

Since the failed element isn't in the log, its length will never be released,
which could permanently leak stream capacity over time.

[Severity: Medium]
This is a pre-existing issue, but what happens if the formatted output exceeds
the internal string buffer size?

Both bstr_printf() and vsnprintf() return the number of characters that would
have been written, not the actual number written.

For example, in bpf_stream_vprintk():

kernel/bpf/stream.c:bpf_stream_vprintk() {
    ...
	ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt__str, data.bin_args);
	/* Exclude NULL byte during push. */
	ret = bpf_stream_push_str(stream, data.buf, ret);
    ...
}

If the string is truncated, the un-capped return value is passed as len to
bpf_stream_push_str(). Since bpf_stream_elem_alloc() has a strict bounds check,
it will reject the allocation and return -ENOMEM.

Does this mean the entire log message is dropped instead of safely pushing the
truncated message prefix?

[Severity: High]
This is a pre-existing issue, but does bpf_stream_read() permanently drop data
if a subsequent copy_to_user() fails?

In bpf_stream_read():

kernel/bpf/stream.c:bpf_stream_read() {
    ...
		/* Restore in case of error. */
		if (ret) {
			ret = -EFAULT;
			elem->consumed_len = cons_len;
			break;
		}
    ...
	return ret ? ret : len - rem_len;
}

If copy_to_user() fails on a subsequent element in the loop, it breaks and
unconditionally returns -EFAULT.

Since elements that were successfully read in previous iterations have already
been popped and freed, returning an error instead of the number of successfully
read bytes hides the fact that partial data was consumed. 

Will retrying the read operation cause the previously read data to be
permanently lost?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_C919BB32458A4DAD645A68F441345B971E05@qq.com?part=1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v2] bpf: roll back stream capacity when allocation fails
  2026-07-20  5:13 ` [PATCH bpf-next v2] " Jianlin Shi
  2026-07-20  5:26   ` sashiko-bot
@ 2026-07-27  3:40   ` Jianlin Shi
  2026-07-27  3:58   ` Pu Lehui
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Jianlin Shi @ 2026-07-27  3:40 UTC (permalink / raw)
  To: bpf; +Cc: ast, daniel, andrii, memxor, pulehui, linux-kernel

Friendly ping on v2.

Regarding the related capacity leak in bpf_stream_stage_printk(),
I can send that as a separate follow-up once this patch is settled.

Thanks,
Jianlin


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v2] bpf: roll back stream capacity when allocation fails
  2026-07-20  5:13 ` [PATCH bpf-next v2] " Jianlin Shi
  2026-07-20  5:26   ` sashiko-bot
  2026-07-27  3:40   ` Jianlin Shi
@ 2026-07-27  3:58   ` Pu Lehui
  2026-08-03  2:40   ` Kumar Kartikeya Dwivedi
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Pu Lehui @ 2026-07-27  3:58 UTC (permalink / raw)
  To: Jianlin Shi, bpf; +Cc: ast, daniel, andrii, memxor, linux-kernel


On 2026/7/20 13:13, Jianlin Shi wrote:
> bpf_stream_push_str() accounts the string length before allocating a
> stream element. If the allocation fails, the length remains charged even
> though no element is queued and therefore cannot be released by a reader.
> Repeated failures can exhaust the stream capacity permanently until the
> BPF program is freed.
> 
> Roll back the capacity charge when creating the stream element fails.
> 
> Fixes: 5ab154f1463a ("bpf: Introduce BPF standard streams")
> Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
> ---
> v2:
> - Retarget to bpf-next as suggested by Pu Lehui.
> 
>   kernel/bpf/stream.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
> index be9ce98e9..4b8a74b91 100644
> --- a/kernel/bpf/stream.c
> +++ b/kernel/bpf/stream.c
> @@ -79,7 +79,14 @@ static int bpf_stream_push_str(struct bpf_stream *stream, const char *str, int l
>   {
>   	int ret = bpf_stream_consume_capacity(stream, len);
>   
> -	return ret ?: __bpf_stream_push_str(&stream->log, str, len);
> +	if (ret)
> +		return ret;
> +
> +	ret = __bpf_stream_push_str(&stream->log, str, len);
> +	if (ret)
> +		atomic_sub(len, &stream->capacity);
> +
> +	return ret;
>   }
>   
>   static struct bpf_stream *bpf_stream_get(enum bpf_stream_id stream_id, struct bpf_prog_aux *aux)


Reviewed-by: Pu Lehui <pulehui@huawei.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v2] bpf: roll back stream capacity when allocation fails
  2026-07-20  5:13 ` [PATCH bpf-next v2] " Jianlin Shi
                     ` (2 preceding siblings ...)
  2026-07-27  3:58   ` Pu Lehui
@ 2026-08-03  2:40   ` Kumar Kartikeya Dwivedi
  2026-08-06 15:49     ` Jianlin Shi
  2026-08-15  8:19   ` [PATCH bpf-next v3 0/5] bpf: fix stream capacity, read, and oversize handling Jianlin Shi
       [not found]   ` <20260815081920.609936-1-shijianlin11@foxmail.com>
  5 siblings, 1 reply; 20+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-08-03  2:40 UTC (permalink / raw)
  To: Jianlin Shi, bpf; +Cc: ast, daniel, andrii, pulehui, linux-kernel

On Mon Jul 20, 2026 at 7:13 AM CEST, Jianlin Shi wrote:
> bpf_stream_push_str() accounts the string length before allocating a
> stream element. If the allocation fails, the length remains charged even
> though no element is queued and therefore cannot be released by a reader.
> Repeated failures can exhaust the stream capacity permanently until the
> BPF program is freed.
>
> Roll back the capacity charge when creating the stream element fails.
>
> Fixes: 5ab154f1463a ("bpf: Introduce BPF standard streams")
> Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
> ---

Sorry about the delay. This particular fix is correct, but I'd prefer if we
could refactor bpf_stream_release_capacity() to accept length as its second
parameter and then use it here, to drop dependency on 'elem'. That also better
mirrors the consume side.

As for other Sashiko concerns, since you will respin, you can append more
patches to the series.

Regarding the three issues pointed out by Sashiko in reply to v2, they all look
valid to me. For the staging leak, ss->len should only be increased after
__bpf_stream_push_str() succeeds. We shoulf fix that.

On the read side, account for any bytes successfully copied before a fault and
return that byte count, returning -EFAULT only when no bytes were copied. So a
partial buffer which allows copying some bytes successfully and fails the rest,
we should return the partial count of bytes copied.

For oversized formatted output, keeping the current behavior of dropping the
message seems reasonable, but -ENOMEM is misleading. Since the formatter's
return value excludes the trailing NUL and a value greater than or equal to
MAX_BPRINTF_BUF indicates truncation, please reject such lengths with -E2BIG,
preferably before charging stream capacity. This also fixes the boundary case
where a length of MAX_BPRINTF_BUF currently copies the trailing NUL into the
stream.

So use the actual formatted length, capped at MAX_BPRINTF_BUF - 1 (vscnprintf()
for the staged path), so oversized messages are truncated without copying the
trailing NUL.

Please also add selftests for such cases. For the partial unmapped buffer, you
could map a page and pass a buffer straddling the page boundary, such that we
take a fault when copying into the remainder.

pw-bot: cr

> v2:
> - Retarget to bpf-next as suggested by Pu Lehui.
>
>  kernel/bpf/stream.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
> index be9ce98e9..4b8a74b91 100644
> --- a/kernel/bpf/stream.c
> +++ b/kernel/bpf/stream.c
> @@ -79,7 +79,14 @@ static int bpf_stream_push_str(struct bpf_stream *stream, const char *str, int l
>  {
>  	int ret = bpf_stream_consume_capacity(stream, len);
>
> -	return ret ?: __bpf_stream_push_str(&stream->log, str, len);
> +	if (ret)
> +		return ret;
> +
> +	ret = __bpf_stream_push_str(&stream->log, str, len);
> +	if (ret)
> +		atomic_sub(len, &stream->capacity);
> +
> +	return ret;
>  }
>
>  static struct bpf_stream *bpf_stream_get(enum bpf_stream_id stream_id, struct bpf_prog_aux *aux)


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v2] bpf: roll back stream capacity when allocation fails
  2026-08-03  2:40   ` Kumar Kartikeya Dwivedi
@ 2026-08-06 15:49     ` Jianlin Shi
  0 siblings, 0 replies; 20+ messages in thread
From: Jianlin Shi @ 2026-08-06 15:49 UTC (permalink / raw)
  To: memxor; +Cc: bpf, ast, daniel, andrii, pulehui, linux-kernel

On Mon, 03 Aug 2026 04:40:50 +0200 Kumar Kartikeya Dwivedi wrote:
> Sorry about the delay. This particular fix is correct, but I'd prefer if we
> could refactor bpf_stream_release_capacity() to accept length as its second
> parameter and then use it here, to drop dependency on 'elem'. That also better
> mirrors the consume side.
>
> As for other Sashiko concerns, since you will respin, you can append more
> patches to the series.
>
> [...]
>
> pw-bot: cr

Thanks for the review.  I will address the feedback, including the
release_capacity() refactor and the additional issues as follow-up
patches, and send a v3 series shortly.

Thanks,
Jianlin


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 0/5] bpf: fix stream capacity, read, and oversize handling
  2026-07-20  5:13 ` [PATCH bpf-next v2] " Jianlin Shi
                     ` (3 preceding siblings ...)
  2026-08-03  2:40   ` Kumar Kartikeya Dwivedi
@ 2026-08-15  8:19   ` Jianlin Shi
       [not found]   ` <20260815081920.609936-1-shijianlin11@foxmail.com>
  5 siblings, 0 replies; 20+ messages in thread
From: Jianlin Shi @ 2026-08-15  8:19 UTC (permalink / raw)
  To: bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, emil, memxor,
	pulehui, shuah, linux-kselftest, linux-kernel

v2 of the capacity-rollback fix was correct for bpf_stream_push_str(),
but review asked to:

1. refactor bpf_stream_release_capacity() to take a length;
2. fix the staging-path capacity leak and use vscnprintf();
3. return partial bpf_stream_read() progress on copy_to_user() fault;
4. reject truncated bpf_stream_vprintk() output with -E2BIG before
   charging capacity;
5. add selftests for the oversize and straddling-buffer cases.

This series addresses those points.

Tested locally:
  stream_oversize and stream_partial_read (equivalent to
  ./test_progs -t stream_oversize,stream_partial_read).

v2: https://lore.kernel.org/bpf/tencent_C919BB32458A4DAD645A68F441345B971E05@qq.com/
v1: https://lore.kernel.org/bpf/tencent_E69EAE29327E25B3548A9AF3F4FA289A6806@qq.com/

Jianlin Shi (5):
  bpf: roll back stream capacity when allocation fails
  bpf: fix stream capacity leak in staging path
  bpf: return partial progress from bpf_stream_read on fault
  bpf: reject oversized bpf_stream_vprintk output with -E2BIG
  selftests/bpf: cover stream capacity and partial read edge cases

 kernel/bpf/stream.c                           | 54 ++++++++------
 .../testing/selftests/bpf/prog_tests/stream.c | 70 +++++++++++++++++++
 tools/testing/selftests/bpf/progs/stream.c    | 12 ++++
 3 files changed, 116 insertions(+), 20 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 1/5] bpf: roll back stream capacity when allocation fails
       [not found]   ` <20260815081920.609936-1-shijianlin11@foxmail.com>
@ 2026-08-15  8:19     ` Jianlin Shi
  2026-08-15  8:32       ` sashiko-bot
  2026-08-15  8:19     ` [PATCH bpf-next v3 2/5] bpf: fix stream capacity leak in staging path Jianlin Shi
                       ` (3 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Jianlin Shi @ 2026-08-15  8:19 UTC (permalink / raw)
  To: bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, emil, memxor,
	pulehui, shuah, linux-kselftest, linux-kernel

bpf_stream_push_str() accounts the string length before allocating a
stream element. If the allocation fails, the length remains charged even
though no element is queued and therefore cannot be released by a reader.
Repeated failures can exhaust the stream capacity permanently until the
BPF program is freed.

Refactor bpf_stream_release_capacity() to take a length so the consume
and release sides are symmetric, and use it to roll back the charge when
creating the stream element fails.

Fixes: 5ab154f1463a ("bpf: Introduce BPF standard streams")
Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
 kernel/bpf/stream.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
index be9ce98e9469..0b157ec4e38e 100644
--- a/kernel/bpf/stream.c
+++ b/kernel/bpf/stream.c
@@ -68,10 +68,8 @@ static int bpf_stream_consume_capacity(struct bpf_stream *stream, int len)
 	return 0;
 }
 
-static void bpf_stream_release_capacity(struct bpf_stream *stream, struct bpf_stream_elem *elem)
+static void bpf_stream_release_capacity(struct bpf_stream *stream, int len)
 {
-	int len = elem->total_len;
-
 	atomic_sub(len, &stream->capacity);
 }
 
@@ -79,7 +77,14 @@ static int bpf_stream_push_str(struct bpf_stream *stream, const char *str, int l
 {
 	int ret = bpf_stream_consume_capacity(stream, len);
 
-	return ret ?: __bpf_stream_push_str(&stream->log, str, len);
+	if (ret)
+		return ret;
+
+	ret = __bpf_stream_push_str(&stream->log, str, len);
+	if (ret)
+		bpf_stream_release_capacity(stream, len);
+
+	return ret;
 }
 
 static struct bpf_stream *bpf_stream_get(enum bpf_stream_id stream_id, struct bpf_prog_aux *aux)
@@ -188,7 +193,7 @@ static int bpf_stream_read(struct bpf_stream *stream, void __user *buf, int len)
 		if (cont)
 			continue;
 		bpf_stream_backlog_pop(stream);
-		bpf_stream_release_capacity(stream, elem);
+		bpf_stream_release_capacity(stream, elem->total_len);
 		bpf_stream_free_elem(elem);
 	}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 2/5] bpf: fix stream capacity leak in staging path
       [not found]   ` <20260815081920.609936-1-shijianlin11@foxmail.com>
  2026-08-15  8:19     ` [PATCH bpf-next v3 1/5] bpf: roll back stream capacity when allocation fails Jianlin Shi
@ 2026-08-15  8:19     ` Jianlin Shi
  2026-08-15  8:32       ` sashiko-bot
  2026-08-15  9:02       ` bot+bpf-ci
  2026-08-15  8:19     ` [PATCH bpf-next v3 3/5] bpf: return partial progress from bpf_stream_read on fault Jianlin Shi
                       ` (2 subsequent siblings)
  4 siblings, 2 replies; 20+ messages in thread
From: Jianlin Shi @ 2026-08-15  8:19 UTC (permalink / raw)
  To: bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, emil, memxor,
	pulehui, shuah, linux-kselftest, linux-kernel

bpf_stream_stage_printk() increments ss->len before pushing the
formatted string to the staging log. If element allocation fails,
ss->len remains inflated and bpf_stream_stage_commit() permanently
charges the stream capacity for data that was never queued.

Only account the string length after a successful push, and use
vscnprintf() so the staged length is the truncated payload without
the trailing NUL.

Fixes: 5ab154f1463a ("bpf: Introduce BPF standard streams")
Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
 kernel/bpf/stream.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
index 0b157ec4e38e..ef5a7b06ffcb 100644
--- a/kernel/bpf/stream.c
+++ b/kernel/bpf/stream.c
@@ -316,17 +316,19 @@ int bpf_stream_stage_printk(struct bpf_stream_stage *ss, const char *fmt, ...)
 {
 	struct bpf_bprintf_buffers *buf;
 	va_list args;
-	int ret;
+	int len, ret;
 
 	if (bpf_try_get_buffers(&buf))
 		return -EBUSY;
 
 	va_start(args, fmt);
-	ret = vsnprintf(buf->buf, ARRAY_SIZE(buf->buf), fmt, args);
+	/* Cap to the written length, excluding the trailing NUL. */
+	len = vscnprintf(buf->buf, ARRAY_SIZE(buf->buf), fmt, args);
 	va_end(args);
-	ss->len += ret;
 	/* Exclude NULL byte during push. */
-	ret = __bpf_stream_push_str(&ss->log, buf->buf, ret);
+	ret = __bpf_stream_push_str(&ss->log, buf->buf, len);
+	if (!ret)
+		ss->len += len;
 	bpf_put_buffers();
 	return ret;
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 3/5] bpf: return partial progress from bpf_stream_read on fault
       [not found]   ` <20260815081920.609936-1-shijianlin11@foxmail.com>
  2026-08-15  8:19     ` [PATCH bpf-next v3 1/5] bpf: roll back stream capacity when allocation fails Jianlin Shi
  2026-08-15  8:19     ` [PATCH bpf-next v3 2/5] bpf: fix stream capacity leak in staging path Jianlin Shi
@ 2026-08-15  8:19     ` Jianlin Shi
  2026-08-15  8:19     ` [PATCH bpf-next v3 4/5] bpf: reject oversized bpf_stream_vprintk output with -E2BIG Jianlin Shi
  2026-08-15  8:19     ` [PATCH bpf-next v3 5/5] selftests/bpf: cover stream capacity and partial read edge cases Jianlin Shi
  4 siblings, 0 replies; 20+ messages in thread
From: Jianlin Shi @ 2026-08-15  8:19 UTC (permalink / raw)
  To: bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, emil, memxor,
	pulehui, shuah, linux-kselftest, linux-kernel

bpf_stream_read() pops and frees stream elements after a successful
copy_to_user(). If a later copy_to_user() fails, it currently restores
only the current element's consumed_len and returns -EFAULT, hiding
bytes already delivered to userspace and making the consumed data
unrecoverable on retry.

On a short copy, keep the successfully copied prefix of the current
element and return the number of bytes copied. Return -EFAULT only when
no bytes were copied for the call.

Fixes: 5ab154f1463a ("bpf: Introduce BPF standard streams")
Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
 kernel/bpf/stream.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
index ef5a7b06ffcb..c1077160074c 100644
--- a/kernel/bpf/stream.c
+++ b/kernel/bpf/stream.c
@@ -167,6 +167,7 @@ static int bpf_stream_read(struct bpf_stream *stream, void __user *buf, int len)
 
 	while (rem_len) {
 		int pos = len - rem_len;
+		int chunk, n;
 		bool cont;
 
 		node = bpf_stream_backlog_peek(stream);
@@ -180,13 +181,14 @@ static int bpf_stream_read(struct bpf_stream *stream, void __user *buf, int len)
 
 		cons_len = elem->consumed_len;
 		cont = bpf_stream_consume_elem(elem, &rem_len) == false;
-
-		ret = copy_to_user(buf + pos, elem->str + cons_len,
-				   elem->consumed_len - cons_len);
-		/* Restore in case of error. */
-		if (ret) {
-			ret = -EFAULT;
-			elem->consumed_len = cons_len;
+		chunk = elem->consumed_len - cons_len;
+
+		n = copy_to_user(buf + pos, elem->str + cons_len, chunk);
+		if (n) {
+			/* Keep any successfully copied bytes; -EFAULT only if none. */
+			elem->consumed_len -= n;
+			rem_len += n;
+			ret = (len == rem_len) ? -EFAULT : 0;
 			break;
 		}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 4/5] bpf: reject oversized bpf_stream_vprintk output with -E2BIG
       [not found]   ` <20260815081920.609936-1-shijianlin11@foxmail.com>
                       ` (2 preceding siblings ...)
  2026-08-15  8:19     ` [PATCH bpf-next v3 3/5] bpf: return partial progress from bpf_stream_read on fault Jianlin Shi
@ 2026-08-15  8:19     ` Jianlin Shi
  2026-08-15  8:19     ` [PATCH bpf-next v3 5/5] selftests/bpf: cover stream capacity and partial read edge cases Jianlin Shi
  4 siblings, 0 replies; 20+ messages in thread
From: Jianlin Shi @ 2026-08-15  8:19 UTC (permalink / raw)
  To: bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, emil, memxor,
	pulehui, shuah, linux-kselftest, linux-kernel

bstr_printf() returns the would-be length excluding the trailing NUL.
When that value is >= MAX_BPRINTF_BUF the message was truncated, but
bpf_stream_push_str() still tried to allocate with the inflated length
and failed with -ENOMEM. The boundary case of exactly MAX_BPRINTF_BUF
could also copy the trailing NUL into the stream element.

Reject such lengths with -E2BIG before charging stream capacity, and
tighten bpf_stream_elem_alloc() to accept only payloads strictly shorter
than the bprintf buffer.

Fixes: 5ab154f1463a ("bpf: Introduce BPF standard streams")
Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
 kernel/bpf/stream.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
index c1077160074c..bd1e98fde4b0 100644
--- a/kernel/bpf/stream.c
+++ b/kernel/bpf/stream.c
@@ -22,11 +22,11 @@ static struct bpf_stream_elem *bpf_stream_elem_alloc(int len)
 	size_t alloc_size;
 
 	/*
-	 * Length denotes the amount of data to be written as part of stream element,
-	 * thus includes '\0' byte. We're capped by how much bpf_bprintf_buffers can
-	 * accomodate, therefore deny allocations that won't fit into them.
+	 * Length is the payload pushed into the stream, excluding the
+	 * trailing NUL of the bprintf buffer. Reject anything that cannot
+	 * fit without copying that NUL into the stream element.
 	 */
-	if (len < 0 || len > max_len)
+	if (len < 0 || len >= max_len)
 		return NULL;
 
 	alloc_size = offsetof(struct bpf_stream_elem, str[len]);
@@ -245,6 +245,11 @@ __bpf_kfunc int bpf_stream_vprintk(int stream_id, const char *fmt__str, const vo
 		return ret;
 
 	ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt__str, data.bin_args);
+	/* Truncation: reject before capacity charge (not -ENOMEM). */
+	if (ret >= MAX_BPRINTF_BUF) {
+		bpf_bprintf_cleanup(&data);
+		return -E2BIG;
+	}
 	/* Exclude NULL byte during push. */
 	ret = bpf_stream_push_str(stream, data.buf, ret);
 	bpf_bprintf_cleanup(&data);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 5/5] selftests/bpf: cover stream capacity and partial read edge cases
       [not found]   ` <20260815081920.609936-1-shijianlin11@foxmail.com>
                       ` (3 preceding siblings ...)
  2026-08-15  8:19     ` [PATCH bpf-next v3 4/5] bpf: reject oversized bpf_stream_vprintk output with -E2BIG Jianlin Shi
@ 2026-08-15  8:19     ` Jianlin Shi
  2026-08-15  8:27       ` sashiko-bot
  4 siblings, 1 reply; 20+ messages in thread
From: Jianlin Shi @ 2026-08-15  8:19 UTC (permalink / raw)
  To: bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, emil, memxor,
	pulehui, shuah, linux-kselftest, linux-kernel

Add coverage for the stream fixes requested on the capacity rollback
series:

- oversized bpf_stream_printk() returns -E2BIG and does not leak
  capacity for a subsequent successful write;
- bpf_prog_stream_read() returns the successfully copied prefix when
  the userspace buffer straddles an unmapped page.

Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
 .../testing/selftests/bpf/prog_tests/stream.c | 70 +++++++++++++++++++
 tools/testing/selftests/bpf/progs/stream.c    | 12 ++++
 2 files changed, 82 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/stream.c b/tools/testing/selftests/bpf/prog_tests/stream.c
index c3cce5c292bd..3ae98e3c4909 100644
--- a/tools/testing/selftests/bpf/prog_tests/stream.c
+++ b/tools/testing/selftests/bpf/prog_tests/stream.c
@@ -58,6 +58,76 @@ void test_stream_syscall(void)
 	stream__destroy(skel);
 }
 
+void test_stream_oversize(void)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, opts);
+	struct stream *skel;
+	int ret, prog_fd;
+	char buf[8] = {};
+
+	skel = stream__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "stream__open_and_load"))
+		return;
+
+	prog_fd = bpf_program__fd(skel->progs.stream_oversize);
+	ret = bpf_prog_test_run_opts(prog_fd, &opts);
+	ASSERT_OK(ret, "oversize run");
+	ASSERT_EQ(opts.retval, -E2BIG, "oversize retval");
+
+	/* Oversized push must not permanently consume capacity. */
+	prog_fd = bpf_program__fd(skel->progs.stream_syscall);
+	ret = bpf_prog_test_run_opts(prog_fd, &opts);
+	ASSERT_OK(ret, "syscall run");
+	ASSERT_OK(opts.retval, "syscall retval");
+
+	ret = bpf_prog_stream_read(prog_fd, BPF_STREAM_STDOUT, buf, sizeof(buf), NULL);
+	ASSERT_EQ(ret, 3, "bytes after oversize");
+	ASSERT_OK(memcmp(buf, "foo", 3), "payload after oversize");
+
+	stream__destroy(skel);
+}
+
+void test_stream_partial_read(void)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, opts);
+	struct stream *skel;
+	int ret, prog_fd;
+	long page_size;
+	char *page, *buf;
+	char rest[8] = {};
+
+	skel = stream__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "stream__open_and_load"))
+		return;
+
+	prog_fd = bpf_program__fd(skel->progs.stream_syscall);
+	ret = bpf_prog_test_run_opts(prog_fd, &opts);
+	ASSERT_OK(ret, "ret");
+	ASSERT_OK(opts.retval, "retval");
+
+	page_size = sysconf(_SC_PAGESIZE);
+	page = mmap(NULL, page_size * 2, PROT_READ | PROT_WRITE,
+		    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	if (!ASSERT_NEQ(page, MAP_FAILED, "mmap")) {
+		stream__destroy(skel);
+		return;
+	}
+	/* Leave only the first page mapped so a straddling copy faults. */
+	ASSERT_OK(munmap(page + page_size, page_size), "munmap second page");
+
+	buf = page + page_size - 1;
+	ret = bpf_prog_stream_read(prog_fd, BPF_STREAM_STDOUT, buf, 3, NULL);
+	ASSERT_EQ(ret, 1, "partial bytes");
+	ASSERT_EQ(buf[0], 'f', "first byte");
+
+	ret = bpf_prog_stream_read(prog_fd, BPF_STREAM_STDOUT, rest, sizeof(rest), NULL);
+	ASSERT_EQ(ret, 2, "remaining bytes");
+	ASSERT_OK(memcmp(rest, "oo", 2), "remaining data");
+
+	munmap(page, page_size);
+	stream__destroy(skel);
+}
+
 static void test_address(struct bpf_program *prog, unsigned long *fault_addr_p)
 {
 	LIBBPF_OPTS(bpf_test_run_opts, opts);
diff --git a/tools/testing/selftests/bpf/progs/stream.c b/tools/testing/selftests/bpf/progs/stream.c
index 6f999ba951a3..6fab9b8b21a0 100644
--- a/tools/testing/selftests/bpf/progs/stream.c
+++ b/tools/testing/selftests/bpf/progs/stream.c
@@ -36,7 +36,12 @@ struct {
 } array SEC(".maps");
 
 #define ENOSPC 28
+#define E2BIG 7
 #define _STR "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+#define _X64 "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+/* 1024 bytes: truncated by bstr_printf, must return -E2BIG. */
+#define _BIG_STR (_X64 _X64 _X64 _X64 _X64 _X64 _X64 _X64 \
+		  _X64 _X64 _X64 _X64 _X64 _X64 _X64 _X64)
 
 int size;
 u64 fault_addr;
@@ -117,6 +122,13 @@ int stream_syscall(void *ctx)
 	return 0;
 }
 
+SEC("syscall")
+__success __retval(-E2BIG)
+int stream_oversize(void *ctx)
+{
+	return bpf_stream_printk(BPF_STDOUT, _BIG_STR);
+}
+
 SEC("syscall")
 __arch_x86_64
 __arch_arm64
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v3 5/5] selftests/bpf: cover stream capacity and partial read edge cases
  2026-08-15  8:19     ` [PATCH bpf-next v3 5/5] selftests/bpf: cover stream capacity and partial read edge cases Jianlin Shi
@ 2026-08-15  8:27       ` sashiko-bot
  0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-08-15  8:27 UTC (permalink / raw)
  To: Jianlin Shi; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `test_stream_oversize` test fails to verify its stated goal because it uses a completely different BPF program to verify the capacity rollback.
--

--- Patch [5]: [PATCH bpf-next v3 5/5] selftests/bpf: cover stream capacity and partial read edge cases ---
commit 168aa19f7a74d7d1bc5cd6a9913bc5c68df1058d
Author: Jianlin Shi <shijianlin11@foxmail.com>

selftests/bpf: cover stream capacity and partial read edge cases

This commit adds test coverage for stream capacity rollback after oversized
writes and partial reads when buffers straddle unmapped pages.

> diff --git a/tools/testing/selftests/bpf/prog_tests/stream.c b/tools/testing/selftests/bpf/prog_tests/stream.c
> index e4e9374309e26..fefe6431dc7c9 100644
> --- a/tools/testing/selftests/bpf/prog_tests/stream.c
> +++ b/tools/testing/selftests/bpf/prog_tests/stream.c
[ ... ]
> @@ -58,6 +58,76 @@ void test_stream_syscall(void)
>  	stream__destroy(skel);
>  }
>  
> +void test_stream_oversize(void)
> +{
> +	LIBBPF_OPTS(bpf_test_run_opts, opts);
> +	struct stream *skel;
> +	int ret, prog_fd;
> +	char buf[8] = {};
> +
> +	skel = stream__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "stream__open_and_load"))
> +		return;
> +
> +	prog_fd = bpf_program__fd(skel->progs.stream_oversize);
> +	ret = bpf_prog_test_run_opts(prog_fd, &opts);
> +	ASSERT_OK(ret, "oversize run");
> +	ASSERT_EQ(opts.retval, -E2BIG, "oversize retval");
> +
> +	/* Oversized push must not permanently consume capacity. */
> +	prog_fd = bpf_program__fd(skel->progs.stream_syscall);

[Severity: Medium]
Does this code verify the capacity rollback correctly?

Because BPF streams are maintained on a per-program basis, switching to
the stream_syscall program means it tests a completely different, pristine
stream that was never affected by the oversized push.

Should the test verify a subsequent successful push using the same program
that performed the oversized push?

> +	ret = bpf_prog_test_run_opts(prog_fd, &opts);
> +	ASSERT_OK(ret, "syscall run");
> +	ASSERT_OK(opts.retval, "syscall retval");
> +
> +	ret = bpf_prog_stream_read(prog_fd, BPF_STREAM_STDOUT, buf, sizeof(buf), NULL);
> +	ASSERT_EQ(ret, 3, "bytes after oversize");
> +	ASSERT_OK(memcmp(buf, "foo", 3), "payload after oversize");
> +
> +	stream__destroy(skel);
> +}
[ ... ]
> diff --git a/tools/testing/selftests/bpf/progs/stream.c b/tools/testing/selftests/bpf/progs/stream.c
> index 00a37933e411d..6aa9a67483500 100644
> --- a/tools/testing/selftests/bpf/progs/stream.c
> +++ b/tools/testing/selftests/bpf/progs/stream.c
[ ... ]
> @@ -118,6 +123,13 @@ int stream_syscall(void *ctx)
>  	return 0;
>  }
>  
> +SEC("syscall")
> +__success __retval(-E2BIG)
> +int stream_oversize(void *ctx)
> +{
> +	return bpf_stream_printk(BPF_STDOUT, _BIG_STR);
> +}

[Severity: Medium]
Since this program returns immediately after the oversized push, it leaves
no way to verify the capacity rollback on the same stream from userspace.

Could this program be updated to perform a subsequent successful push to the
same stream, to correctly validate that the capacity was not permanently
consumed?

> +
>  SEC("syscall")
>  __arch_x86_64
>  __arch_arm64

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815081920.609936-1-shijianlin11@foxmail.com?part=5

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v3 1/5] bpf: roll back stream capacity when allocation fails
  2026-08-15  8:19     ` [PATCH bpf-next v3 1/5] bpf: roll back stream capacity when allocation fails Jianlin Shi
@ 2026-08-15  8:32       ` sashiko-bot
  0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-08-15  8:32 UTC (permalink / raw)
  To: Jianlin Shi; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch addresses a stream capacity leak in `bpf_stream_push_str()` when element allocation fails, but leaves an identical leak in the stream staging API (`bpf_stream_stage_printk`).
- [Medium] Unbounded return values from `vsnprintf()` and `bstr_printf()` are passed directly to stream push operations, leading to inadvertently dropped messages and aborted stack traces.
--

--- Patch [1]: [PATCH bpf-next v3 1/5] bpf: roll back stream capacity when allocation fails ---
commit 057b94c20f902b9cfbc6a855c11361e82913b675
Author: Jianlin Shi <shijianlin11@foxmail.com>

bpf: roll back stream capacity when allocation fails

bpf_stream_push_str() accounts the string length before allocating a
stream element. If the allocation fails, the length remains charged even
though no element is queued and therefore cannot be released by a reader.
Repeated failures can exhaust the stream capacity permanently until the
BPF program is freed.

> diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
> index be9ce98e94692..0b157ec4e38eb 100644
> --- a/kernel/bpf/stream.c
> +++ b/kernel/bpf/stream.c

[ ... ]

> @@ -79,7 +77,14 @@ static int bpf_stream_push_str(struct bpf_stream *stream, const char *str, int l
>  {
>  	int ret = bpf_stream_consume_capacity(stream, len);
>  
> -	return ret ?: __bpf_stream_push_str(&stream->log, str, len);
> +	if (ret)
> +		return ret;
> +
> +	ret = __bpf_stream_push_str(&stream->log, str, len);
> +	if (ret)
> +		bpf_stream_release_capacity(stream, len);
> +
> +	return ret;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but since you are fixing the
capacity leak here, does the stream staging API have an identical leak?

Looking at bpf_stream_stage_printk() in kernel/bpf/stream.c:

    ss->len += ret;
    /* Exclude NULL byte during push. */
    ret = __bpf_stream_push_str(&ss->log, buf->buf, ret);

Here ss->len is unconditionally incremented even if __bpf_stream_push_str()
fails. Later, bpf_stream_stage_commit() consumes this inflated ss->len
from the capacity:

    ret = bpf_stream_consume_capacity(stream, ss->len);

Because the failed element is never added to the stream queue, it will
never be popped by a reader, which permanently leaks the capacity. Should
this be rolled back similarly?

[Severity: Medium]
This is also a pre-existing issue, but while reviewing the length handling,
could the unbounded return values from vsnprintf() and bstr_printf() cause
dropped messages?

In bpf_stream_vprintk():

    ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt__str, data.bin_args);
    /* Exclude NULL byte during push. */
    ret = bpf_stream_push_str(stream, data.buf, ret);

And similarly in bpf_stream_stage_printk():

    ret = vsnprintf(buf->buf, ARRAY_SIZE(buf->buf), fmt, args);
    ...
    ret = __bpf_stream_push_str(&ss->log, buf->buf, ret);

Both vsnprintf() and bstr_printf() return the unbounded number of
characters that would have been written, even if truncated. If this length
exceeds MAX_BPRINTF_BUF (1024 bytes), passing it as the length to
bpf_stream_push_str() or __bpf_stream_push_str() will cause
bpf_stream_elem_alloc() to return NULL.

Instead of pushing the successfully truncated string, the entire message
is dropped. Furthermore, in bpf_stream_stage_dump_stack(), this -ENOMEM
return causes the arch_bpf_stack_walk to prematurely abort. Should the
length be capped before being passed to the push functions?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815081920.609936-1-shijianlin11@foxmail.com?part=1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v3 2/5] bpf: fix stream capacity leak in staging path
  2026-08-15  8:19     ` [PATCH bpf-next v3 2/5] bpf: fix stream capacity leak in staging path Jianlin Shi
@ 2026-08-15  8:32       ` sashiko-bot
  2026-08-15  9:02       ` bot+bpf-ci
  1 sibling, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-08-15  8:32 UTC (permalink / raw)
  To: Jianlin Shi; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unbounded memory growth caused by pushing zero-length strings, which completely bypasses the BPF stream capacity limits.
- [Medium] bpf_stream_vprintk() fails to cap the return value of bstr_printf(), causing valid truncated logs to be entirely dropped (returning -ENOMEM) and improperly injecting NUL bytes when lengths match the buffer size.
--

--- Patch [2]: [PATCH bpf-next v3 2/5] bpf: fix stream capacity leak in staging path ---
commit c8e97cb1d20310041c929271dc5ec88dac514fc8
Author: Jianlin Shi <shijianlin11@foxmail.com>
bpf: fix stream capacity leak in staging path

This patch fixes a capacity leak in the staging path of BPF streams by
accounting for the string length only after a successful push. It also
updates the string formatting to use vscnprintf(), correctly capping the
staged length to the truncated payload instead of the theoretical format
length.

> diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
> index 0b157ec4e38eb..ef5a7b06ffcb1 100644
> --- a/kernel/bpf/stream.c
> +++ b/kernel/bpf/stream.c
> @@ -316,17 +316,19 @@ int bpf_stream_stage_printk(struct bpf_stream_stage *ss, const char *fmt, ...)
>  {
>  	struct bpf_bprintf_buffers *buf;
>  	va_list args;
> -	int ret;
> +	int len, ret;
>  
>  	if (bpf_try_get_buffers(&buf))
>  		return -EBUSY;
>  
>  	va_start(args, fmt);
> -	ret = vsnprintf(buf->buf, ARRAY_SIZE(buf->buf), fmt, args);
> +	/* Cap to the written length, excluding the trailing NUL. */
> +	len = vscnprintf(buf->buf, ARRAY_SIZE(buf->buf), fmt, args);
>  	va_end(args);

[Severity: Medium]
This isn't a bug introduced by this patch, but since you are fixing the
length truncation here, I noticed a similar issue in bpf_stream_vprintk()
with bstr_printf():

kernel/bpf/stream.c:bpf_stream_vprintk() {
	...
	ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt__str, data.bin_args);
	/* Exclude NULL byte during push. */
	ret = bpf_stream_push_str(stream, data.buf, ret);
	...
}

Does bstr_printf() return the theoretical untruncated string length, similar
to vsnprintf()? If the string is truncated, couldn't this attempt to push
out-of-bounds capacity or inject NUL bytes into the stream since the return
value is not capped?

> -	ss->len += ret;
>  	/* Exclude NULL byte during push. */
> -	ret = __bpf_stream_push_str(&ss->log, buf->buf, ret);
> +	ret = __bpf_stream_push_str(&ss->log, buf->buf, len);
> +	if (!ret)
> +		ss->len += len;
>  	bpf_put_buffers();
>  	return ret;
>  }

[Severity: High]
This is a pre-existing issue, but I noticed a potential unbounded memory
growth when pushing zero-length strings.

Looking at bpf_stream_consume_capacity() and __bpf_stream_push_str():

kernel/bpf/stream.c:bpf_stream_consume_capacity() {
	...
	if (atomic_add_return(len, &stream->capacity) >= BPF_STREAM_MAX_CAPACITY) {
	...
}

kernel/bpf/stream.c:__bpf_stream_push_str() {
	...
	elem = bpf_stream_elem_alloc(len);
	if (!elem)
		return -ENOMEM;
	...
}

kernel/bpf/stream.c:bpf_stream_elem_alloc() {
	...
	alloc_size = offsetof(struct bpf_stream_elem, str[len]);
	elem = kmalloc_nolock(alloc_size, __GFP_ZERO, -1);
	...
}

If a BPF program repeatedly calls bpf_stream_stage_printk() or
bpf_stream_vprintk() with an empty string format (len = 0),
bpf_stream_consume_capacity() will add 0 to the capacity tracker, bypassing
the BPF_STREAM_MAX_CAPACITY limit entirely.

Meanwhile, __bpf_stream_push_str() still allocates structural memory for the
log element via kmalloc_nolock() even when length is 0.

Could this allow a BPF program to bypass the stream capacity limits and
continuously deplete kernel memory by flooding the log with zero-length
strings?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815081920.609936-1-shijianlin11@foxmail.com?part=2

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v3 2/5] bpf: fix stream capacity leak in staging path
  2026-08-15  8:19     ` [PATCH bpf-next v3 2/5] bpf: fix stream capacity leak in staging path Jianlin Shi
  2026-08-15  8:32       ` sashiko-bot
@ 2026-08-15  9:02       ` bot+bpf-ci
  1 sibling, 0 replies; 20+ messages in thread
From: bot+bpf-ci @ 2026-08-15  9:02 UTC (permalink / raw)
  To: shijianlin11, bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, emil, memxor,
	pulehui, shuah, linux-kselftest, linux-kernel, ast, andrii,
	daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]

> diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
> index 0b157ec4e38e..ef5a7b06ffcb 100644
> --- a/kernel/bpf/stream.c
> +++ b/kernel/bpf/stream.c
> @@ -316,17 +316,19 @@ int bpf_stream_stage_printk(struct bpf_stream_stage *ss, const char *fmt, ...)
>  {
>  	struct bpf_bprintf_buffers *buf;
>  	va_list args;
> -	int ret;
> +	int len, ret;
>
>  	if (bpf_try_get_buffers(&buf))
>  		return -EBUSY;
>
>  	va_start(args, fmt);
> -	ret = vsnprintf(buf->buf, ARRAY_SIZE(buf->buf), fmt, args);
> +	/* Cap to the written length, excluding the trailing NUL. */
> +	len = vscnprintf(buf->buf, ARRAY_SIZE(buf->buf), fmt, args);
>  	va_end(args);
> -	ss->len += ret;
>  	/* Exclude NULL byte during push. */
> -	ret = __bpf_stream_push_str(&ss->log, buf->buf, ret);
> +	ret = __bpf_stream_push_str(&ss->log, buf->buf, len);
> +	if (!ret)
> +		ss->len += len;

This isn't a bug, but with the existing '/* Exclude NULL byte during
push. */' a couple of lines down, does the new comment above vscnprintf()
add anything, or could one of the two be dropped?

>  	bpf_put_buffers();
>  	return ret;
>  }


---
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/31874860107

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2026-08-15  9:02 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-20  1:23 [PATCH bpf] bpf: roll back stream capacity when allocation fails Jianlin Shi
2026-07-20  1:35 ` sashiko-bot
2026-07-20  3:40 ` Pu Lehui
2026-07-20  5:13   ` Jianlin Shi
2026-07-20  5:13 ` [PATCH bpf-next v2] " Jianlin Shi
2026-07-20  5:26   ` sashiko-bot
2026-07-27  3:40   ` Jianlin Shi
2026-07-27  3:58   ` Pu Lehui
2026-08-03  2:40   ` Kumar Kartikeya Dwivedi
2026-08-06 15:49     ` Jianlin Shi
2026-08-15  8:19   ` [PATCH bpf-next v3 0/5] bpf: fix stream capacity, read, and oversize handling Jianlin Shi
     [not found]   ` <20260815081920.609936-1-shijianlin11@foxmail.com>
2026-08-15  8:19     ` [PATCH bpf-next v3 1/5] bpf: roll back stream capacity when allocation fails Jianlin Shi
2026-08-15  8:32       ` sashiko-bot
2026-08-15  8:19     ` [PATCH bpf-next v3 2/5] bpf: fix stream capacity leak in staging path Jianlin Shi
2026-08-15  8:32       ` sashiko-bot
2026-08-15  9:02       ` bot+bpf-ci
2026-08-15  8:19     ` [PATCH bpf-next v3 3/5] bpf: return partial progress from bpf_stream_read on fault Jianlin Shi
2026-08-15  8:19     ` [PATCH bpf-next v3 4/5] bpf: reject oversized bpf_stream_vprintk output with -E2BIG Jianlin Shi
2026-08-15  8:19     ` [PATCH bpf-next v3 5/5] selftests/bpf: cover stream capacity and partial read edge cases Jianlin Shi
2026-08-15  8:27       ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.