linux-kernel - Re: [PATCH v2] arm64: perf: Fix access percpu variables in preemptible context

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 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]

Date:   Tue, 12 Jan 2021 16:32:54 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     Will Deacon <will@...nel.org>
Cc:     Lecopzer Chen <lecopzer.chen@...iatek.com>, sumit.garg@...aro.org,
        acme@...nel.org, alexander.shishkin@...ux.intel.com,
        alexandru.elisei@....com, catalin.marinas@....com,
        jolsa@...hat.com, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org,
        matthias.bgg@...il.com, mingo@...hat.com, namhyung@...nel.org,
        oliver.sang@...el.com, peterz@...radead.org, yj.chiang@...iatek.com
Subject: Re: [PATCH v2] arm64: perf: Fix access percpu variables in
 preemptible context

On Tue, Jan 12, 2021 at 03:07:36PM +0000, Will Deacon wrote:
> On Fri, Jan 08, 2021 at 08:55:27PM +0800, Lecopzer Chen wrote:
> > > On Mon, 21 Dec 2020 at 21:53, Lecopzer Chen <lecopzer.chen@...iatek.com> wrote:
> > > >
> > > > commit 367c820ef08082 ("arm64: Enable perf events based hard lockup detector")
> > > > reinitilizes lockup detector after arm64 PMU is initialized and open
> > > > a window for accessing smp_processor_id() in preemptible context.
> > > > Since hardlockup_detector_perf_init() always called in init stage
> > > > with a single cpu, but we initialize lockup detector after the init task
> > > > is migratable.
> > > >
> > > > Fix this by utilizing lockup detector reconfiguration which calls
> > > > softlockup_start_all() on each cpu and calls watatchdog_nmi_enable() later.
> > > > Because softlockup_start_all() use IPI call function to make sure
> > > > watatchdog_nmi_enable() will bind on each cpu and fix this issue.
> > > 
> > > IMO, this just creates unnecessary dependency for hardlockup detector
> > > init via softlockup detector (see the alternative definition of
> > > lockup_detector_reconfigure()).
> > 
> > 
> > The arm64/Kconfig select HAVE_HARDLOCKUP_DETECTOR_PERF if we have NMI:
> > 	select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && HAVE_PERF_EVENTS_NMI
> > 
> > And in lib/Kconfig.debug HARDLOCKUP_DETECTOR select SOFTLOCKUP_DETECTOR automatically.
> > 	config HARDLOCKUP_DETECTOR_PERF
> > 		bool
> > 		select SOFTLOCKUP_DETECTOR
> > 
> > So we don't need to explicitly select softlockup.
> > And actually this patch is not a perfect solution like you said
> > (hardlockup depends on softlockup),
> > but the key point is that lockup_detector_init() seems only design for
> > using in early init stage and not for calling in later deffered initial process.
> 
> I agree; the current usage in armv8_pmu_driver_init() looks very broken to
> me, and bodging it with raw_smp_processor_id() isn't the right solution.
> 
> Maybe we should just revert 367c820ef08082, as this looks like a design
> issue rather than something with a simple fix?

I think that would make sense for now, then we can reconsider the whole
thing rather than looking for a point-fix.

Thanks,
Mark.