linux-kernel - Re: [RFC PATCH 1/2] perf: arm_spe: Fix consistency of PMSCR register bit CX

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 10 Feb 2022 17:23:50 +0000
From:   German Gomez <german.gomez@....com>
To:     Leo Yan <leo.yan@...aro.org>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        will@...nel.org, mark.rutland@....com, james.clark@....com
Subject: Re: [RFC PATCH 1/2] perf: arm_spe: Fix consistency of PMSCR register
 bit CX

Hi Leo,

On 08/02/2022 13:00, Leo Yan wrote:
> Hi German,
>
> On Mon, Feb 07, 2022 at 12:06:14PM +0000, German Gomez wrote:
>
> [...]
> Indeed!  I can reproduce the issue now.  And I can capture backtrace
> for arm_spe_pmu_start() with below commands:
>
> # cd /home/leoy/linux/tools/perf
> # ./perf probe --add "arm_spe_pmu_start" -s /home/leoy/linux/ -k /home/leoy/linux/vmlinux
> # echo 1 > /sys/kernel/debug/tracing/events/probe/arm_spe_pmu_start/enable
> # echo stacktrace > /sys/kernel/debug/tracing/events/probe/arm_spe_pmu_start/trigger
>
> ... run your commands with non-root user ...
>
> # cat /sys/kernel/debug/tracing/trace
>
>              dd-7697    [000] d.h2.   506.068700: arm_spe_pmu_start: (arm_spe_pmu_start+0x8/0xe0)
>              dd-7697    [000] d.h3.   506.068701: <stack trace>
> => kprobe_dispatcher
> => kprobe_breakpoint_handler
> => call_break_hook
> [...]
> => do_el0_svc
> => el0_svc
> => el0t_64_sync_handler
> => el0t_64_sync
>
> The backtrace clearly shows the function arm_spe_pmu_start() is
> invoked in the 'dd' process (dd-7697); the flow is:
> - perf program sends IPI to CPU0;
> - 'dd' process is running on CPU0 and it's interrupted to handle IPI;
> - 'dd' process has root capabilities, so it will enable context
>   tracing for non-root perf session.

Thanks for testing, and sharing the commands in your replies!

>
>>>> One way to fix this is by caching the value of the CX bit during the
>>>> initialization of the PMU event, so that it remains consistent for the
>>>> duration of the session.
>>>>
>>>> [...]
> So the patch makes sense to me.  Just a minor comment:
>
> Here we can define a u64 for recording pmscr value rather than a
> bool value.
>
> struct arm_spe_pmu {
>     ...
>     u64 pmscr;
> };

I agree with the comment from Will that it makes more sense to store the
value of the register in the perf_event somehow (due to misunderstanding
from my side, I thought arm_spe_pmu struct was local to the session).

What about perf_event's void *pmu_private?

Thanks,
German