Skip to content

Conversation

@kernel-patches-daemon-bpf
Copy link

Pull request for series with
subject: bpf: refactor stack map trace depth calculation into helper function
version: 10
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1015803

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: e758657
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1015803
version: 10

@kernel-patches-review-bot
Copy link

commit 6125147de25b820e196b179ca2f7b6f510b34230
Author: Arnaud Lecomte <[email protected]>

bpf: fix stackmap overflow check in __bpf_get_stackid()

This patch adds bounds checking to prevent out-of-bounds writes when
copying stack trace data into stack map buckets. It caps trace_nr to
the bucket capacity and fixes trace->nr restoration in bpf_get_stackid_pe.

Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=c9b724fbb41cf2538b7b
Fixes: ee2a098851bf ("bpf: Adjust BPF stack helper functions to accommodate skip > 0")

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 5e9ad0503..2365541c8 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -251,8 +251,8 @@ static long __bpf_get_stackid(struct bpf_map *map,
>  {
>  	struct bpf_stack_map *smap = container_of(map, struct bpf_stack_map, map);
>  	struct stack_map_bucket *bucket, *new_bucket, *old_bucket;
> +	u32 hash, id, trace_nr, trace_len, i, max_depth;
>  	u32 skip = flags & BPF_F_SKIP_FIELD_MASK;
> -	u32 hash, id, trace_nr, trace_len, i;
>  	bool user = flags & BPF_F_USER_STACK;
>  	u64 *ips;
>  	bool hash_matches;
> @@ -261,7 +261,8 @@ static long __bpf_get_stackid(struct bpf_map *map,
>  		/* skipping more than usable stack trace */
>  		return -EFAULT;
>
> -	trace_nr = trace->nr - skip;
> +	max_depth = stack_map_calculate_max_depth(map->value_size, stack_map_data_size(map), flags);
> +	trace_nr = min_t(u32, trace->nr - skip, max_depth - skip);
                                             ^^^^^^^^^^^^^^^^

Can this underflow when max_depth < skip?

Looking at stack_map_calculate_max_depth():

    max_depth = size / elem_size;
    max_depth += skip;              /* line 60 */
    if (max_depth > curr_sysctl_max_stack)
        return curr_sysctl_max_stack;

If sysctl_perf_event_max_stack is set below the skip value (which can
be up to 255 via BPF_F_SKIP_FIELD_MASK), the function returns a value
smaller than skip. For example:

- BPF program uses skip = 100
- Admin sets sysctl_perf_event_max_stack = 50
- stack_map_calculate_max_depth() returns 50
- max_depth - skip = 50 - 100 underflows to 4294967246
- min_t() becomes ineffective, bypassing the bounds check

This would reintroduce the original overflow bug that this patch is
trying to fix.

[ ... ]

> @@ -390,15 +391,11 @@ BPF_CALL_3(bpf_get_stackid_pe, struct bpf_perf_event_data_kern *, ctx,
>  		return -EFAULT;
>
>  	nr_kernel = count_kernel_ip(trace);
> +	__u64 nr = trace->nr; /* save original */
>
>  	if (kernel) {
> -		__u64 nr = trace->nr;
> -
>  		trace->nr = nr_kernel;
>  		ret = __bpf_get_stackid(map, trace, flags);
> -
> -		/* restore nr */
> -		trace->nr = nr;
>  	} else { /* user */
>  		u64 skip = flags & BPF_F_SKIP_FIELD_MASK;
>
> @@ -409,6 +406,10 @@ BPF_CALL_3(bpf_get_stackid_pe, struct bpf_perf_event_data_kern *, ctx,
>  		flags = (flags & ~BPF_F_SKIP_FIELD_MASK) | skip;
>  		ret = __bpf_get_stackid(map, trace, flags);
>  	}
> +
> +	/* restore nr */
> +	trace->nr = nr;
> +
>  	return ret;
>  }

This change looks correct - it properly restores trace->nr after both
the kernel and user paths.



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

In-Reply-To-Subject: bpf: fix stackmap overflow check in __bpf_get_stackid()
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18807608505

@kernel-patches-daemon-bpf
Copy link
Author

Forwarding comment 3447728721 via email
In-Reply-To: [email protected]
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: ff88079
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1015803
version: 10

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: f9db3a3
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1015803
version: 10

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 8842732
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1015803
version: 10

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 23f852d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1015803
version: 10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant