rdcycle from userland with RISCV_PMU_SBI=y

[Anup Patel]

On Fri, Sep 2, 2022 at 1:23 PM Mathieu Malaterre <malat at debian.org> wrote:
>
> Atish,
>
> On Fri, Sep 2, 2022 at 9:12 AM Atish Patra <atishp at atishpatra.org> wrote:
> >
> >
> >
> > On Thu, Sep 1, 2022 at 3:34 PM Atish Patra <atishp at atishpatra.org> wrote:
> >>
> >>
> >>
> >> On Thu, Sep 1, 2022 at 2:15 PM Aurelien Jarno <aurelien at aurel32.net> wrote:
> >>>
> >>> Hi all,
> >>>
> >>> Mathieu Malaterre reported an issue in Debian with the highway package,
> >>> which uses the rdcycle pseudo instruction, and started to fail recently.
> >>>
> >>> We tracked down an issue to the "perf platform driver based on SBI PMU
> >>> extension" recently added in Linux 5.18 [1]. On a board which runs an
> >>> SBI with the PMU extension, this causes the rdcycle pseudo instruction
> >>> to generate a SIGILL. This is because the driver explicitly set
> >>> CSR_SCOUNTEREN to 0x02, giving access from userland to rdtime but not
> >>> rdcycle.
> >>>
> >>> I wonder if this change is intentional, and if there is a reason to
> >>> forbid using the rdcycle pseudo instruction from userland. If it is the
> >>> case, this should probably be changed so that the behaviour does not
> >>> differ from board to board depending on the available PMU extension.
> >>>
> >>
> >> Yes. The change was intentional due to security reasons. One rogue process can have access to cycle &
> >> instructions of the entire kernel always which can lead to some sort of side channel attacks.
> >>
> >> However, I agree that we can't break userspace. I was not aware of the fact that there are already users of rdcycle
> >> in the userspace. I will send a patch to restore the original behavior by enabling CY, IR bits in scounteren.
> >>
> >
> > Merging the thread from sw-dev:
> >
> > "
> > > > On Thu, Sep 1, 2022 at 7:36 PM Paul Walmsley <paul.walmsley at sifive.com> wrote:
> > > >>
> > > >> If I recall correctly,  other architectures don't allow direct access to their cycle counters from userspace for security reasons.   Any reason why RISC-V shouldn't follow the same approach?
> > > >
> > > >
> > > > That was the intention behind disabling the access in the PMU driver. At that time, I didn’t find any users. Obviously I was wrong 😂. But is that a sufficient reason to existing break user space ?
> > >
> > > As already pointed out, we can't compromise security by having all
> > > apps unrestricted access to the cycle counter. We already have the
> > > Linux perf subsystem for managing counters so apps should always use
> > > Linux perf syscalls.
> > >
> > > IMO, the package "highway" directly accessing the cycle counter should
> > > be fixed instead of fixing the Linux SBI PMU driver.
> > >
> > > Further investigating the highway project
> > > (https://github.com/google/highway), it seems this project is using
> > > "rdcycle" to track timer ticks which is totally wrong. Instead the use
> > > of "rdcycle" should be replaced with "rdtime" in this project.
> > > (Refer, line 156 of
> > > https://github.com/google/highway/blob/master/hwy/nanobenchmark.cc)
> >
> > Pay attention that quite a few projects are using rdcycle in user-space already:
> >
> > * https://codesearch.debian.net/search?q=%22rdcycle+%250%22
> >
> > Anyway if you believe rdtime is the right fix, we can fix one project
> > at a time..."
> >
> > Thanks for sharing the list. I took a quick look. The first few ones at least  (firefox, llvm, chrome)
> > seems to use rdcycle for timestamp. All of the packages use "rdtsc" for x86.
> > The equivalent instruction in RISC-V should be rdtime not rdcycle.
>
> [CC-ing highway main dev]
>
> Here is his comment on the rdtime patch proposal:
>
> > For benchmarking we really would prefer a cycle count, rather than "real time clock ticks", for which the spec makes no guarantees. If/when those turn out to be millisecond-resolution, they are not very useful.

The RISC-V spec does not make any guarantee about granularity of the
cycle counter as well. In fact, on systems with cpufreq enabled the
rate of cycle counter increment will vary at runtime.

On other hand, the time CSR (i.e. rdtime) will always increment at a
fixed platform specific rate which can be read from
"/proc/device-tree/cpus/timebase-frequency".

Regards,
Anup


>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv