BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next] bpf: Zero queue and stack outputs on lock failure
@ 2026-07-19 12:54 Kumar Kartikeya Dwivedi
  2026-07-19 13:15 ` sashiko-bot
  2026-07-20 22:44 ` Emil Tsalapatis
  0 siblings, 2 replies; 3+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-07-19 12:54 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Eduard Zingerman, Emil Tsalapatis, kkd, kernel-team

Queue and stack pop/peek helpers accept an uninitialized output buffer
because the verifier expects the helper to initialize it. The empty-map
error path clears the buffer, but a failed lock acquisition returns
-EBUSY without writing it.

Clear the output before returning -EBUSY so BPF programs cannot observe
uninitialized stack contents after a failed helper call.

Fixes: a34a9f1a19af ("bpf: Avoid deadlock when using queue and stack maps from NMI")
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/queue_stack_maps.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/queue_stack_maps.c b/kernel/bpf/queue_stack_maps.c
index 9a5f94371e50..c1c9dee4dcdd 100644
--- a/kernel/bpf/queue_stack_maps.c
+++ b/kernel/bpf/queue_stack_maps.c
@@ -99,8 +99,10 @@ static long __queue_map_get(struct bpf_map *map, void *value, bool delete)
 	int err = 0;
 	void *ptr;

-	if (raw_res_spin_lock_irqsave(&qs->lock, flags))
+	if (raw_res_spin_lock_irqsave(&qs->lock, flags)) {
+		memset(value, 0, qs->map.value_size);
 		return -EBUSY;
+	}

 	if (queue_stack_map_is_empty(qs)) {
 		memset(value, 0, qs->map.value_size);
@@ -130,8 +132,10 @@ static long __stack_map_get(struct bpf_map *map, void *value, bool delete)
 	void *ptr;
 	u32 index;

-	if (raw_res_spin_lock_irqsave(&qs->lock, flags))
+	if (raw_res_spin_lock_irqsave(&qs->lock, flags)) {
+		memset(value, 0, qs->map.value_size);
 		return -EBUSY;
+	}

 	if (queue_stack_map_is_empty(qs)) {
 		memset(value, 0, qs->map.value_size);
--
2.53.0


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

* Re: [PATCH bpf-next] bpf: Zero queue and stack outputs on lock failure
  2026-07-19 12:54 [PATCH bpf-next] bpf: Zero queue and stack outputs on lock failure Kumar Kartikeya Dwivedi
@ 2026-07-19 13:15 ` sashiko-bot
  2026-07-20 22:44 ` Emil Tsalapatis
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-07-19 13:15 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi; +Cc: bpf

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

Pre-existing issues:
- [High] The helper bpf_get_func_arg() fails to initialize its output buffer on error, leading to an information leak of kernel stack memory.
- [High] The helper bpf_get_branch_snapshot() fails to initialize its output buffer on error or partial read, leaking kernel stack memory.
- [High] The helpers bpf_get_task_stack() and bpf_get_task_stack_sleepable() fail to initialize their output buffer on certain error paths.
- [High] The `bpf_dynptr_read` helper accepts an uninitialized output buffer but fails to clear it on error paths, leading to a kernel stack memory leak.
--

--- Patch [1]: [PATCH bpf-next] bpf: Zero queue and stack outputs on lock failure ---
commit 5fdb8c3a145c7d80ff008684d830f42d96eff7e6
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Zero queue and stack outputs on lock failure

This commit fixes an issue where queue and stack pop/peek helpers could
return -EBUSY on lock failure without initializing their output buffers.
It clears the buffer before returning to prevent BPF programs from
observing uninitialized stack contents.

> diff --git a/kernel/bpf/queue_stack_maps.c b/kernel/bpf/queue_stack_maps.c
> index 9a5f94371e506..c1c9dee4dcdd0 100644
> --- a/kernel/bpf/queue_stack_maps.c
> +++ b/kernel/bpf/queue_stack_maps.c
> @@ -99,8 +99,10 @@ static long __queue_map_get(struct bpf_map *map, void *value, bool delete)
>  	int err = 0;
>  	void *ptr;
>  
> -	if (raw_res_spin_lock_irqsave(&qs->lock, flags))
> +	if (raw_res_spin_lock_irqsave(&qs->lock, flags)) {
> +		memset(value, 0, qs->map.value_size);
>  		return -EBUSY;
> +	}
>  
>  	if (queue_stack_map_is_empty(qs)) {
>  		memset(value, 0, qs->map.value_size);
> @@ -130,8 +132,10 @@ static long __stack_map_get(struct bpf_map *map, void *value, bool delete)
>  	void *ptr;
>  	u32 index;
>  
> -	if (raw_res_spin_lock_irqsave(&qs->lock, flags))
> +	if (raw_res_spin_lock_irqsave(&qs->lock, flags)) {
> +		memset(value, 0, qs->map.value_size);
>  		return -EBUSY;
> +	}
>  
>  	if (queue_stack_map_is_empty(qs)) {
>  		memset(value, 0, qs->map.value_size);

[Severity: High]
This is a pre-existing issue, but I noticed a similar pattern in other BPF
helpers while reviewing this fix. For example, does get_func_arg() in 
kernel/trace/bpf_trace.c also leak uninitialized stack memory on its error
path?

kernel/trace/bpf_trace.c:get_func_arg() {
    ...
    if ((u64) n >= nr_args)
        return -EINVAL;
    ...
}

The verifier assumes the MEM_UNINIT output buffer is unconditionally
initialized. If this helper returns -EINVAL without clearing the output
buffer, could a BPF program read the uninitialized memory? It looks like
the JIT inline implementation in kernel/bpf/fixups.c:fixup_bpf_calls()
also jumps over the memory write instruction on failure.

[Severity: High]
This isn't a bug introduced by this patch, but does bpf_get_branch_snapshot()
have the exact same issue with its ARG_PTR_TO_UNINIT_MEM buffer?

kernel/trace/bpf_trace.c:bpf_get_branch_snapshot() {
    ...
    if (unlikely(flags))
        return -EINVAL;
    ...
}

If it returns early due to flags or !entry_cnt, or if
perf_snapshot_branch_stack() returns fewer entries than the requested
buffer size, the trailing bytes of the buffer are left unwritten. Could a
tracing program exploit this to read kernel stack contents?

[Severity: High]
This is a pre-existing issue, but does __bpf_get_task_stack() also need 
similar zeroing on its error paths?

kernel/bpf/stackmap.c:__bpf_get_task_stack() {
    ...
    if (!try_get_task_stack(task))
        return -EFAULT;
    ...
}

Both bpf_get_task_stack() and bpf_get_task_stack_sleepable() use
ARG_PTR_TO_UNINIT_MEM for their buffer. If try_get_task_stack() fails, it
returns an error without clearing the caller-provided uninitialized buffer,
which the verifier assumes is fully initialized.

[Severity: High]
This isn't a bug introduced by this patch, but I see the same bug class
present in __bpf_dynptr_read().

kernel/bpf/helpers.c:__bpf_dynptr_read() {
    ...
    if (!src->data || flags)
        return -EINVAL;
    ...
}

Since the verifier definition for bpf_dynptr_read uses
ARG_PTR_TO_UNINIT_MEM for its destination buffer, returning an error here
without zeroing the dst buffer appears to trick the verifier into marking
uninitialized stack memory as initialized. Could this be used by
unprivileged programs to leak kernel memory?

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

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

* Re: [PATCH bpf-next] bpf: Zero queue and stack outputs on lock failure
  2026-07-19 12:54 [PATCH bpf-next] bpf: Zero queue and stack outputs on lock failure Kumar Kartikeya Dwivedi
  2026-07-19 13:15 ` sashiko-bot
@ 2026-07-20 22:44 ` Emil Tsalapatis
  1 sibling, 0 replies; 3+ messages in thread
From: Emil Tsalapatis @ 2026-07-20 22:44 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi, bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Eduard Zingerman, kkd, kernel-team

On Sun Jul 19, 2026 at 8:54 AM EDT, Kumar Kartikeya Dwivedi wrote:
> Queue and stack pop/peek helpers accept an uninitialized output buffer
> because the verifier expects the helper to initialize it. The empty-map
> error path clears the buffer, but a failed lock acquisition returns
> -EBUSY without writing it.
>
> Clear the output before returning -EBUSY so BPF programs cannot observe
> uninitialized stack contents after a failed helper call.
>
> Fixes: a34a9f1a19af ("bpf: Avoid deadlock when using queue and stack maps from NMI")
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>

> ---
>  kernel/bpf/queue_stack_maps.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/queue_stack_maps.c b/kernel/bpf/queue_stack_maps.c
> index 9a5f94371e50..c1c9dee4dcdd 100644
> --- a/kernel/bpf/queue_stack_maps.c
> +++ b/kernel/bpf/queue_stack_maps.c
> @@ -99,8 +99,10 @@ static long __queue_map_get(struct bpf_map *map, void *value, bool delete)
>  	int err = 0;
>  	void *ptr;
>
> -	if (raw_res_spin_lock_irqsave(&qs->lock, flags))
> +	if (raw_res_spin_lock_irqsave(&qs->lock, flags)) {
> +		memset(value, 0, qs->map.value_size);
>  		return -EBUSY;
> +	}
>
>  	if (queue_stack_map_is_empty(qs)) {
>  		memset(value, 0, qs->map.value_size);
> @@ -130,8 +132,10 @@ static long __stack_map_get(struct bpf_map *map, void *value, bool delete)
>  	void *ptr;
>  	u32 index;
>
> -	if (raw_res_spin_lock_irqsave(&qs->lock, flags))
> +	if (raw_res_spin_lock_irqsave(&qs->lock, flags)) {
> +		memset(value, 0, qs->map.value_size);
>  		return -EBUSY;
> +	}
>
>  	if (queue_stack_map_is_empty(qs)) {
>  		memset(value, 0, qs->map.value_size);
> --
> 2.53.0


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

end of thread, other threads:[~2026-07-20 22:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-19 12:54 [PATCH bpf-next] bpf: Zero queue and stack outputs on lock failure Kumar Kartikeya Dwivedi
2026-07-19 13:15 ` sashiko-bot
2026-07-20 22:44 ` Emil Tsalapatis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox