Re: [PATCH V5 3/3] xen/vm_event: Deny register writes if refused by vm_event reply

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 07/14/2015 05:41 PM, Jan Beulich wrote:
>>>> On 14.07.15 at 15:45, <rcojocaru@bitdefender.com> wrote:
>> On 07/14/2015 03:35 PM, Jan Beulich wrote:
>>>>>> On 13.07.15 at 19:14, <rcojocaru@bitdefender.com> wrote:
>>>> --- a/xen/arch/x86/vm_event.c
>>>> +++ b/xen/arch/x86/vm_event.c
>>>> @@ -22,11 +22,19 @@
>>>>  
>>>>  #include <xen/sched.h>
>>>>  #include <asm/hvm/hvm.h>
>>>> +#include <asm/vm_event.h>
>>>>  
>>>>  int vm_event_init_domain(struct domain *d)
>>>>  {
>>>>      struct vcpu *v;
>>>>  
>>>> +    if ( !d->arch.event_write_data )
>>>> +        d->arch.event_write_data = xzalloc_array(struct monitor_write_data,
>>>> +                                                 d->max_vcpus);
>>>
>>> Looking at this again I wonder why the data isn't being made part of
>>> struct arch_vcpu's vm_event sub-structure. That would also address
>>> the complaint I have here about this not being a guaranteed maximum
>>> page size runtime allocation.
>>
>> I think this is just how the initial suggestion was worded, I'll change it.
> 
> Right - after having sent the reply I started wondering whether
> maybe I had asked for this. But if I did, then surely not with
> xzalloc_array(), but vzalloc().
> 
> If you moved this into struct arch_vcpu (again), then its size would
> likely call for the whole vm_event structure to become indirectly
> accessed and allocated.

In that case would it suffice to just switch to vzalloc() in this case?

I'm not opposed to just placing all the data (this and the
memory-content hiding data) in struct vm_event and allocate that as a
whole, but that would change patch 1/3, 3/3 and also touch other code.


Thanks,
Razvan