linux-kernel - Re: [PATCH v4 3/3] perf/core: Check sample_type in perf_sample_save

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAM9d7chNz8-84m28q5qSLjUjZ=Ni1CA_JzbB_P+YJooLQd85YA@mail.gmail.com>
Date: Fri, 10 May 2024 14:29:58 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Yabin Cui <yabinc@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, 
	Arnaldo Carvalho de Melo <acme@...nel.org>, Mark Rutland <mark.rutland@....com>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, 
	Ian Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>, 
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org, 
	bpf@...r.kernel.org
Subject: Re: [PATCH v4 3/3] perf/core: Check sample_type in perf_sample_save_brstack

On Fri, May 10, 2024 at 12:14 PM Yabin Cui <yabinc@...gle.com> wrote:
>
> Check sample_type in perf_sample_save_brstack() to prevent
> saving branch stack data when it isn't required.
>
> Suggested-by: Namhyung Kim <namhyung@...nel.org>
> Signed-off-by: Yabin Cui <yabinc@...gle.com>

It seems powerpc has the similar bug, then you need this:

Fixes: eb55b455ef9c ("perf/core: Add perf_sample_save_brstack() helper")

Thanks,
Namhyung

> ---
>  arch/x86/events/amd/core.c |  3 +--
>  arch/x86/events/core.c     |  3 +--
>  arch/x86/events/intel/ds.c |  3 +--
>  include/linux/perf_event.h | 13 ++++++++-----
>  4 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> index 985ef3b47919..fb9bf3aa1b42 100644
> --- a/arch/x86/events/amd/core.c
> +++ b/arch/x86/events/amd/core.c
> @@ -967,8 +967,7 @@ static int amd_pmu_v2_handle_irq(struct pt_regs *regs)
>                 if (!x86_perf_event_set_period(event))
>                         continue;
>
> -               if (has_branch_stack(event))
> -                       perf_sample_save_brstack(&data, event, &cpuc->lbr_stack, NULL);
> +               perf_sample_save_brstack(&data, event, &cpuc->lbr_stack, NULL);
>
>                 if (perf_event_overflow(event, &data, regs))
>                         x86_pmu_stop(event, 0);
> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index 5b0dd07b1ef1..ff5577315938 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -1702,8 +1702,7 @@ int x86_pmu_handle_irq(struct pt_regs *regs)
>
>                 perf_sample_data_init(&data, 0, event->hw.last_period);
>
> -               if (has_branch_stack(event))
> -                       perf_sample_save_brstack(&data, event, &cpuc->lbr_stack, NULL);
> +               perf_sample_save_brstack(&data, event, &cpuc->lbr_stack, NULL);
>
>                 if (perf_event_overflow(event, &data, regs))
>                         x86_pmu_stop(event, 0);
> diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
> index c2b5585aa6d1..f25236ffa28f 100644
> --- a/arch/x86/events/intel/ds.c
> +++ b/arch/x86/events/intel/ds.c
> @@ -1754,8 +1754,7 @@ static void setup_pebs_fixed_sample_data(struct perf_event *event,
>         if (x86_pmu.intel_cap.pebs_format >= 3)
>                 setup_pebs_time(event, data, pebs->tsc);
>
> -       if (has_branch_stack(event))
> -               perf_sample_save_brstack(data, event, &cpuc->lbr_stack, NULL);
> +       perf_sample_save_brstack(data, event, &cpuc->lbr_stack, NULL);
>  }
>
>  static void adaptive_pebs_save_regs(struct pt_regs *regs,
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 8617815456b0..ecfbe22ff299 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -1269,6 +1269,11 @@ static inline void perf_sample_save_raw_data(struct perf_sample_data *data,
>         data->sample_flags |= PERF_SAMPLE_RAW;
>  }
>
> +static inline bool has_branch_stack(struct perf_event *event)
> +{
> +       return event->attr.sample_type & PERF_SAMPLE_BRANCH_STACK;
> +}
> +
>  static inline void perf_sample_save_brstack(struct perf_sample_data *data,
>                                             struct perf_event *event,
>                                             struct perf_branch_stack *brs,
> @@ -1276,6 +1281,9 @@ static inline void perf_sample_save_brstack(struct perf_sample_data *data,
>  {
>         int size = sizeof(u64); /* nr */
>
> +       if (!has_branch_stack(event))
> +               return;
> +
>         if (branch_sample_hw_index(event))
>                 size += sizeof(u64);
>         size += brs->nr * sizeof(struct perf_branch_entry);
> @@ -1665,11 +1673,6 @@ extern void perf_bp_event(struct perf_event *event, void *data);
>  # define perf_arch_bpf_user_pt_regs(regs) regs
>  #endif
>
> -static inline bool has_branch_stack(struct perf_event *event)
> -{
> -       return event->attr.sample_type & PERF_SAMPLE_BRANCH_STACK;
> -}
> -
>  static inline bool needs_branch_stack(struct perf_event *event)
>  {
>         return event->attr.branch_sample_type != 0;
> --
> 2.45.0.118.g7fe29c98d7-goog
>