On 22.02.2019 09:48, Cornelia Huck wrote:
> On Thu, 21 Feb 2019 19:43:03 +0100
> Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote:
>
>> While we will not implement interception for query functions yet, we can
>> and should disable functions that have a control bit based on the given
>> CPU model.
>>
>> Let us start with enabling the subfunction interface.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
>> ---
>> arch/s390/include/asm/kvm_host.h | 1 +
>> arch/s390/kvm/kvm-s390.c | 34 +++++++++++++++++++++-----------
>> 2 files changed, 23 insertions(+), 12 deletions(-)
>>
>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 7f4bc58a53b97..5058fe58ece77 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -1258,11 +1258,20 @@ static int kvm_s390_set_processor_feat(struct kvm *kvm,
>> static int kvm_s390_set_processor_subfunc(struct kvm *kvm,
>> struct kvm_device_attr *attr)
>> {
>> - /*
>> - * Once supported by kernel + hw, we have to store the subfunctions
>> - * in kvm->arch and remember that user space configured them.
>> - */
>> - return -ENXIO;
>> + mutex_lock(&kvm->lock);
>> + if (kvm->created_vcpus) {
>> + mutex_unlock(&kvm->lock);
>> + return -EBUSY;
>> + }
>> +
>> + if (copy_from_user(&kvm->arch.model.subfuncs, (void __user *)attr->addr,
>> + sizeof(struct kvm_s390_vm_cpu_subfunc))) {
>
> Is there any need for sanitizing/checking the data provided by
> userspace before sticking it into our internal structures? Not that we
> do anything useful with it yet :)
No, its more like facility bits. Its the value that is returned by the query
function of several instructions. If QEMU wants to lie then the guest might
misbehave but the host kernel is fine.
>
>> + mutex_unlock(&kvm->lock);
>> + return -EFAULT;
>> + }
>> + mutex_unlock(&kvm->lock);
>> +
>> + return 0;
>> }
>
