linux-kernel - Re: [RFC PATCH v2 3/7] arm64: alternative: Apply alternatives early in boot process

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:	Wed, 16 Sep 2015 17:24:52 +0100
From:	Will Deacon <will.deacon@....com>
To:	Daniel Thompson <daniel.thompson@...aro.org>
Cc:	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Catalin Marinas <Catalin.Marinas@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"patches@...aro.org" <patches@...aro.org>,
	"linaro-kernel@...ts.linaro.org" <linaro-kernel@...ts.linaro.org>,
	John Stultz <john.stultz@...aro.org>,
	Sumit Semwal <sumit.semwal@...aro.org>,
	Marc Zyngier <Marc.Zyngier@....com>,
	Andrew Thoelke <Andrew.Thoelke@....com>,
	Dave P Martin <Dave.Martin@....com>
Subject: Re: [RFC PATCH v2 3/7] arm64: alternative: Apply alternatives early
 in boot process

On Wed, Sep 16, 2015 at 04:51:12PM +0100, Daniel Thompson wrote:
> On 16/09/15 14:05, Will Deacon wrote:
> > On Mon, Sep 14, 2015 at 02:26:17PM +0100, Daniel Thompson wrote:
> >> Currently alternatives are applied very late in the boot process (and
> >> a long time after we enable scheduling). Some alternative sequences,
> >> such as those that alter the way CPU context is stored, must be applied
> >> much earlier in the boot sequence.
> >>
> >> Introduce apply_alternatives_early() to allow some alternatives to be
> >> applied immediately after we detect the CPU features of the boot CPU.
> >>
> >> Currently apply_alternatives_all() is not optimized and will re-patch
> >> code that has already been updated. This is harmless but could be
> >> removed by adding extra flags to the alternatives store.
> >>
> >> Signed-off-by: Daniel Thompson <daniel.thompson@...aro.org>
> >> ---
> > [snip]
> >>   /*
> >> + * This is called very early in the boot process (directly after we run
> >> + * a feature detect on the boot CPU). No need to worry about other CPUs
> >> + * here.
> >> + */
> >> +void apply_alternatives_early(void)
> >> +{
> >> +	struct alt_region region = {
> >> +		.begin	= __alt_instructions,
> >> +		.end	= __alt_instructions_end,
> >> +	};
> >> +
> >> +	__apply_alternatives(&region);
> >> +}
> >
> > How do you choose which alternatives are applied early and which are
> > applied later? AFAICT, this just applies everything before we've
> > established the capabilities of the CPUs in the system, which could cause
> > problems for big/little SoCs.
> 
> They are applied twice. This relies for correctness on the fact that 
> cpufeatures can be set but not unset.
> 
> In other words the boot CPU does a feature detect and, as a result, a 
> subset of the required alternatives will be applied. However after this 
> the other CPUs will boot and the the remaining alternatives applied as 
> before.
> 
> The current implementation is inefficient (because it will redundantly 
> patch the same code twice) but I don't think it is broken.

What about a big/little system where we boot on the big cores and only
they support LSE atomics?

> > Also, why do we need this for the NMI?
> 
> I was/am concerned that a context saved before the alternatives are 
> applied might be restored afterwards. If that happens the bit that 
> indicates what value to put into the PMR would read during the restore 
> without having been saved first. Applying early ensures that the context 
> save/restore code is updated before it is ever used.

Damn, and stop_machine makes use of local_irq_restore immediately after
the patching has completed, so it's a non-starter. Still, special-casing
this feature via an explicit apply_alternatives call would be better
than moving everything earlier, I think.

We also need to think about how an incoming NMI interacts with
concurrent patching of later features. I suspect we want to set the I
bit, like you do for WFI, unless you can guarantee that no patched
sequences run in NMI context.

Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/