linux-kernel - Re: [PATCH bpf-next v4 1/2] perf: Refactor get_perf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <a48e281e6912037490270da40f79e45c1ad1e430c8c1cc84c48306106b06113a@mail.kernel.org>
Date: Tue, 28 Oct 2025 17:09:28 +0000 (UTC)
From: bot+bpf-ci@...nel.org
To: chen.dylane@...ux.dev,peterz@...radead.org,mingo@...hat.com,acme@...nel.org,namhyung@...nel.org,mark.rutland@....com,alexander.shishkin@...ux.intel.com,jolsa@...nel.org,irogers@...gle.com,adrian.hunter@...el.com,kan.liang@...ux.intel.com,song@...nel.org,ast@...nel.org,daniel@...earbox.net,andrii@...nel.org,martin.lau@...ux.dev,eddyz87@...il.com,yonghong.song@...ux.dev,john.fastabend@...il.com,kpsingh@...nel.org,sdf@...ichev.me,haoluo@...gle.com
Cc: linux-perf-users@...r.kernel.org,linux-kernel@...r.kernel.org,bpf@...r.kernel.org,chen.dylane@...ux.dev,ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,martin.lau@...nel.org,eddyz87@...il.com,yonghong.song@...ux.dev,clm@...a.com,ihor.solodrai@...ux.dev
Subject: Re: [PATCH bpf-next v4 1/2] perf: Refactor get_perf_callchain


```
commit 59cfc61f83ad4ec998f4242ca000c554e3001b47
Author: Tao Chen <chen.dylane@...ux.dev>

perf: Refactor get_perf_callchain

This patch refactors get_perf_callchain() by extracting logic into new
helper functions to provide more flexible stack-sampling APIs for BPF.

> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index fd1d91017..14a382cad 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -67,6 +67,7 @@ struct perf_callchain_entry_ctx {
>  	u32				nr;
>  	short				contexts;
>  	bool				contexts_maxed;
> +	bool				add_mark;
>  };

[ ... ]

> @@ -1718,9 +1719,17 @@ DECLARE_PER_CPU(struct perf_callchain_entry, perf_callchain_entry);
>
>  extern void perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs);
>  extern void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs);
> +
> +extern void __init_perf_callchain_ctx(struct perf_callchain_entry_ctx *ctx,
> +				      struct perf_callchain_entry *entry,
> +				      u32 max_stack, bool add_mark);
> +
> +extern void __get_perf_callchain_kernel(struct perf_callchain_entry_ctx *ctx, struct pt_regs *regs);
> +extern void __get_perf_callchain_user(struct perf_callchain_entry_ctx *ctx, struct pt_regs *regs);
> +
>  extern struct perf_callchain_entry *
>  get_perf_callchain(struct pt_regs *regs, bool kernel, bool user,
> -		   u32 max_stack, bool crosstask, bool add_mark);
> +		   u32 max_stack, bool crosstask);

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 4d53cdd13..e28b35c7e 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -315,7 +315,7 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
>  		max_depth = sysctl_perf_event_max_stack;
>
>  	trace = get_perf_callchain(regs, kernel, user, max_depth,
> -				   false, false);
> +				   false);
                                       ^^^^^ removed last parameter

Previously bpf_get_stackid() passed add_mark=false to get_perf_callchain().
Now the parameter is gone, but get_perf_callchain() hardcodes add_mark=true
when calling __init_perf_callchain_ctx(). This means BPF stack traces will
now include PERF_CONTEXT_KERNEL/PERF_CONTEXT_USER marker values when they
didn't before.

The markers get added by __get_perf_callchain_kernel() and
__get_perf_callchain_user() when ctx->add_mark is true. These markers are
sentinel values (-128 for KERNEL, -512 for USER) that appear in the trace
IP array alongside actual instruction pointers.

>
>  	if (unlikely(!trace))
>  		/* couldn't fetch the stack trace */
> @@ -452,7 +452,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
>  		trace = get_callchain_entry_for_task(task, max_depth);
>  	else
>  		trace = get_perf_callchain(regs, kernel, user, max_depth,
> -					   crosstask, false);
> +					   crosstask);
                                                   ^^^^^ same issue here

Same problem in __bpf_get_stack() which was also passing add_mark=false.

This changes BPF stack map behavior in user-visible ways:

1) bpf_get_stackid() hashes the entire trace->ip[] array including any
   markers, so identical stacks will now produce different stack IDs.

2) bpf_get_stack() copies trace->ip[] directly to userspace buffers,
   so BPF programs will see marker values mixed with instruction pointers.

Can BPF tools handle these unexpected marker values? The commit message
says "The next patch will modify the BPF part" but doesn't explain this
intermediate behavior change.

> diff --git a/kernel/events/callchain.c b/kernel/events/callchain.c
> index 808c0d7a3..2c36e4906 100644
> --- a/kernel/events/callchain.c
> +++ b/kernel/events/callchain.c

[ ... ]

>  struct perf_callchain_entry *
>  get_perf_callchain(struct pt_regs *regs, bool kernel, bool user,
> -		   u32 max_stack, bool crosstask, bool add_mark)
> +		   u32 max_stack, bool crosstask)
>  {
>  	struct perf_callchain_entry *entry;
>  	struct perf_callchain_entry_ctx ctx;
> -	int rctx, start_entry_idx;
> +	int rctx;
>
>  	/* crosstask is not supported for user stacks */
>  	if (crosstask && user && !kernel)
> @@ -232,34 +273,14 @@ get_perf_callchain(struct pt_regs *regs, bool kernel, bool user,
>  	if (!entry)
>  		return NULL;
>
> -	ctx.entry		= entry;
> -	ctx.max_stack		= max_stack;
> -	ctx.nr			= entry->nr = 0;
> -	ctx.contexts		= 0;
> -	ctx.contexts_maxed	= false;
> +	__init_perf_callchain_ctx(&ctx, entry, max_stack, true);
                                                                ^^^^
Here's where add_mark is hardcoded to true for all callers. The old code
allowed callers to control this via parameter. BPF was passing false,
perf_callchain() was passing true. Now both get true unconditionally.

[ ... ]

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 7541f6f85..eb0f11059 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -8218,7 +8218,7 @@ perf_callchain(struct perf_event *event, struct pt_regs *regs)
>  		return &__empty_callchain;
>
>  	callchain = get_perf_callchain(regs, kernel, user,
> -				       max_stack, crosstask, true);
> +				       max_stack, crosstask);
>  	return callchain ?: &__empty_callchain;
>  }


```

---
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: `perf: Refactor get_perf_callchain`
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18882141979