Hi, On 4/1/20 5:58 PM, Marc Zyngier wrote: > When a guest delibarately uses an SSMC32 function number (which is allowed),
s/SSMC32/SMC32 > we should make sure we drop the top 32bits from the input arguments, as they > could legitimately be junk. > > Reported-by: Christoffer Dall <christoffer.d...@arm.com> > Signed-off-by: Marc Zyngier <m...@kernel.org> > --- > virt/kvm/arm/psci.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c > index 17e2bdd4b76f..69ff4a51ceb5 100644 > --- a/virt/kvm/arm/psci.c > +++ b/virt/kvm/arm/psci.c > @@ -187,6 +187,18 @@ static void kvm_psci_system_reset(struct kvm_vcpu *vcpu) > kvm_prepare_system_event(vcpu, KVM_SYSTEM_EVENT_RESET); > } > > +static void kvm_psci_narrow_to_32bit(struct kvm_vcpu *vcpu) > +{ > + int i; > + > + /* > + * Zero the input registers' upper 32 bits. They will be fully > + * zeroed on exit, so we're fine changing them in place. > + */ > + for (i = 1; i < 4; i++) > + vcpu_set_reg(vcpu, i, (u32)vcpu_get_reg(vcpu, i)); One minor suggestion, it could be lower_32_bits instead, but that's down to personal preference and entirely up to you. > +} > + > static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu) > { > struct kvm *kvm = vcpu->kvm; > @@ -211,12 +223,16 @@ static int kvm_psci_0_2_call(struct kvm_vcpu *vcpu) > val = PSCI_RET_SUCCESS; > break; > case PSCI_0_2_FN_CPU_ON: > + kvm_psci_narrow_to_32bit(vcpu); > + fallthrough; > case PSCI_0_2_FN64_CPU_ON: > mutex_lock(&kvm->lock); > val = kvm_psci_vcpu_on(vcpu); > mutex_unlock(&kvm->lock); > break; > case PSCI_0_2_FN_AFFINITY_INFO: > + kvm_psci_narrow_to_32bit(vcpu); > + fallthrough; > case PSCI_0_2_FN64_AFFINITY_INFO: > val = kvm_psci_vcpu_affinity_info(vcpu); > break; >From ARM DEN 0022D, those are indeed the only functions with ids that differ >from SMC32 to SMC64, and have arguments that KVM doesn't ignore (like it does with CPU_SUSPEND). I also had a look at smccc_get_arg{1,2,3}, because they read the register values and return an unsigned long. smccc_get_arg1 is called after the registers have been narrowed, or the result is cast into an u32 when called before that. smccc_get_arg{2,3} are always called as part of the individual PSCI function implementations, which come after the arguments have been narrowed. With that: Reviewed-by: Alexandru Elisei <alexandru.eli...@arm.com> Thanks, Alex _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm