* [PATCH bpf] bpf: roll back stream capacity when allocation fails
@ 2026-07-20 1:23 Jianlin Shi
2026-07-20 3:40 ` Pu Lehui
2026-07-20 5:13 ` [PATCH bpf-next v2] " Jianlin Shi
0 siblings, 2 replies; 8+ 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] 8+ 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 3:40 ` Pu Lehui
2026-07-20 5:13 ` Jianlin Shi
2026-07-20 5:13 ` [PATCH bpf-next v2] " Jianlin Shi
1 sibling, 1 reply; 8+ 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] 8+ 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; 8+ 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] 8+ 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 3:40 ` Pu Lehui
@ 2026-07-20 5:13 ` Jianlin Shi
2026-07-27 3:40 ` Jianlin Shi
` (2 more replies)
1 sibling, 3 replies; 8+ 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] 8+ 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-27 3:40 ` Jianlin Shi
2026-07-27 3:58 ` Pu Lehui
2026-08-03 2:40 ` Kumar Kartikeya Dwivedi
2 siblings, 0 replies; 8+ 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] 8+ 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-27 3:40 ` Jianlin Shi
@ 2026-07-27 3:58 ` Pu Lehui
2026-08-03 2:40 ` Kumar Kartikeya Dwivedi
2 siblings, 0 replies; 8+ 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] 8+ 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-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
2 siblings, 1 reply; 8+ 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] 8+ 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; 8+ 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] 8+ messages in thread
end of thread, other threads:[~2026-08-06 15:50 UTC | newest]
Thread overview: 8+ 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 3:40 ` Pu Lehui
2026-07-20 5:13 ` Jianlin Shi
2026-07-20 5:13 ` [PATCH bpf-next v2] " Jianlin Shi
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox