linux-kernel - Re: [PATCH 02/14] arm64: Call ARCH_WORKAROUND

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, 24 May 2018 12:28:04 +0100
From:   Marc Zyngier <marc.zyngier@....com>
To:     Mark Rutland <mark.rutland@....com>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        kvmarm@...ts.cs.columbia.edu, Kees Cook <keescook@...omium.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Andy Lutomirski <luto@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 02/14] arm64: Call ARCH_WORKAROUND_2 on transitions
 between EL0 and EL1

On 24/05/18 12:23, Mark Rutland wrote:
> On Thu, May 24, 2018 at 12:00:58PM +0100, Mark Rutland wrote:
>> On Tue, May 22, 2018 at 04:06:36PM +0100, Marc Zyngier wrote:
>>> In order for the kernel to protect itself, let's call the SSBD mitigation
>>> implemented by the higher exception level (either hypervisor or firmware)
>>> on each transition between userspace and kernel.
>>>
>>> We must take the PSCI conduit into account in order to target the
>>> right exception level, hence the introduction of a runtime patching
>>> callback.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@....com>
>>> ---
>>>  arch/arm64/kernel/cpu_errata.c | 18 ++++++++++++++++++
>>>  arch/arm64/kernel/entry.S      | 22 ++++++++++++++++++++++
>>>  include/linux/arm-smccc.h      |  5 +++++
>>>  3 files changed, 45 insertions(+)
>>>
>>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>>> index a900befadfe8..46b3aafb631a 100644
>>> --- a/arch/arm64/kernel/cpu_errata.c
>>> +++ b/arch/arm64/kernel/cpu_errata.c
>>> @@ -232,6 +232,24 @@ enable_smccc_arch_workaround_1(const struct arm64_cpu_capabilities *entry)
>>>  }
>>>  #endif	/* CONFIG_HARDEN_BRANCH_PREDICTOR */
>>>  
>>> +#ifdef CONFIG_ARM64_SSBD
>>> +void __init arm64_update_smccc_conduit(struct alt_instr *alt,
>>> +				       __le32 *origptr, __le32 *updptr,
>>> +				       int nr_inst)
>>> +{
>>> +	u32 insn;
>>> +
>>> +	BUG_ON(nr_inst != 1);
>>> +
>>> +	if (psci_ops.conduit == PSCI_CONDUIT_HVC)
>>> +		insn = aarch64_insn_get_hvc_value();
>>> +	else
>>> +		insn = aarch64_insn_get_smc_value();
>>
>> Shouldn't this also handle the case where there is no conduit?
> 
> Due to the config symbol not being defined yet, and various other fixups
> in later patches, this is actually benign.
> 
> However, if you make this:
> 
> 	switch (psci_ops.conduit) {
> 	case PSCI_CONDUIT_NONE:
> 		return;
> 	case PSCI_CONDUIT_HVC:
> 		insn = aarch64_insn_get_hvc_value();
> 		break;
> 	case PSCI_CONDUIT_SMC:
> 		insn = aarch64_insn_get_smc_value();
> 		break;
> 	}
> 
> ... then we won't even bother patching the nop in the default case
> regardless, which is nicer, IMO.

Yup, looks better to me too. I'll fold that in.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...