Re: [Xen-devel] [PATCH 2/2] xen/physmap: Do not permit a guest to populate PoD pages for itself

2016-08-19 Thread Jan Beulich
>>> On 19.08.16 at 17:02,  wrote:
> On 19/08/16 15:58, Jan Beulich wrote:
> On 19.08.16 at 16:12,  wrote:
>>> --- a/xen/common/memory.c
>>> +++ b/xen/common/memory.c
>>> @@ -903,7 +903,16 @@ long do_memory_op(unsigned long cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>>>  
>>>  if ( op == XENMEM_populate_physmap
>>>   && (reservation.mem_flags & XENMEMF_populate_on_demand) )
>>> +{
>>> +/* Disallow populating PoD pages on oneself. */
>>> +if ( d == curr_d )
>>> +{
>>> +rcu_unlock_domain(d);
>>> +return start_extent;
>>> +}
>>> +
>>>  args.memflags |= MEMF_populate_on_demand;
>>> +}
>>>  
>>>  if ( xsm_memory_adjust_reservation(XSM_TARGET, curr_d, d) )
>>>  {
>> Considering the code continues
>>
>> rcu_unlock_domain(d);
>> return start_extent;
>> }
>>
>> switch ( op )
>> {
>> case XENMEM_increase_reservation:
>> increase_reservation(&args);
>> break;
>> case XENMEM_decrease_reservation:
>> decrease_reservation(&args);
>> break;
>> default: /* XENMEM_populate_physmap */
>> populate_physmap(&args);
>> break;
>> }
>>
>> I think it is time to move this XENMEM_populate_physmap specific
>> code chunk into populate_physmap(), at once eliminating the need
>> for another not entirely trivial error exit path here.
> 
> Do you mean just my code addition, or the whole if condition?  The
> former is easy, but the latter is hard as reservation is specifically
> not passed into populate_physmap().

Oh, I didn't realize that - just your addition then.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 2/2] xen/physmap: Do not permit a guest to populate PoD pages for itself

2016-08-19 Thread Andrew Cooper
On 19/08/16 15:58, Jan Beulich wrote:
 On 19.08.16 at 16:12,  wrote:
>> --- a/xen/common/memory.c
>> +++ b/xen/common/memory.c
>> @@ -903,7 +903,16 @@ long do_memory_op(unsigned long cmd, 
>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>  
>>  if ( op == XENMEM_populate_physmap
>>   && (reservation.mem_flags & XENMEMF_populate_on_demand) )
>> +{
>> +/* Disallow populating PoD pages on oneself. */
>> +if ( d == curr_d )
>> +{
>> +rcu_unlock_domain(d);
>> +return start_extent;
>> +}
>> +
>>  args.memflags |= MEMF_populate_on_demand;
>> +}
>>  
>>  if ( xsm_memory_adjust_reservation(XSM_TARGET, curr_d, d) )
>>  {
> Considering the code continues
>
> rcu_unlock_domain(d);
> return start_extent;
> }
>
> switch ( op )
> {
> case XENMEM_increase_reservation:
> increase_reservation(&args);
> break;
> case XENMEM_decrease_reservation:
> decrease_reservation(&args);
> break;
> default: /* XENMEM_populate_physmap */
> populate_physmap(&args);
> break;
> }
>
> I think it is time to move this XENMEM_populate_physmap specific
> code chunk into populate_physmap(), at once eliminating the need
> for another not entirely trivial error exit path here.

Do you mean just my code addition, or the whole if condition?  The
former is easy, but the latter is hard as reservation is specifically
not passed into populate_physmap().

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 2/2] xen/physmap: Do not permit a guest to populate PoD pages for itself

2016-08-19 Thread Jan Beulich
>>> On 19.08.16 at 16:12,  wrote:
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -903,7 +903,16 @@ long do_memory_op(unsigned long cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  
>  if ( op == XENMEM_populate_physmap
>   && (reservation.mem_flags & XENMEMF_populate_on_demand) )
> +{
> +/* Disallow populating PoD pages on oneself. */
> +if ( d == curr_d )
> +{
> +rcu_unlock_domain(d);
> +return start_extent;
> +}
> +
>  args.memflags |= MEMF_populate_on_demand;
> +}
>  
>  if ( xsm_memory_adjust_reservation(XSM_TARGET, curr_d, d) )
>  {

Considering the code continues

rcu_unlock_domain(d);
return start_extent;
}

switch ( op )
{
case XENMEM_increase_reservation:
increase_reservation(&args);
break;
case XENMEM_decrease_reservation:
decrease_reservation(&args);
break;
default: /* XENMEM_populate_physmap */
populate_physmap(&args);
break;
}

I think it is time to move this XENMEM_populate_physmap specific
code chunk into populate_physmap(), at once eliminating the need
for another not entirely trivial error exit path here.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel