On Tue, Dec 12, 2017 at 10:37:31AM +0100, David Hildenbrand wrote:
> On 12.12.2017 10:02, Roman Kagan wrote:
> > On Mon, Dec 11, 2017 at 10:14:48PM +0100, David Hildenbrand wrote:
> >>
> >>> #endif /* __LINUX_KVM_H */
> >>> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> >>> index 015fb06c7522..0303bd8f4d6a 100644
> >>> --- a/arch/x86/kvm/hyperv.c
> >>> +++ b/arch/x86/kvm/hyperv.c
> >>> @@ -29,6 +29,7 @@
> >>> #include <linux/kvm_host.h>
> >>> #include <linux/highmem.h>
> >>> #include <linux/sched/cputime.h>
> >>> +#include <linux/eventfd.h>
> >>>
> >>> #include <asm/apicdef.h>
> >>> #include <trace/events/kvm.h>
> >>> @@ -1226,6 +1227,54 @@ static int kvm_hv_hypercall_complete_userspace(struct kvm_vcpu *vcpu)
> >>> return 1;
> >>> }
> >>>
> >>> +static u16 hvcall_sigevent_param(struct kvm_vcpu *vcpu, gpa_t gpa, u64 *param)
> >>> +{
> >>> + struct page *page;
> >>> + void *pg;
> >>> + u64 *msg;
> >>> +
> >>> + if ((gpa & (__alignof__(*msg) - 1)) ||
> >>> + offset_in_page(gpa) + sizeof(*msg) > PAGE_SIZE)
> >>> + return HV_STATUS_INVALID_ALIGNMENT;
> >>> +
> >>
> >> Don't we also need srcu_read_lock(&vcpu->kvm->srcu) for
> >> kvm_vcpu_gfn_to_page?
> >
> > Seems so indeed. I also think I'll switch to using
> > kvm_vcpu_read_guest().
>
> Probably better (because I think you also missed releasing the page)
Hmm, I wonder where? (kvm_vcpu_read_guest ended up being more concise
so I'll stick with it but I'd like to learn from my errors anyway.)
>
> >
> >>> + page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
> >>> + if (is_error_page(page))
> >>> + return HV_STATUS_INSUFFICIENT_MEMORY;
> >>> +
> >>> + pg = kmap_atomic(page);
> >>> + msg = pg + offset_in_page(gpa);
> >>> + *param = *msg;
> >>> + kunmap_atomic(pg);
> >>> + return HV_STATUS_SUCCESS;
> >>> +}
> >>> +
> >>> +static u16 kvm_hvcall_signal_event(struct kvm_vcpu *vcpu, bool fast, u64 param)
> >>> +{
> >>> + u16 ret;
> >>> + u32 conn_id;
> >>> + int idx;
> >>> + struct eventfd_ctx *eventfd;
> >>> +
> >>> + if (unlikely(!fast)) {
> >>> + u64 gpa = param;
> >>> + ret = hvcall_sigevent_param(vcpu, gpa, ¶m);
> >>> + if (ret != HV_STATUS_SUCCESS)
> >>> + return ret;
> >>> + }
> >>> +
> >>> + conn_id = (param & 0xffffffff) + ((param >> 32) & 0xffff);
> >>
> >> conn_id = param + ((param >> 32) & 0xffff);
> >>
> >> (as param will me automatically truncated to 32 bits - sizeof(conn_id))
> >
> > Agreed. However, it looks like what I really need is
> >
> > conn_id = (param & KVM_HYPERV_CONN_ID_MASK) + ((param >> 32) & 0xffff);
> >
> > otherwise the overflown conn_id may pass the following check
>
> That would have been my next question :)
Thanks,
Roman.
