Re: [v8][PATCH 03/17] introduce XENMEM_reserved_device_memory_map

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 12/08/2014 05:00 AM, Jan Beulich wrote:
>>>> On 08.12.14 at 07:17, <tiejun.chen@intel.com> wrote:
>> On 2014/12/3 3:47, Konrad Rzeszutek Wilk wrote:
>>> On Mon, Dec 01, 2014 at 05:24:21PM +0800, Tiejun Chen wrote:
>>>> @@ -1101,6 +1129,29 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>>>            break;
>>>>        }
>>>>
>>>> +#ifdef HAS_PASSTHROUGH
>>>> +    case XENMEM_reserved_device_memory_map:
>>>> +    {
>>>> +        struct get_reserved_device_memory grdm;
>>>> +
>>>> +        if ( copy_from_guest(&grdm.map, arg, 1) ||
>>>> +             !guest_handle_okay(grdm.map.buffer, grdm.map.nr_entries) )
>>>> +            return -EFAULT;
>>>> +
>>>
>>> Shouldn't there be an XSM check here?
>>
>> I'm not sure, and Jan should be the author for this patch, so Jan can
>> give you a correct reply.
>
> Hmm, not sure: Daniel, does an operation like this need an XSM
> check? It's not clear whether the absence of such a check in e.g.
> the handling of XENMEM_memory_map, XENMEM_machphys_mapping,
> or XENMEM_maximum_ram_page is intentional (and can be used as
> justification for it to be absent here too - after all the operation is for
> a domain to find out information about only itself).
>
> Jan

I can see a possible reason why an XSM check might be needed here, but
I'm not sufficiently clear on what reserved device memory is to tell
for sure.  My best guess is that it is not needed.

 From my reading of this patchset, this hypercall just identifies regions
of memory that are reserved, similar to exposing the host's e820 map to a
guest.  That seems similar enough to the other XENMEM_* leaks that it is
acceptable to also allow it.  If there is a reason that it would be useful
to hide this, adding hooks to all these locations so that only domains
that use passthrough devices (and therefore need to know about the host
system's memory) will have access is probably the best option.

If a guest who has control of a passthrough device can cause these
reserved ranges to change, then there may be reason to prevent others
from querying them, but that doesn't appear to be the case here.

-- 
Daniel De Graaf
National Security Agency