[PATCH bpf] bpf: roll back stream capacity when allocation fails

[PATCH bpf] bpf: roll back stream capacity when allocation fails

[Jianlin Shi]

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

Re: [PATCH bpf] bpf: roll back stream capacity when allocation fails

[sashiko-bot]

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

Re: [PATCH bpf] bpf: roll back stream capacity when allocation fails

[Pu Lehui]

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.

Re: [PATCH bpf] bpf: roll back stream capacity when allocation fails

[Jianlin Shi]

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

[PATCH bpf-next v2] bpf: roll back stream capacity when allocation fails

[Jianlin Shi]

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

Re: [PATCH bpf-next v2] bpf: roll back stream capacity when allocation fails

[sashiko-bot]

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

Re: [PATCH bpf-next v2] bpf: roll back stream capacity when allocation fails

[Jianlin Shi]

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

Re: [PATCH bpf-next v2] bpf: roll back stream capacity when allocation fails

[Pu Lehui]

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>

Re: [PATCH bpf-next v2] bpf: roll back stream capacity when allocation fails

[Kumar Kartikeya Dwivedi]

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)