Re: [PATCH v2 16/16] arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support

[Marc Zyngier]

On 31/01/18 13:56, Hanjun Guo wrote:
> Hi Marc,
> 
> On 2018/1/30 1:45, Marc Zyngier wrote:
>>  static int enable_psci_bp_hardening(void *data)
>>  {
>>  	const struct arm64_cpu_capabilities *entry = data;
>>  
>> -	if (psci_ops.get_version)
>> +	if (psci_ops.get_version) {
>> +		if (check_smccc_arch_workaround_1(entry))
>> +			return 0;
> 
> If I'm using the new version SMCCC, the firmware have the choice to decide
> whether this machine needs the workaround, even if the CPU is vulnerable
> for CVE-2017-5715, but..
> 
>> +
>>  		install_bp_hardening_cb(entry,
>>  				       (bp_hardening_cb_t)psci_ops.get_version,
>>  				       __psci_hyp_bp_inval_start,
>>  				       __psci_hyp_bp_inval_end);
> 
> ..the code above seems will enable get_psci_version() for CPU and will
> trap to trust firmware even the new version of firmware didn't say
> we need the workaround, did I understand it correctly?

Well, you only get there if we've established that your CPU is affected
(it has an entry matching its MIDR with the HARDEN_BRANCH_PREDICTOR
capability), and that entry points to enable_psci_bp_hardening. It is
not the firmware that decides whether we need hardening, but the kernel.
The firmware merely provides a facility to apply the hardening.

> I'm ask this because some platform will not expose to users to
> take advantage of CVE-2017-5715, and we can use different firmware
> to report we need such workaround or not, then use a single kernel
> image for both vulnerable platforms and no vulnerable ones.

You cannot have your cake and eat it. If you don't want to workaround
the issue, you can disable the hardening. But asking for the same kernel
to do both depending on what the firmware reports doesn't make much
sense to me.

Thanks,

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