Re: [PATCH 0/2] KVM: fixes for the kernel-hardening tree

[Christian Borntraeger]

On 10/25/2017 11:45 AM, David Hildenbrand wrote:
> On 23.10.2017 16:15, Paolo Bonzini wrote:
>> On 23/10/2017 14:39, Cornelia Huck wrote:
>>> On Mon, 23 Oct 2017 11:52:51 +0200
>>> David Hildenbrand <david@xxxxxxxxxx> wrote:
>>>
>>>> On 21.10.2017 01:25, Paolo Bonzini wrote:
>>>>> Two KVM ioctls (KVM_GET/SET_CPUID2) directly access the cpuid_entries
>>>>> field of struct kvm_vcpu_arch.  Therefore, the new usercopy hardening
>>>>> work in linux-next, which forbids copies from and to slab objects
>>>>> unless they are from kmalloc or explicitly whitelisted, breaks KVM
>>>>> completely.
>>>>>
>>>>> This series fixes it by adding the two new usercopy arguments
>>>>> to kvm_init (more precisely to a new function kvm_init_usercopy,
>>>>> while kvm_init passes zeroes as a default).
>>>>>
>>>>> There's also another broken ioctl, KVM_XEN_HVM_CONFIG, but it is
>>>>> obsolete and not a big deal at all.
>>>>>
>>>>> I'm Ccing all submaintainers in case they have something similar
>>>>> going on in their kvm_arch and kvm_vcpu_arch structs.  KVM has a
>>>>> pretty complex userspace API, so thorough with linux-next is highly
>>>>> recommended.  
>>>>
>>>> I assume on s390x, at least
>>>>
>>>> kvm_arch_vcpu_ioctl_get_one_reg() and
>>>> kvm_arch_vcpu_ioctl_set_one_reg()
>>>>
>>>> have to be fixed.
>>>
>>> At a glance, seems like it.
>>>
>>>>
>>>> Christian, are you already looking into this?
>>>
>>> I'm afraid I'm also busy with travel preparation/travel, so I'd be glad
>>> for any takers.
>>
>> Let's do a generic fix now, so that we don't need to rush the switch to
>> explicit whitelisting.
> 
> You mean a arch specific fix (allow writes/reads to arch) or even more
> generic?

Kees,

I am somewhat worried about these changes. The onereg interface is certainly
broken right now, but newer QEMUs will not use it. So its very likely that
testing will not find the regression. I would assume that usercopy hardinging
will introduce a lot of non-obvious regressions in seldomly used code. Have you
considered some debugging aids to find "now broken" things?
> 
> Otherwise I can you send a patch to fix these two functions.

Having said that, yes if you can send a fixup for 
kvm_arch_vcpu_ioctl_get/set_one_reg that would be great.

Christian